More non-security fixes

pom.xml

@@ -23,7 +23,7 @@
 
     <groupId>com.canonical.openssl</groupId>
     <artifactId>openssl-fips-java</artifactId>
-    <version>0.7.0</version>
+    <version>0.7.1</version>
 
     <properties>
         <maven.compiler.source>17</maven.compiler.source>

src/main/java/com/canonical/openssl/cipher/OpenSSLCipher.java

@@ -30,6 +30,7 @@
 import javax.crypto.spec.IvParameterSpec;
 import javax.crypto.spec.GCMParameterSpec;
 import java.security.InvalidAlgorithmParameterException;
+import java.security.MessageDigest;
 import java.security.SecureRandom;
 import javax.crypto.ShortBufferException;
 import javax.crypto.IllegalBlockSizeException;
@@ -74,6 +75,10 @@ abstract public class OpenSSLCipher extends CipherSpi {
     boolean firstUpdate = true;
     private ClearableBuffer aeadDecryptBuffer;
 
+    // Last (key, IV) latched for AEAD encryption on this instance, used to reject GCM/CCM nonce reuse.
+    private byte[] lastEncKey;
+    private byte[] lastEncIv;
+
     private static final class ClearableBuffer {
         private byte[] buf = new byte[256];
         private int count;
@@ -151,6 +156,9 @@ protected OpenSSLCipher(String nameKeySizeAndMode, String padding) {
         this.mode = parts[2];
         this.padding = padding;
         this.cipherContext = createContext0(nameKeySizeAndMode, padding);
+        if (this.cipherContext == 0) {
+            throw new ProviderException("Failed to create cipher context for " + nameKeySizeAndMode);
+        }
         this.cipherState = new CipherState(this.cipherContext);
         cleanable = cleaner.register(this, cipherState);
     }
@@ -271,10 +279,30 @@ protected void engineInit(int opmode, Key key, AlgorithmParameterSpec params, Se
         if (newKeyBytes == null) {
             throw new InvalidKeyException("Key does not support encoding");
         }
+        boolean encrypting = (opmode == Cipher.ENCRYPT_MODE || opmode == Cipher.WRAP_MODE);
+        boolean isAEAD = isModeGCM() || isModeCCM();
+        // Reject reuse of the same key+IV for AEAD encryption: GCM/CCM nonce reuse is catastrophic.
+        if (encrypting && isAEAD && lastEncIv != null && lastEncKey != null
+                && MessageDigest.isEqual(specIv, lastEncIv)
+                && MessageDigest.isEqual(newKeyBytes, lastEncKey)) {
+            Arrays.fill(newKeyBytes, (byte) 0);
+            throw new InvalidAlgorithmParameterException(
+                "Cannot reuse the same key and IV for " + mode + " encryption (nonce reuse)");
+        }
         resetStateForInit(opmode);
         this.keyBytes = newKeyBytes;
         this.iv = specIv;
         cipherState.setIV(this.iv);
+        if (encrypting && isAEAD) {
+            if (lastEncKey != null) {
+                Arrays.fill(lastEncKey, (byte) 0);
+            }
+            if (lastEncIv != null) {
+                Arrays.fill(lastEncIv, (byte) 0);
+            }
+            lastEncKey = newKeyBytes.clone();
+            lastEncIv = specIv.clone();
+        }
         if (!isModeCCM()) {
             doInit0(null, 0, 0, keyBytes, iv, this.opmode);
             Arrays.fill(keyBytes, (byte)0);

src/main/java/com/canonical/openssl/kdf/OpenSSLPBKDF2.java

@@ -17,6 +17,9 @@
 package com.canonical.openssl.kdf;
 
 import com.canonical.openssl.util.NativeLibraryLoader;
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
+import java.nio.charset.StandardCharsets;
 import java.security.spec.KeySpec;
 import java.util.Arrays;
 import javax.crypto.SecretKey;
@@ -142,10 +145,18 @@ protected SecretKey engineGenerateSecret(KeySpec keyspec) throws InvalidKeySpecE
                     + " (FIPS SP 800-132)");
             }
             int keyLengthBytes = resolveKeyLengthBytes(pbeKeySpec.getKeyLength());
-            PBKDF2SecretKey secretKey = new PBKDF2SecretKey(pbeKeySpec.getPassword(),
+            char[] password = pbeKeySpec.getPassword();
+            PBKDF2SecretKey secretKey = new PBKDF2SecretKey(password,
                                     pbeKeySpec.getSalt(), pbeKeySpec.getIterationCount());
-            byte[] secretBytes = generateSecret0(pbeKeySpec.getPassword(), pbeKeySpec.getSalt(),
+            byte[] passwordBytes = encodePassword(password);
+            byte[] secretBytes;
+            try {
+                secretBytes = generateSecret0(passwordBytes, pbeKeySpec.getSalt(),
                                                 pbeKeySpec.getIterationCount(), keyLengthBytes);
+            } finally {
+                Arrays.fill(passwordBytes, (byte) 0);
+                Arrays.fill(password, '\0');
+            }
             if (secretBytes == null) {
                 throw new InvalidKeySpecException("PBKDF2 derivation failed");
             }
@@ -222,10 +233,18 @@ protected SecretKey engineTranslateKey(SecretKey key) throws InvalidKeyException
                 "Key length " + (keyLengthBytes * 8) + " bits exceeds the maximum supported "
                 + (MAX_KEY_LENGTH_BYTES * 8) + " bits");
         }
-        PBKDF2SecretKey secretKey = new PBKDF2SecretKey(pbeKey.getPassword(), pbeKey.getSalt(),
+        char[] password = pbeKey.getPassword();
+        PBKDF2SecretKey secretKey = new PBKDF2SecretKey(password, pbeKey.getSalt(),
                                                         pbeKey.getIterationCount());
-        byte[] secretBytes = generateSecret0(pbeKey.getPassword(), pbeKey.getSalt(),
+        byte[] passwordBytes = encodePassword(password);
+        byte[] secretBytes;
+        try {
+            secretBytes = generateSecret0(passwordBytes, pbeKey.getSalt(),
                                              pbeKey.getIterationCount(), keyLengthBytes);
+        } finally {
+            Arrays.fill(passwordBytes, (byte) 0);
+            Arrays.fill(password, '\0');
+        }
         if (secretBytes == null) {
             throw new InvalidKeyException("PBKDF2 derivation failed");
         }
@@ -237,6 +256,17 @@ protected SecretKey engineTranslateKey(SecretKey key) throws InvalidKeyException
         return secretKey;
     }
 
-    private native byte[] generateSecret0(char[] password, byte[] salt, int iterationCount, int keyLength);
+    // UTF-8 encode the password for portability
+    private static byte[] encodePassword(char[] password) {
+        ByteBuffer bb = StandardCharsets.UTF_8.encode(CharBuffer.wrap(password));
+        byte[] bytes = new byte[bb.remaining()];
+        bb.get(bytes);
+        if (bb.hasArray()) {
+            Arrays.fill(bb.array(), (byte) 0);
+        }
+        return bytes;
+    }
+
+    private native byte[] generateSecret0(byte[] password, byte[] salt, int iterationCount, int keyLength);
     private static native int getMaxKeyLengthBytes0();
 }

src/main/java/com/canonical/openssl/keyagreement/OpenSSLKeyAgreement.java

@@ -129,6 +129,9 @@ protected void engineInit(Key key, SecureRandom random) throws InvalidKeyExcepti
             cleanable.clean();
         }
         nativeHandle = initialize(key);
+        if (nativeHandle == 0) {
+            throw new InvalidKeyException("Failed to initialize key agreement");
+        }
         cleanable = cleaner.register(this, new KeyAgreementState(nativeHandle));
         state = State.INITIALIZED;
     }

