Skip to main content
added 125 characters in body
Source Link

First of all, thank you for using my Bouncy Castle additions.:

Maarten Bodewes [email protected] initial implementation of HKDF and NIST SP 800-108 MAC based KDF functions.

Design review:

You are trying to emulate Java's Cipher class. However, I don't think this is such a good idea if your encrypt methods and is so much different. The Cipher class in Java is reusable for the same key (and, unfortunately, same IV) and this one is clearly not. I would not try this hard. Better make this class more specific to the use case and avoid generic wrapper classes.

In itself, you may wonder if combining the key derivation and cipher is a good idea. You're removing the update functionality, for instance. This is less of a problem if this is for a specific use case.

Code review:

public class AesGcmSymmetricCipher {

Better made final.

The name doesn't fully capture what the class is about. Symmetric is already captured by Aes. The key derivation part is missing on the other hand.

public static final int KEY_BYTES = 32; // AES-256

I generally use 256 / Byte.SIZE so it is clear where the 32 comes from, that way you don't need the comment.

public byte[] encrypt(byte[] key, byte[] clearData) throws InvalidAlgorithmParameterException, InvalidKeyException, BadPaddingException, IllegalBlockSizeException {

clearData is generally called plaintext or simply message within the crypto community.

Other users will probably wonder what happened with the IV or nonce and where to insert it.

Arrays.clear(iv);
Arrays.clear(key);

Beware that Oracle made a huge mistakes by making the actual keys in software impossible to destroy. You can do this and fool yourself. It's great that you thought of this though - you're not in the wrong here.

return "AES-256/GCM/NoPadding using HKDF";

Nice but it doesn't capture all the little details - how is the IV calculated, for instance - so it doesn't seem to be of much use. You cannot specify it to anybody and they will know how to implement it.


Otherwise the class and design seems spot on to me, so well done.

First of all, thank you for using my Bouncy Castle additions.

Design review:

You are trying to emulate Java's Cipher class. However, I don't think this is such a good idea if your encrypt methods and is so much different. The Cipher class in Java is reusable for the same key (and, unfortunately, same IV) and this one is clearly not. I would not try this hard. Better make this class more specific to the use case and avoid generic wrapper classes.

In itself, you may wonder if combining the key derivation and cipher is a good idea. You're removing the update functionality, for instance. This is less of a problem if this is for a specific use case.

Code review:

public class AesGcmSymmetricCipher {

Better made final.

The name doesn't fully capture what the class is about. Symmetric is already captured by Aes. The key derivation part is missing on the other hand.

public static final int KEY_BYTES = 32; // AES-256

I generally use 256 / Byte.SIZE so it is clear where the 32 comes from, that way you don't need the comment.

public byte[] encrypt(byte[] key, byte[] clearData) throws InvalidAlgorithmParameterException, InvalidKeyException, BadPaddingException, IllegalBlockSizeException {

clearData is generally called plaintext or simply message within the crypto community.

Other users will probably wonder what happened with the IV or nonce and where to insert it.

Arrays.clear(iv);
Arrays.clear(key);

Beware that Oracle made a huge mistakes by making the actual keys in software impossible to destroy. You can do this and fool yourself. It's great that you thought of this though - you're not in the wrong here.

return "AES-256/GCM/NoPadding using HKDF";

Nice but it doesn't capture all the little details - how is the IV calculated, for instance - so it doesn't seem to be of much use. You cannot specify it to anybody and they will know how to implement it.


Otherwise the class and design seems spot on to me, so well done.

First of all, thank you for using my Bouncy Castle additions:

Maarten Bodewes [email protected] initial implementation of HKDF and NIST SP 800-108 MAC based KDF functions.

Design review:

You are trying to emulate Java's Cipher class. However, I don't think this is such a good idea if your encrypt methods and is so much different. The Cipher class in Java is reusable for the same key (and, unfortunately, same IV) and this one is clearly not. I would not try this hard. Better make this class more specific to the use case and avoid generic wrapper classes.

In itself, you may wonder if combining the key derivation and cipher is a good idea. You're removing the update functionality, for instance. This is less of a problem if this is for a specific use case.

Code review:

public class AesGcmSymmetricCipher {

Better made final.

The name doesn't fully capture what the class is about. Symmetric is already captured by Aes. The key derivation part is missing on the other hand.

public static final int KEY_BYTES = 32; // AES-256

I generally use 256 / Byte.SIZE so it is clear where the 32 comes from, that way you don't need the comment.

public byte[] encrypt(byte[] key, byte[] clearData) throws InvalidAlgorithmParameterException, InvalidKeyException, BadPaddingException, IllegalBlockSizeException {

clearData is generally called plaintext or simply message within the crypto community.

Other users will probably wonder what happened with the IV or nonce and where to insert it.

Arrays.clear(iv);
Arrays.clear(key);

Beware that Oracle made a huge mistakes by making the actual keys in software impossible to destroy. You can do this and fool yourself. It's great that you thought of this though - you're not in the wrong here.

return "AES-256/GCM/NoPadding using HKDF";

Nice but it doesn't capture all the little details - how is the IV calculated, for instance - so it doesn't seem to be of much use. You cannot specify it to anybody and they will know how to implement it.


Otherwise the class and design seems spot on to me, so well done.

added 334 characters in body
Source Link

First of all, thank you for using my Bouncy Castle additions.

Design review:

You are trying to emulate Java's Cipher class. However, I don't think this is such a good idea if your encrypt methods and is so much different. The Cipher class in Java is reusable for the same key (and, unfortunately, same IV) and this one is clearly not. I would not try this hard. Better make this class more specific to the use case and avoid generic wrapper classes.

In itself, you may wonder if combining the key derivation and cipher is a good idea. You're removing the update functionality, for instance. This is less of a problem if this is for a specific use case.

Code review:

public class AesGcmSymmetricCipher {

Better made final.

AesGcmSymmetricCipher

ThisThe name doesn't fully capture what the class is about. Symmetric is already captured by Aes. The key derivation part is missing on the other hand.

public static final int KEY_BYTES = 32; // AES-256

I generally use 256 / Byte.SIZE so it is clear where the 32 comes from, that way you don't need the comment.

public byte[] encrypt(byte[] key, byte[] clearData) throws InvalidAlgorithmParameterException, InvalidKeyException, BadPaddingException, IllegalBlockSizeException {

clearData is generally called plaintext or simply message within the crypto community.

Other users will probably wonder what happened with the IV or nonce and where to insert it.

Arrays.clear(iv);
Arrays.clear(key);

Beware that Oracle made a huge mistakes by making the actual keys in software impossible to destroy. You can do this and fool yourself. It's great that you thought of this though - you're not in the wrong here.

return "AES-256/GCM/NoPadding using HKDF";

Nice but it doesn't capture all the little details - how is the IV calculated, for instance - so it doesn't seem to be of much use. You cannot specify it to anybody and they will know how to implement it.


Otherwise the class and design seems spot on to me, so well done.

First of all, thank you for using my Bouncy Castle additions.

Design review:

You are trying to emulate Java's Cipher class. However, I don't think this is such a good idea if your encrypt methods and is so much different. The Cipher class in Java is reusable for the same key (and, unfortunately, same IV) and this one is clearly not. I would not try this hard. Better make this class more specific to the use case and avoid generic wrapper classes.

Code review:

public class AesGcmSymmetricCipher {

Better made final.

AesGcmSymmetricCipher

This doesn't fully capture what the class is about. Symmetric is already captured by Aes.

public static final int KEY_BYTES = 32; // AES-256

I generally use 256 / Byte.SIZE so it is clear where the 32 comes from, that way you don't need the comment.

public byte[] encrypt(byte[] key, byte[] clearData) throws InvalidAlgorithmParameterException, InvalidKeyException, BadPaddingException, IllegalBlockSizeException {

clearData is generally called plaintext or simply message within the crypto community.

Arrays.clear(iv);
Arrays.clear(key);

Beware that Oracle made a huge mistakes by making the actual keys in software impossible to destroy. You can do this and fool yourself. It's great that you thought of this though - you're not in the wrong here.

return "AES-256/GCM/NoPadding using HKDF";

Nice but it doesn't capture all the little details - how is the IV calculated, for instance - so it doesn't seem to be of much use. You cannot specify it to anybody and they will know how to implement it.


Otherwise the class and design seems spot on to me, so well done.

First of all, thank you for using my Bouncy Castle additions.

Design review:

You are trying to emulate Java's Cipher class. However, I don't think this is such a good idea if your encrypt methods and is so much different. The Cipher class in Java is reusable for the same key (and, unfortunately, same IV) and this one is clearly not. I would not try this hard. Better make this class more specific to the use case and avoid generic wrapper classes.

In itself, you may wonder if combining the key derivation and cipher is a good idea. You're removing the update functionality, for instance. This is less of a problem if this is for a specific use case.

Code review:

public class AesGcmSymmetricCipher {

Better made final.

The name doesn't fully capture what the class is about. Symmetric is already captured by Aes. The key derivation part is missing on the other hand.

public static final int KEY_BYTES = 32; // AES-256

I generally use 256 / Byte.SIZE so it is clear where the 32 comes from, that way you don't need the comment.

public byte[] encrypt(byte[] key, byte[] clearData) throws InvalidAlgorithmParameterException, InvalidKeyException, BadPaddingException, IllegalBlockSizeException {

clearData is generally called plaintext or simply message within the crypto community.

Other users will probably wonder what happened with the IV or nonce and where to insert it.

Arrays.clear(iv);
Arrays.clear(key);

Beware that Oracle made a huge mistakes by making the actual keys in software impossible to destroy. You can do this and fool yourself. It's great that you thought of this though - you're not in the wrong here.

return "AES-256/GCM/NoPadding using HKDF";

Nice but it doesn't capture all the little details - how is the IV calculated, for instance - so it doesn't seem to be of much use. You cannot specify it to anybody and they will know how to implement it.


Otherwise the class and design seems spot on to me, so well done.

Source Link

First of all, thank you for using my Bouncy Castle additions.

Design review:

You are trying to emulate Java's Cipher class. However, I don't think this is such a good idea if your encrypt methods and is so much different. The Cipher class in Java is reusable for the same key (and, unfortunately, same IV) and this one is clearly not. I would not try this hard. Better make this class more specific to the use case and avoid generic wrapper classes.

Code review:

public class AesGcmSymmetricCipher {

Better made final.

AesGcmSymmetricCipher

This doesn't fully capture what the class is about. Symmetric is already captured by Aes.

public static final int KEY_BYTES = 32; // AES-256

I generally use 256 / Byte.SIZE so it is clear where the 32 comes from, that way you don't need the comment.

public byte[] encrypt(byte[] key, byte[] clearData) throws InvalidAlgorithmParameterException, InvalidKeyException, BadPaddingException, IllegalBlockSizeException {

clearData is generally called plaintext or simply message within the crypto community.

Arrays.clear(iv);
Arrays.clear(key);

Beware that Oracle made a huge mistakes by making the actual keys in software impossible to destroy. You can do this and fool yourself. It's great that you thought of this though - you're not in the wrong here.

return "AES-256/GCM/NoPadding using HKDF";

Nice but it doesn't capture all the little details - how is the IV calculated, for instance - so it doesn't seem to be of much use. You cannot specify it to anybody and they will know how to implement it.


Otherwise the class and design seems spot on to me, so well done.