2

note: Java NOOB.

Alright, I know this has been answered a few dozen times on here, but the solutions don't seem to work/apply directly to where I understand them. (Yes, I know I don't completely understand encryption/decryption, AES, etc. but that is not the point, I am trying to understand this)

I have a utility api where I want to pass a string and return an encrypted string. Then I want to pass the encrypted string, and return a decrypted string. Simple. It works fine for many strings I pass in, but on some, I'm getting the exception javax.crypto.BadPaddingException: Given final block not properly padded.

For example, the following encrypts/decrypts fine.
util/encrypt/?token=123456789012wha = 4TR0CbCcQKqeRK73zr83aw==
util/decrypt/?token=4TR0CbCcQKqeRK73zr83aw== = 123456789012wha

The following encrypts, but does not decrypt:
util/encrypt/?token=123456789012what = NYaWmwnySoGNHyNmY9Jh+f3O2rqoLI1IAcnsl5V4OCE=
util/decrypt/?token=NYaWmwnySoGNHyNmY9Jh+f3O2rqoLI1IAcnsl5V4OCE= = exception

Here is the code in my controller:

private static final String ALGO = "AES";


@RequestMapping(value = "/util/encrypt/", method = RequestMethod.GET)
@ResponseBody
public String encrypt(HttpServletResponse httpResponse,
        @RequestParam(value = "token", required=true) String token) throws Exception 
{
    Key key = generateKey();
    Cipher c = Cipher.getInstance(ALGO);
    c.init(Cipher.ENCRYPT_MODE, key);
    byte[] encVal = c.doFinal(token.getBytes());
    String encryptedValue = Base64.encodeBase64String(encVal);
    return encryptedValue.trim();
}



@RequestMapping(value = "/util/decrypt/", method = RequestMethod.GET)
@ResponseBody
public String decrypt(HttpServletResponse httpResponse,
        @RequestParam(value = "token", required=true) String token) throws Exception 
{
    token = URLDecoder.decode(token, "UTF-8");
    Key key = generateKey();
    Cipher c = Cipher.getInstance(ALGO);
    c.init(Cipher.DECRYPT_MODE, key);
    byte[] decordedValue = Base64.decodeBase64(token);
    byte[] decValue = c.doFinal(decordedValue);
    String decryptedValue = new String(decValue);
    return decryptedValue.trim();
}



private Key generateKey() throws Exception 
{
    Key key = new SecretKeySpec(getAesKey().getBytes(), ALGO);
    return key;
}

I figure it must be something with the call Cipher.getInstance() and I've tried using Cipher.getInstance("AES/CBC/PKCS5Padding") but that always fails when decrypting. I would love to really understand what is happening here and how to fix it.

earthling
  • 5,084
  • 9
  • 46
  • 90
  • 1
    Note that `"AES"` defaults to `"AES/ECB/PKCS5Padding"` (for the SUN provider) and is not fit for encrypting strings. Also note that you normally require an authentication tag when sending ciphertext over a network connection (e.g. to defend against padding oracle attacks). Working code does not equal cryptographically secure code. – Maarten Bodewes Jun 25 '13 at 00:25
  • ECB mode is incredibly dangerous to use like this. A trivial chosen plaintext attack allows an adversary to discover the ciphertexts of whichever blocks he wants. Then he can juggle around those blocks as much as necessary (even between different ciphertexts, ECB doesn't care), and then submit the result for decryption. – ntoskrnl Jun 25 '13 at 08:03
  • @ntoskrnl - are you referring to the `Cipher.getInstance("AES")` call? I assume the recommended change is to use `AES/CBC/PKCS5Padding`, correct? I still need to learn the difference between ECB and CBC. – earthling Jun 25 '13 at 17:03
  • @owlstead, how would you guys recommend implementing this? there is one suggestion below to use code.google.com/p/keyczar. – earthling Jun 25 '13 at 17:04
  • 2
    @earthling I would certainly perform a HMAC signing after encryption (using a different key), and verify the MAC value before decryption. Higher level cryptography frameworks such as Keyczar (which I don't use personally) are almost certainly safer than rolling your own solution - especially if you simply follow examples on the internet instead of knowing cryptography in general. – Maarten Bodewes Jun 25 '13 at 23:20
  • 1
    @earthling Yes, CBC is certainly better than ECB. But as owlstead pointed out, verifying the authenticity of the ciphertext is fairly important in this case too. You can do it either using HMAC, or (preferably) by using something like [GCM](http://en.wikipedia.org/wiki/Galois/Counter_Mode), which combines encryption and authentication. Note that you'll have to also transmit 128 bits for the IV and 256 bits for the authentication tag with CBC + HMAC-SHA256, while you'd typically use a 96-bit IV and 96-bit authentication tag with GCM. – ntoskrnl Jun 26 '13 at 06:32
  • 1
    @ntoskrnl ...and you can use the same key for GCM. But GCM is not as widespread as HMAC. – Maarten Bodewes Jun 27 '13 at 22:30
  • @owlstead Actually, GCM is is in the [NSA Suite B](http://en.wikipedia.org/wiki/NSA_Suite_B_Cryptography), which basically means that it's certified for use by the US government. Many application vendors opt to use those algorithms, simply because they are known to work optimally. GCM (and Suite B) are relatively recent though, which probably explains why they are not yet used so widely. – ntoskrnl Jun 28 '13 at 06:14
  • 1
    @ntoskrnl Yeah, and I'm a big advocate of them, but sometimes you still need to check if they are available for the required runtimes. The fact that they are mentioned in (upcoming) TLS and XML encryption specifications gives us some hope :) – Maarten Bodewes Jun 28 '13 at 08:45

2 Answers2

3

Use the function encodeBase64URLSafeString. the javadoc says

Encodes binary data using a URL-safe variation of the base64 algorithm but does not chunk the output. The url-safe variation emits - and _ instead of + and / characters. Note: no padding is added.

This should solve the problem.

fmodos
  • 4,472
  • 1
  • 16
  • 17
2

Since this class operates directly on byte streams, and not character streams, it is hard-coded to only encode/decode character encodings which are compatible with the lower 127 ASCII chart (ISO-8859-1, Windows-1252, UTF-8, etc).

Your conversion to/from byte[]/string does not preserve all the data. Textual representation of the ciphertext isn't really necessary so why are you converting to a string?

Jean-Bernard Pellerin
  • 12,556
  • 10
  • 57
  • 79
  • I'm not quire sure I understand what you're asking. I'm converting the cyphertext to a string to return to the client. – earthling Jun 24 '13 at 22:40
  • 1
    not all byte sequences are valid UTF-8. at some point your code is going to raise an exception. the other answer here will fix this and also solve the problem you're seeing which is because of the '+' in the URL. and - i know you think you're too smart to care, but - writing your own crypto is just wrong. find a library that does what you want. – andrew cooke Jun 24 '13 at 23:14
  • I'm not too smart to care, thankfully :) I'd love a library if you know one. When I search I get AES examples and not a library. Thanks for your help. – earthling Jun 24 '13 at 23:24
  • I think I want something like php's mcrypt libary. That is what our front end team is using. – earthling Jun 24 '13 at 23:32
  • 2
    sorry that was a bit snarky. in general, if you can get https://code.google.com/p/keyczar/ to work then that's the best solution. it tries hard to keep you from making mistakes. – andrew cooke Jun 24 '13 at 23:41
  • -1 base 64 is certainly not the problem here, and the use of strings was explicit in the use case. – Maarten Bodewes Jun 25 '13 at 00:22