src/main/java/com/canonical/openssl/keyencapsulation/OpenSSLKEMRSA.java

@@ -97,23 +97,27 @@ public RSAKEMEncapsulator(PublicKey key) throws InvalidKeyException {
                 throw new InvalidKeyException("Key does not support encoding");
             }
             nativeHandle = encapsulatorInit0(encoded);
-            cleanable = cleaner.register(this, new EncapsulatorState(nativeHandle));
             if (nativeHandle == 0) {
                 throw new InvalidKeyException("Failed to initialize RSA-KEM encapsulator");
             }
+            cleanable = cleaner.register(this, new EncapsulatorState(nativeHandle));
         }
 
         public KEM.Encapsulated engineEncapsulate(int from, int to, String algorithm) {
-            // TODO: ignoring from, to in the prototype
             int secretSize = engineSecretSize();
+            if (from < 0 || from > to || to > secretSize) {
+                throw new IndexOutOfBoundsException(
+                    "Invalid range [" + from + ", " + to + ") for secret of size " + secretSize);
+            }
             byte[] secretBytes = new byte[secretSize];
 
             int encapsulationSize = engineEncapsulationSize();
             byte[] encapsulatedBytes = new byte[encapsulationSize];
 
             engineEncapsulate0(secretBytes, encapsulatedBytes);
             try {
-                SecretKey secretKey = new SecretKeySpec(secretBytes, algorithm);
+                SecretKey secretKey =
+                    new SecretKeySpec(secretBytes, from, to - from, algorithm);
                 return new KEM.Encapsulated(secretKey, encapsulatedBytes, null);
             } finally {
                 Arrays.fill(secretBytes, (byte)0);
@@ -169,17 +173,24 @@ public RSAKEMDecapsulator(PrivateKey key) throws InvalidKeyException {
             } finally {
                 Arrays.fill(encoded, (byte) 0);
             }
-            cleanable = cleaner.register(this, new DecapsulatorState(nativeHandle));
             if (nativeHandle == 0) {
                 throw new InvalidKeyException("Failed to initialize RSA-KEM decapsulator");
             }
+            cleanable = cleaner.register(this, new DecapsulatorState(nativeHandle));
         }
 
         public SecretKey engineDecapsulate(byte[] encapsulation, int from, int to, String algorithm)
                 throws DecapsulateException {
+            // The secret size is only known once the encapsulation has been
+            // unwrapped, so decapsulate first and then validate the range.
             byte[] secretBytes = engineDecapsulate0(encapsulation);
             try {
-                return new SecretKeySpec(secretBytes, algorithm);
+                if (from < 0 || from > to || to > secretBytes.length) {
+                    throw new IndexOutOfBoundsException(
+                        "Invalid range [" + from + ", " + to + ") for secret of size "
+                            + secretBytes.length);
+                }
+                return new SecretKeySpec(secretBytes, from, to - from, algorithm);
             } finally {
                 Arrays.fill(secretBytes, (byte)0);
             }

src/main/java/com/canonical/openssl/mac/OpenSSLMAC.java

@@ -23,6 +23,7 @@
 import java.nio.ByteBuffer;
 import java.security.InvalidKeyException;
 import java.security.Key;
+import java.security.ProviderException;
 import java.util.Arrays;
 import javax.crypto.MacSpi;
 import java.security.InvalidAlgorithmParameterException;
@@ -122,6 +123,11 @@ protected void engineInit(Key key, AlgorithmParameterSpec spec) throws InvalidKe
         this.keyBytes = newKeyBytes;
         this.cachedIV = iv;
         nativeHandle = doInit0(getAlgorithm(), getCipherType(), getDigestType(), iv, outputLength, keyBytes);
+        if (nativeHandle == 0) {
+            Arrays.fill(keyBytes, (byte) 0);
+            this.keyBytes = null;
+            throw new InvalidKeyException("Failed to initialize MAC");
+        }
         macState = new MACState(nativeHandle);
         macState.setKeyBytes(keyBytes);
         cleanable = cleaner.register(this, macState);
@@ -138,6 +144,9 @@ protected void engineReset() {
             cleanable.clean();
         }
         nativeHandle = doInit0(getAlgorithm(), getCipherType(), getDigestType(), this.cachedIV, this.outputLength, keyBytes);
+        if (nativeHandle == 0) {
+            throw new ProviderException("Failed to reset MAC");
+        }
         macState = new MACState(nativeHandle);
         macState.setKeyBytes(keyBytes);
         cleanable = cleaner.register(this, macState);

src/main/java/com/canonical/openssl/md/OpenSSLMD.java

@@ -24,6 +24,7 @@
 import java.nio.ByteBuffer;
 import java.security.DigestException;
 import java.security.MessageDigestSpi;
+import java.security.ProviderException;
 import java.util.Arrays;
 
 /* This implementation will be exercised by the user through the
@@ -65,6 +66,9 @@ protected OpenSSLMD(String algorithm) {
     private void ensureInitialized() {
         if (!initialized) {
             nativeHandle = doInit0(mdName);
+            if (nativeHandle == 0) {
+                throw new ProviderException("Failed to initialize message digest " + mdName);
+            }
             cleanable = cleaner.register(this, new MDState(nativeHandle));
             initialized = true;
         }
@@ -94,6 +98,9 @@ protected void engineReset() {
             cleanable.clean();
         }
         nativeHandle = doInit0(mdName);
+        if (nativeHandle == 0) {
+            throw new ProviderException("Failed to initialize message digest " + mdName);
+        }
         cleanable = cleaner.register(this, new MDState(nativeHandle));
         initialized = true;
     }

src/main/java/com/canonical/openssl/signature/OpenSSLSignature.java

@@ -137,6 +137,9 @@ protected void engineInitSign(PrivateKey key) throws InvalidKeyException {
                cleanable.clean();
            }
            nativeHandle = engineInitSign0(getSignatureName(), privKey, params);
+           if (nativeHandle == 0) {
+               throw new InvalidKeyException("Failed to initialize signature for signing");
+           }
            cleanable = cleaner.register(this, new SignatureState(nativeHandle));
        } else {
            throw new InvalidKeyException ("Supplied PrivateKey is of type: " + key.getClass());
@@ -159,6 +162,9 @@ protected void engineInitVerify(PublicKey key) throws InvalidKeyException {
                 cleanable.clean();
             }
             nativeHandle = engineInitVerify0(getSignatureName(), pubKey, params);
+            if (nativeHandle == 0) {
+                throw new InvalidKeyException("Failed to initialize signature for verification");
+            }
             cleanable = cleaner.register(this, new SignatureState(nativeHandle));
         } else {
             throw new InvalidKeyException ("Supplied PublicKey is not OpenSSL-based");

src/main/native/c/OpenSSLPBKDF2.c

@@ -23,34 +23,34 @@
 /*
  * Class:     com_canonical_openssl_kdf_OpenSSLPBKDF2
  * Method:    generateSecret0
- * Signature: ([C[BII)[B
+ * Signature: ([B[BII)[B
  */
 JNIEXPORT jbyteArray JNICALL Java_com_canonical_openssl_kdf_OpenSSLPBKDF2_generateSecret0
-  (JNIEnv *env, jobject this, jcharArray password, jbyteArray salt, jint iteration_count, jint key_length) {
+  (JNIEnv *env, jobject this, jbyteArray password, jbyteArray salt, jint iteration_count, jint key_length) {
     if (key_length <= 0 || key_length > MAX_KEY_SIZE) {
         throwProviderException(env, "Invalid PBKDF2 key length");
         return NULL;
     }
 
-    int password_length = (*env)->GetArrayLength(env, password);
+    int password_length = array_length(env, password);
     int salt_length = array_length(env, salt);
     byte output[MAX_KEY_SIZE] = {0};
     jbyteArray result = NULL;
 
-    jchar *password_chars = (*env)->GetCharArrayElements(env, password, NULL);
-    if (password_chars == NULL) {
+    jbyte *password_bytes = (*env)->GetByteArrayElements(env, password, NULL);
+    if (password_bytes == NULL) {
         return NULL;
     }
     jbyte *salt_bytes = (*env)->GetByteArrayElements(env, salt, NULL);
     if (salt_bytes == NULL) {
-        (*env)->ReleaseCharArrayElements(env, password, password_chars, JNI_ABORT);
+        (*env)->ReleaseByteArrayElements(env, password, password_bytes, JNI_ABORT);
         return NULL;
     }
 
-    kdf_spec *spec = create_pbkdf_spec((byte *)password_chars, password_length * sizeof(jchar),
+    kdf_spec *spec = create_pbkdf_spec((byte *)password_bytes, password_length,
                         (byte *)salt_bytes, salt_length, iteration_count);
 
-    (*env)->ReleaseCharArrayElements(env, password, password_chars, JNI_ABORT);
+    (*env)->ReleaseByteArrayElements(env, password, password_bytes, JNI_ABORT);
     (*env)->ReleaseByteArrayElements(env, salt, salt_bytes, JNI_ABORT);
 
     if (spec == NULL) {

src/main/native/c/init.c

@@ -100,6 +100,26 @@ static void unload_global_libctx() {
     unload_libctx(ctx);
 }
 
+/*
+ * Note on OPENSSL_CUSTOM_CONF and the config file:
+ *
+ * When the FIPS provider is not already available by default (i.e. outside the
+ * Ubuntu Pro auto-FIPS setup), we load OpenSSL's configuration from the file
+ * named by the OPENSSL_CUSTOM_CONF environment variable, or from
+ * /usr/local/ssl/openssl.cnf if that variable is not set. That config decides
+ * which provider gets loaded as "fips".
+ *
+ * Both the environment variable and the config file it points to are TRUSTED
+ * inputs. Anyone who can change either of them can choose which OpenSSL provider
+ * module is loaded into this process. We use secure_getenv, so the variable is
+ * ignored when the process is running with elevated privileges (setuid/setgid),
+ * but in every other case the caller is responsible for protecting these inputs.
+ *
+ * In deployments where FIPS compliance is required, make sure the config file
+ * (and the directory containing it) is owned by root and not writable by
+ * untrusted users, so that "fips" cannot be redirected to a non-validated
+ * module.
+ */
 int JNI_OnLoad(JavaVM* vm, void *reserved) {
     const char *default_cnf = "/usr/local/ssl/openssl.cnf";
     const char *custom_cnf = secure_getenv("OPENSSL_CUSTOM_CONF");

src/main/native/include/jni/OpenSSLPBKDF2.h

@@ -26,10 +26,10 @@ extern "C" {
 /*
  * Class:     com_canonical_openssl_kdf_OpenSSLPBKDF2
  * Method:    generateSecret0
- * Signature: ([C[BII)[B
+ * Signature: ([B[BII)[B
  */
 JNIEXPORT jbyteArray JNICALL Java_com_canonical_openssl_kdf_OpenSSLPBKDF2_generateSecret0
-  (JNIEnv *, jobject, jcharArray, jbyteArray, jint, jint);
+  (JNIEnv *, jobject, jbyteArray, jbyteArray, jint, jint);
 
 JNIEXPORT jint JNICALL Java_com_canonical_openssl_kdf_OpenSSLPBKDF2_getMaxKeyLengthBytes0
   (JNIEnv *, jclass);

src/test/consumer-snap/snapcraft.yaml

@@ -51,7 +51,7 @@ parts:
 
 apps:
   kem-test:
-    command: /usr/lib/jvm/java-21-openjdk-amd64/bin/java -cp $SNAP/bin/KEMTest.jar:$SNAP/imported-libs/jar/openssl-fips-java-0.7.0.jar KEMTest
+    command: /usr/lib/jvm/java-21-openjdk-amd64/bin/java -cp $SNAP/bin/KEMTest.jar:$SNAP/imported-libs/jar/openssl-fips-java-0.7.1.jar KEMTest
     environment:
       OPENSSL_MODULES: $SNAP/usr/local/lib64/ossl-modules/
       OPENSSL_CUSTOM_CONF: $SNAP/usr/local/ssl/openssl.cnf