Fixes for DH parameter and key pair export, padPKCS7() block size checks, PKCS12 mapping comment

jni/jni_dh.c

@@ -697,8 +697,10 @@ Java_com_wolfssl_wolfcrypt_Dh_wc_1DhGenerateParams(
     DhKey* dh = NULL;
     RNG* rng = NULL;
     byte* p = NULL;
+    byte* q = NULL;
     byte* g = NULL;
     word32 pSz = 0;
+    word32 qSz = 0;
     word32 gSz = 0;
 
     /* Get RNG object */
@@ -739,11 +741,16 @@ Java_com_wolfssl_wolfcrypt_Dh_wc_1DhGenerateParams(
         return NULL;
     }
 
-    /* Get sizes for p and g - use modSz in bytes as buffer size */
-    pSz = (modSz + 7) / 8;  /* modSz is in bits, convert to bytes */
-    gSz = (modSz + 7) / 8;
+    /* Get sizes for p, q and g */
+    ret = wc_DhExportParamsRaw(dh, NULL, &pSz, NULL, &qSz, NULL, &gSz);
+    if (ret != LENGTH_ONLY_E) {
+        wc_FreeDhKey(dh);
+        XFREE(dh, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        throwWolfCryptExceptionFromError(env, ret);
+        return NULL;
+    }
 
-    /* Allocate buffers for p and g */
+    /* Allocate buffers for p, q and g */
     p = (byte*)XMALLOC(pSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
     if (p == NULL) {
         wc_FreeDhKey(dh);
@@ -763,8 +770,20 @@ Java_com_wolfssl_wolfcrypt_Dh_wc_1DhGenerateParams(
     }
     XMEMSET(g, 0, gSz);
 
-    /* Export parameters from DhKey */
-    ret = wc_DhExportParamsRaw(dh, p, &pSz, NULL, NULL, g, &gSz);
+    q = (byte*)XMALLOC(qSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+    if (q == NULL) {
+        XFREE(p, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        XFREE(g, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        wc_FreeDhKey(dh);
+        XFREE(dh, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+        throwOutOfMemoryException(env, "Failed to allocate q buffer");
+        return NULL;
+    }
+    XMEMSET(q, 0, qSz);
+
+    /* Native requires a q buffer, q is not returned */
+    ret = wc_DhExportParamsRaw(dh, p, &pSz, q, &qSz, g, &gSz);
+    XFREE(q, NULL, DYNAMIC_TYPE_TMP_BUFFER);
     if (ret != 0) {
         XFREE(p, NULL, DYNAMIC_TYPE_TMP_BUFFER);
         XFREE(g, NULL, DYNAMIC_TYPE_TMP_BUFFER);
@@ -919,6 +938,7 @@ Java_com_wolfssl_wolfcrypt_Dh_wc_1DhExportKeyPair(
     byte* priv = NULL;
     byte* pub = NULL;
     word32 privSz = 0, pubSz = 0;
+    word32 pSz = 0, qSz = 0, gSz = 0;
     jbyteArray privArray = NULL;
     jbyteArray pubArray = NULL;
 
@@ -933,12 +953,14 @@ Java_com_wolfssl_wolfcrypt_Dh_wc_1DhExportKeyPair(
         return NULL;
     }
 
-    /* Get sizes for private and public keys */
-    ret = wc_DhExportKeyPair(key, NULL, &privSz, NULL, &pubSz);
-    if (ret != LENGTH_ONLY_E && ret != 0) {
+    /* wc_DhExportKeyPair() has no size query, p bounds both priv and pub */
+    ret = wc_DhExportParamsRaw(key, NULL, &pSz, NULL, &qSz, NULL, &gSz);
+    if (ret != LENGTH_ONLY_E) {
         throwWolfCryptExceptionFromError(env, ret);
         return NULL;
     }
+    privSz = pSz;
+    pubSz = pSz;
 
     /* Allocate buffers */
     priv = (byte*)XMALLOC(privSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
@@ -1041,6 +1063,7 @@ Java_com_wolfssl_wolfcrypt_Dh_wc_1DhExportParams(
     byte* p = NULL;
     byte* g = NULL;
     byte* q = NULL;
+    byte qDummy = 0;
     word32 pSz = 0, gSz = 0, qSz = 0;
     jbyteArray pArray = NULL;
     jbyteArray gArray = NULL;
@@ -1058,20 +1081,13 @@ Java_com_wolfssl_wolfcrypt_Dh_wc_1DhExportParams(
         return NULL;
     }
 
-    /* Get sizes - try with q first */
+    /* Get sizes for p, q and g */
     ret = wc_DhExportParamsRaw(key, NULL, &pSz, NULL, &qSz, NULL, &gSz);
-    if (ret != LENGTH_ONLY_E && ret != 0) {
-        /* Try without q */
-        ret = wc_DhExportParamsRaw(key, NULL, &pSz, NULL, NULL, NULL, &gSz);
-        if (ret != LENGTH_ONLY_E && ret != 0) {
-            throwWolfCryptExceptionFromError(env, ret);
-            return NULL;
-        }
-        hasQ = 0;
-    }
-    else {
-        hasQ = (qSz > 0);
+    if (ret != LENGTH_ONLY_E) {
+        throwWolfCryptExceptionFromError(env, ret);
+        return NULL;
     }
+    hasQ = (qSz > 0);
 
     /* Allocate buffers */
     p = (byte*)XMALLOC(pSz, NULL, DYNAMIC_TYPE_TMP_BUFFER);
@@ -1100,8 +1116,9 @@ Java_com_wolfssl_wolfcrypt_Dh_wc_1DhExportParams(
         XMEMSET(q, 0, qSz);
     }
 
-    /* Export parameters */
-    ret = wc_DhExportParamsRaw(key, p, &pSz, q, hasQ ? &qSz : NULL, g, &gSz);
+    /* Native requires a q buffer even when the key has no q */
+    ret = wc_DhExportParamsRaw(key, p, &pSz, hasQ ? q : &qDummy, &qSz,
+        g, &gSz);
 
     if (ret == 0) {
         /* Create byte arrays */

src/main/java/com/wolfssl/provider/jce/WolfCryptProvider.java

@@ -1391,9 +1391,10 @@ private void registerServices() {
 
         /* Fake mapping of PKCS12 to WKS type. Use with caution! This is
          * usually used when FIPS compliance is needed but code cannot be
-         * changed that creates a JKS KeyStore object type. Any files loaded
-         * into this fake JKS KeyStore MUST be of actual type WKS or failures
-         * will happen. Remove service first here in case of refresh. */
+         * changed that creates a PKCS12 KeyStore object type. Any files
+         * loaded into this fake PKCS12 KeyStore MUST be of actual type WKS
+         * or failures will happen. Remove service first here in case of
+         * refresh. */
         remove("KeyStore.PKCS12");
         mapPkcs12ToWks = Security.getProperty("wolfjce.mapPKCS12toWKS");
         if (mapPkcs12ToWks != null && !mapPkcs12ToWks.isEmpty() &&

src/main/java/com/wolfssl/wolfcrypt/BlockCipher.java

@@ -31,6 +31,9 @@
  */
 public abstract class BlockCipher extends NativeStruct {
 
+    /* PKCS#7 stores the pad length in one byte (RFC 5652) */
+    private static final int PKCS7_MAX_BLOCK_SIZE = 255;
+
     private WolfCryptState state = WolfCryptState.UNINITIALIZED;
 
     private int opmode;
@@ -328,13 +331,16 @@ public static synchronized int getPKCS7PadSize(int inputSize,
     /**
      * Pad input data with PKCS#7 padding.
      *
+     * A zero-length input is accepted and results in one full block of
+     * padding.
+     *
      * @param in Input data to be padded
      * @param blockSize Block size of algorithm being used
      *
      * @return Byte array which includes PKCS#7 padding on end
      *
-     * @throws WolfCryptException if input is null, zero length,
-     *         or blockSize is invalid
+     * @throws WolfCryptException if input is null or blockSize is not
+     *         between 1 and 255
      */
     public static synchronized byte[] padPKCS7(byte[] in, int blockSize)
         throws WolfCryptException {
@@ -347,8 +353,9 @@ public static synchronized byte[] padPKCS7(byte[] in, int blockSize)
                 "Input array is null");
         }
 
-        if (blockSize == 0) {
-            throw new WolfCryptException("Block size is 0");
+        if (blockSize <= 0 || blockSize > PKCS7_MAX_BLOCK_SIZE) {
+            throw new WolfCryptException("Block size must be between 1 and " +
+                PKCS7_MAX_BLOCK_SIZE);
         }
 
         padSz = blockSize - (in.length % blockSize);

src/test/java/com/wolfssl/provider/jce/test/WolfCryptAlgorithmParameterGeneratorTest.java

@@ -300,13 +300,13 @@ public void testDHParameterGenerationNonStandardSizes()
                     spec.getP().bitLength() <= size + 8);
             }
             catch (RuntimeException e) {
-                /* Dynamic parameter generation may not be supported for all
-                 * sizes, especially when wolfSSL enforces minimum parameter
-                 * sizes. Prime generation for non-standard sizes may also
-                 * fail on resource-constrained devices like Android. Skip
-                 * if generation fails. */
+                /* Native only accepts sizes other than 1024, 2048 and 3072
+                 * when built with WOLFSSL_NO_DH186. Prime generation for
+                 * non-standard sizes may also fail on resource-constrained
+                 * devices like Android. Skip if generation fails. */
                 if (e.getMessage() != null &&
-                    (e.getMessage().contains("Bad function argument") ||
+                    ((size != 1024 &&
+                      e.getMessage().contains("Bad function argument")) ||
                      e.getMessage().contains("prime generation failure"))) {
                     continue;
                 }

src/test/java/com/wolfssl/wolfcrypt/test/AesTest.java

@@ -587,6 +587,29 @@ public void testPadPKCS7() {
             /* expected */
         }
 
+        /* Test padPKCS7() with negative block size */
+        for (int len : new int[] { 5, 32 }) {
+            try {
+                padded = Aes.padPKCS7(new byte[len], -16);
+                fail("negative block size should throw WolfCryptException");
+            } catch (WolfCryptException e) {
+                /* expected */
+            }
+        }
+
+        /* Test padPKCS7() with block size above PKCS#7 limit of 255 */
+        try {
+            padded = Aes.padPKCS7(new byte[10], 256);
+            fail("block size above 255 should throw WolfCryptException");
+        } catch (WolfCryptException e) {
+            /* expected */
+        }
+
+        /* Test padPKCS7() with largest valid block size */
+        padded = Aes.padPKCS7(new byte[10], 255);
+        assertEquals(255, padded.length);
+        assertEquals(245, padded[254] & 0xff);
+
         /* Test unPadPKCS7() with null input */
         try {
             unpadded = Aes.unPadPKCS7(null, Aes.BLOCK_SIZE);

src/test/java/com/wolfssl/wolfcrypt/test/DhTest.java

@@ -32,6 +32,7 @@
 import org.junit.runner.Description;
 import org.junit.runners.model.Statement;
 
+import java.math.BigInteger;
 import java.util.Arrays;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.Executors;
@@ -234,6 +235,81 @@ public void sharedSecretShouldMatch() {
         assertArrayEquals(sharedSecretA, sharedSecretB);
     }
 
+    @Test
+    public void generateDhParamsShouldReturnUsableParams() {
+        byte[][] params = null;
+
+        try {
+            synchronized (rngLock) {
+                params = Dh.generateDhParams(rng, 2048);
+            }
+        } catch (WolfCryptException e) {
+            if (e.getError() == WolfCryptError.NOT_COMPILED_IN) {
+                Assume.assumeTrue("WOLFSSL_KEY_GEN not compiled in", false);
+            }
+            throw e;
+        }
+
+        assertNotNull(params);
+        assertEquals(2, params.length);
+        assertEquals(2048, new BigInteger(1, params[0]).bitLength());
+
+        Dh alice = new Dh(params[0], params[1]);
+        Dh bob = new Dh(params[0], params[1]);
+        try {
+            synchronized (rngLock) {
+                alice.makeKey(rng);
+                bob.makeKey(rng);
+            }
+            assertArrayEquals(alice.makeSharedSecret(bob),
+                bob.makeSharedSecret(alice));
+        } finally {
+            alice.releaseNativeStruct();
+            bob.releaseNativeStruct();
+        }
+    }
+
+    @Test
+    public void exportParamsShouldReturnPAndGWhenKeyHasNoQ() {
+        byte[] p = Util.h2b(DH_P_2048_HEX);
+        byte[] g = Util.h2b("02");
+
+        Dh dh = new Dh(p, g);
+        try {
+            byte[][] params = dh.exportParams();
+            assertEquals(2, params.length);
+            assertArrayEquals(p, params[0]);
+            assertArrayEquals(g, params[1]);
+        } finally {
+            dh.releaseNativeStruct();
+        }
+    }
+
+    @Test
+    public void exportKeyPairShouldReturnImportedKeyPair() {
+        Assume.assumeTrue("WOLFSSL_DH_EXTRA required",
+            FeatureDetect.DhExtraEnabled());
+
+        BigInteger p = new BigInteger(DH_P_2048_HEX, 16);
+        BigInteger priv = new BigInteger(
+            "5A1C3E7F92B4D6A80C2E4F6183A5C7E91B3D5F7092B4D6F8193B5D7F91A3C5E7",
+            16);
+        BigInteger pub = BigInteger.valueOf(2).modPow(priv, p);
+
+        Dh dh = new Dh();
+        try {
+            dh.importKeyPair(priv.toByteArray(), pub.toByteArray(),
+                Util.h2b(DH_P_2048_HEX), Util.h2b("02"));
+
+            byte[][] keyPair = dh.exportKeyPair();
+            assertEquals(2, keyPair.length);
+            assertEquals(priv, new BigInteger(1, keyPair[0]));
+            assertEquals(pub, new BigInteger(1, keyPair[1]));
+        } finally {
+            dh.releaseNativeStruct();
+        }
+    }
+
     /**
      * checkPublicKey() must reject an out of range public key value. This
      * is the same validation wc_DhAgree applies to a peer public key, so a