Problem
I’m implementing a PAKE-protocol as a university project and one step of the protocol involves sending encrypted data from a key created via a hash function (I’m using SHA-256). I want to use AES/GCM but do not have an IV so I’m using HKDF to expand the key into an IV and key with the following implementation:
import org.bouncycastle.crypto.digests.SHA256Digest;
import org.bouncycastle.crypto.generators.HKDFBytesGenerator;
import org.bouncycastle.crypto.params.HKDFParameters;
import org.bouncycastle.util.Arrays;
import javax.crypto.BadPaddingException;
import javax.crypto.Cipher;
import javax.crypto.IllegalBlockSizeException;
import javax.crypto.NoSuchPaddingException;
import javax.crypto.spec.GCMParameterSpec;
import javax.crypto.spec.SecretKeySpec;
import java.security.InvalidAlgorithmParameterException;
import java.security.InvalidKeyException;
import java.security.NoSuchAlgorithmException;
public class AesGcmSymmetricCipher {
public static final String CIPHER_ALGORITHM = "AES/GCM/NoPadding";
public static final int GCM_TAG_BITS = 128;
public static final int IV_BYTES = 12;
public static final int KEY_BYTES = 32; // AES-256
private final Cipher cipher;
public AesGcmSymmetricCipher() throws NoSuchPaddingException, NoSuchAlgorithmException {
this.cipher = Cipher.getInstance(CIPHER_ALGORITHM);
}
public byte[] encrypt(byte[] key, byte[] clearData) throws InvalidAlgorithmParameterException, InvalidKeyException, BadPaddingException, IllegalBlockSizeException {
var spec = expandKeys(key);
cipher.init(Cipher.ENCRYPT_MODE, spec.getKeySpec(), spec.getParameterSpec());
return cipher.doFinal(clearData);
}
public byte[] decrypt(byte[] key, byte[] cipherData) throws InvalidAlgorithmParameterException, InvalidKeyException, BadPaddingException, IllegalBlockSizeException {
var spec = expandKeys(key);
cipher.init(Cipher.DECRYPT_MODE, spec.getKeySpec(), spec.getParameterSpec());
return cipher.doFinal(cipherData);
}
public String getAlgorithm() {
return "AES-256/GCM/NoPadding using HKDF";
}
private KeyedMaterial expandKeys(byte[] initialKey) {
var iv = new byte[IV_BYTES];
var key = new byte[KEY_BYTES];
var digest = new SHA256Digest();
var kdf = new HKDFBytesGenerator(digest);
var parameters = new HKDFParameters(initialKey, null, null);
kdf.init(parameters);
kdf.generateBytes(iv, 0, IV_BYTES);
kdf.generateBytes(key, 0, KEY_BYTES);
var gcmParameters = new GCMParameterSpec(GCM_TAG_BITS, iv);
var keySpec = new SecretKeySpec(key, CIPHER_ALGORITHM);
Arrays.clear(iv);
Arrays.clear(key);
return new KeyedMaterial(gcmParameters, keySpec);
}
private static class KeyedMaterial {
private final GCMParameterSpec parameterSpec;
private final SecretKeySpec keySpec;
public KeyedMaterial(GCMParameterSpec parameterSpec, SecretKeySpec keySpec) {
this.parameterSpec = parameterSpec;
this.keySpec = keySpec;
}
public GCMParameterSpec getParameterSpec() {
return parameterSpec;
}
public SecretKeySpec getKeySpec() {
return keySpec;
}
}
}
I’m interested in feedback especially from a security perspective, if relevant the initial key passed to encrypt
/decrypt
is ephemeral and will only be used once for encryption and once for decryption.
Solution
First of all, thank you for using my Bouncy Castle additions:
Maarten Bodewes 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.