Meaningless Drivel is fun!*
The moose likes Java in General and the fly likes AES Encryption Service Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login


Win a copy of Spring in Action this week in the Spring forum!
JavaRanch » Java Forums » Java » Java in General
Bookmark "AES Encryption Service" Watch "AES Encryption Service" New topic
Author

AES Encryption Service

David Sawyer
Greenhorn

Joined: May 19, 2010
Posts: 14
I'm working on an AES Encryption service. I was wondering in you wouldn't mind taking a look at the design and look for any security holes:

James Sabre
Ranch Hand

Joined: Sep 07, 2004
Posts: 781



1) Why are you using PBE rather than loading the key from some form of key store? It is certainly insecure to hold the password in the source code.
2) Any exception in either encryption or decryption is fatal so you cannot continue. Either the exception must be passed back to the caller or you should wrap the exception in an Error or your own exception and throw that.
3) In the decryption, if the split() does not create three sections then you cannot continue so you should throw an exception. I would follow the same approach as in (2).
4) Since you use utf-8 when converting the cleartext String to bytes you should use utf-8 when converting the decrypted bytes to a String.


Retired horse trader.
 Note: double-underline links may be advertisements automatically added by this site and are probably not endorsed by me.
David Sawyer
Greenhorn

Joined: May 19, 2010
Posts: 14
I'll definitely be moving the password to a keystore as we need different keys for each environment (dev, staging, production). This was more illustrative of what i was doing in one chunk of code. Mostly I wanted to confirm that there were no serious flaws in the encryption/decryption routine.

I will definitely take your advice and add throws rather than catching those exceptions.
David Sawyer
Greenhorn

Joined: May 19, 2010
Posts: 14
Here's the updated version with the throws suggestion implemented and the character encoding fixed:

James Sabre
Ranch Hand

Joined: Sep 07, 2004
Posts: 781

I still don't like the fact that you are using PBE. If you are going to use some form of key store then it is not needed.

The users of your methods don't care about the exceptions that are thrown. What can they do if they get a NoSuchAlgorithmException or any of the other exceptions you throw? Nothing since they are ALL fatal and would represent a system error. In your position I would catch any exception and wrap it in an exception I derived from RuntimeException and throw that.
David Sawyer
Greenhorn

Joined: May 19, 2010
Posts: 14
I understand that from the perspective of not checking in the key etc. that a keystore is more secure, but is there a technical security issue with PBE?

James Sabre
Ranch Hand

Joined: Sep 07, 2004
Posts: 781

David Sawyer wrote:I understand that from the perspective of not checking in the key etc. that a keystore is more secure, but is there a technical security issue with PBE?


PBE is meant for use where a user provides the password that only he knows. Your code does not allow a user to do do this.

I still don't like your exception handling since you are not actually wrapping the exception; you just wrap the message. My encryption exception is


and I would use it as


so that all the information available in the wrapped exception is retained.

P.S. There is no point in generating a 256 bit key from your password if the entropy of your password is less than 256 bits and almost nobody will be able to remember a 256 bit (32 byte) password or type it in without error. Note, to get an entropy of 256 requires about 42 ASCII printable characters.
David Sawyer
Greenhorn

Joined: May 19, 2010
Posts: 14
James, thank you so much for the time. Here's what I've come up with:

First a script to generate the keystore/key:


Next the revised AESEncryptionException:


The AESEncryptionService Interface:


The AESEncryptionServiceImpl:


And Unit Test to make sure it all works:

James Sabre
Ranch Hand

Joined: Sep 07, 2004
Posts: 781

Much much better David. The only significant change I would make is that I would pass the key (as a SecretKey object) into the AESEncryptionServiceImpl when it was constructed so that the encryption methods were isolated from the key storage method (separation of concerns). In your code there are some 'tweeks' I would make

1) I would make the getKeyFromStore() method return a SecretKey (separation of concerns again) then the code
would not be needed. When you generate the key store you specified the key as being AES so you would not need to specify or convert it to AES when you read it from the key store.
2) In your decryption method I would either catch the AESEncryptionException before the Exception and then just re-throw it since there is no point in re-wrapping the wrapped exception OR I would put the exception handling code only round the actual decryption code and not to include the code that is involved in splitting the IV from the ciphertext.
3) You don't handle the exceptions in generateKeyStore() and getKeyFromStore() in the same way as you do elsewhere.

I like the fact that you have test code.
David Sawyer
Greenhorn

Joined: May 19, 2010
Posts: 14
Nice. I like that much better. Here's the current setup:

Interface:


Impl:


Factory:


Script to generate keystore/key:


Unit Test:
Rob Spoor
Sheriff

Joined: Oct 27, 2005
Posts: 19719
    
  20

David, I cut up the throws-clauses of your encrypt and decrypt messages in your second post a bit so that the general layout of this thread isn't so messed up anymore. Can you watch it a bit next time? Thanks.


SCJP 1.4 - SCJP 6 - SCWCD 5 - OCEEJBD 6
How To Ask Questions How To Answer Questions
David Sawyer
Greenhorn

Joined: May 19, 2010
Posts: 14
Rob Prime wrote:Can you watch it a bit next time? Thanks.
Definitely. Will do.
David Sawyer
Greenhorn

Joined: May 19, 2010
Posts: 14
Did anyone else have any security concerns with this final implementation or does this seem secure to everyone?
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: AES Encryption Service