Merge pull request #187 from ColtonWilley/wp_dh_decode_fixes

Fix DH public key decoding, add new DH decoder registrations

Author: SparkiDev

src/wp_dh_kmgmt.c

@@ -2315,6 +2315,16 @@ static int wp_dh_encode_spki_size(const wp_Dh *dh, size_t* keyLen)
     int ret;
     word32 len;
 
+    /* If we have a generated public key that is not set in the inner key,
+     * set it now */
+    if (mp_bitsused(&dh->key.pub) == 0 && dh->pub != NULL && dh->pubSz > 0) {
+        ret = wc_DhImportKeyPair((DhKey*)&dh->key, NULL, 0,
+            dh->pub, (word32)dh->pubSz);
+        if (ret != 0) {
+            ok = 0;
+        }
+    }
+
     ret = wc_DhPubKeyToDer((DhKey*)&dh->key, NULL, &len);
     if (ret != LENGTH_ONLY_E) {
         ok = 0;
@@ -2374,6 +2384,16 @@ static int wp_dh_encode_pki_size(const wp_Dh *dh, size_t* keyLen)
     int ret;
     word32 len;
 
+    /* If we have a generated private key that is not set in the inner key,
+     * set it now */
+    if (mp_bitsused(&dh->key.priv) == 0 && dh->priv != NULL && dh->privSz > 0) {
+        ret = wc_DhImportKeyPair((DhKey*)&dh->key, dh->priv, (word32)dh->privSz,
+            dh->pub, (word32)dh->pubSz);
+        if (ret != 0) {
+            ok = 0;
+        }
+    }
+
     ret = wc_DhPrivKeyToDer((DhKey*)&dh->key, NULL, &len);
     if (ret != LENGTH_ONLY_E) {
         ok = 0;
@@ -2639,6 +2659,7 @@ static int wp_dh_encode(wp_DhEncDecCtx* ctx, OSSL_CORE_BIO *cBio,
     (void)pwCbArg;
 #endif
 
+    BIO_free(out);
     WOLFPROV_LEAVE(WP_LOG_KE, __FILE__ ":" WOLFPROV_STRINGIZE(__LINE__), ok);
     return ok;
 }

src/wp_wolfprov.c

@@ -977,6 +977,16 @@ static const OSSL_ALGORITHM wolfprov_decoder[] = {
     { WP_NAMES_DH, WP_DECODER_PROPERTIES(type-specific),
       wp_dh_type_specific_decoder_functions,
       "" },
+    /* Add the same decoders for name "DHX" */
+    { WP_NAMES_DHX, WP_DECODER_PROPERTIES(SubjectPublicKeyInfo),
+      wp_dh_spki_decoder_functions,
+      "" },
+    { WP_NAMES_DHX, WP_DECODER_PROPERTIES(PrivateKeyInfo),
+      wp_dh_pki_decoder_functions,
+      "" },
+    { WP_NAMES_DHX, WP_DECODER_PROPERTIES(type-specific),
+      wp_dh_type_specific_decoder_functions,
+      "" },
 #endif
 
 #ifdef WP_HAVE_ECC

test/test_dh.c

@@ -20,6 +20,7 @@
 
 #include "unit.h"
 #include <openssl/core_names.h>
+#include <openssl/decoder.h>
 
 #ifdef WP_HAVE_DH
 
@@ -96,6 +97,76 @@ static const unsigned char dh_g[] =
   0x02,
 };
 
+/* Fixed param from krb5 DH generation, named "o2048" */
+static const uint8_t dh_2048[] = {
+    0x30, 0x82, 0x02, 0x0C, 0x02, 0x82, 0x01, 0x01,
+    0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+    0xFF, 0xC9, 0x0F, 0xDA, 0xA2, 0x21, 0x68, 0xC2,
+    0x34, 0xC4, 0xC6, 0x62, 0x8B, 0x80, 0xDC, 0x1C,
+    0xD1, 0x29, 0x02, 0x4E, 0x08, 0x8A, 0x67, 0xCC,
+    0x74, 0x02, 0x0B, 0xBE, 0xA6, 0x3B, 0x13, 0x9B,
+    0x22, 0x51, 0x4A, 0x08, 0x79, 0x8E, 0x34, 0x04,
+    0xDD, 0xEF, 0x95, 0x19, 0xB3, 0xCD, 0x3A, 0x43,
+    0x1B, 0x30, 0x2B, 0x0A, 0x6D, 0xF2, 0x5F, 0x14,
+    0x37, 0x4F, 0xE1, 0x35, 0x6D, 0x6D, 0x51, 0xC2,
+    0x45, 0xE4, 0x85, 0xB5, 0x76, 0x62, 0x5E, 0x7E,
+    0xC6, 0xF4, 0x4C, 0x42, 0xE9, 0xA6, 0x37, 0xED,
+    0x6B, 0x0B, 0xFF, 0x5C, 0xB6, 0xF4, 0x06, 0xB7,
+    0xED, 0xEE, 0x38, 0x6B, 0xFB, 0x5A, 0x89, 0x9F,
+    0xA5, 0xAE, 0x9F, 0x24, 0x11, 0x7C, 0x4B, 0x1F,
+    0xE6, 0x49, 0x28, 0x66, 0x51, 0xEC, 0xE4, 0x5B,
+    0x3D, 0xC2, 0x00, 0x7C, 0xB8, 0xA1, 0x63, 0xBF,
+    0x05, 0x98, 0xDA, 0x48, 0x36, 0x1C, 0x55, 0xD3,
+    0x9A, 0x69, 0x16, 0x3F, 0xA8, 0xFD, 0x24, 0xCF,
+    0x5F, 0x83, 0x65, 0x5D, 0x23, 0xDC, 0xA3, 0xAD,
+    0x96, 0x1C, 0x62, 0xF3, 0x56, 0x20, 0x85, 0x52,
+    0xBB, 0x9E, 0xD5, 0x29, 0x07, 0x70, 0x96, 0x96,
+    0x6D, 0x67, 0x0C, 0x35, 0x4E, 0x4A, 0xBC, 0x98,
+    0x04, 0xF1, 0x74, 0x6C, 0x08, 0xCA, 0x18, 0x21,
+    0x7C, 0x32, 0x90, 0x5E, 0x46, 0x2E, 0x36, 0xCE,
+    0x3B, 0xE3, 0x9E, 0x77, 0x2C, 0x18, 0x0E, 0x86,
+    0x03, 0x9B, 0x27, 0x83, 0xA2, 0xEC, 0x07, 0xA2,
+    0x8F, 0xB5, 0xC5, 0x5D, 0xF0, 0x6F, 0x4C, 0x52,
+    0xC9, 0xDE, 0x2B, 0xCB, 0xF6, 0x95, 0x58, 0x17,
+    0x18, 0x39, 0x95, 0x49, 0x7C, 0xEA, 0x95, 0x6A,
+    0xE5, 0x15, 0xD2, 0x26, 0x18, 0x98, 0xFA, 0x05,
+    0x10, 0x15, 0x72, 0x8E, 0x5A, 0x8A, 0xAC, 0xAA,
+    0x68, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+    0xFF, 0x02, 0x01, 0x02, 0x02, 0x82, 0x01, 0x00,
+    0x7F, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
+    0xE4, 0x87, 0xED, 0x51, 0x10, 0xB4, 0x61, 0x1A,
+    0x62, 0x63, 0x31, 0x45, 0xC0, 0x6E, 0x0E, 0x68,
+    0x94, 0x81, 0x27, 0x04, 0x45, 0x33, 0xE6, 0x3A,
+    0x01, 0x05, 0xDF, 0x53, 0x1D, 0x89, 0xCD, 0x91,
+    0x28, 0xA5, 0x04, 0x3C, 0xC7, 0x1A, 0x02, 0x6E,
+    0xF7, 0xCA, 0x8C, 0xD9, 0xE6, 0x9D, 0x21, 0x8D,
+    0x98, 0x15, 0x85, 0x36, 0xF9, 0x2F, 0x8A, 0x1B,
+    0xA7, 0xF0, 0x9A, 0xB6, 0xB6, 0xA8, 0xE1, 0x22,
+    0xF2, 0x42, 0xDA, 0xBB, 0x31, 0x2F, 0x3F, 0x63,
+    0x7A, 0x26, 0x21, 0x74, 0xD3, 0x1B, 0xF6, 0xB5,
+    0x85, 0xFF, 0xAE, 0x5B, 0x7A, 0x03, 0x5B, 0xF6,
+    0xF7, 0x1C, 0x35, 0xFD, 0xAD, 0x44, 0xCF, 0xD2,
+    0xD7, 0x4F, 0x92, 0x08, 0xBE, 0x25, 0x8F, 0xF3,
+    0x24, 0x94, 0x33, 0x28, 0xF6, 0x72, 0x2D, 0x9E,
+    0xE1, 0x00, 0x3E, 0x5C, 0x50, 0xB1, 0xDF, 0x82,
+    0xCC, 0x6D, 0x24, 0x1B, 0x0E, 0x2A, 0xE9, 0xCD,
+    0x34, 0x8B, 0x1F, 0xD4, 0x7E, 0x92, 0x67, 0xAF,
+    0xC1, 0xB2, 0xAE, 0x91, 0xEE, 0x51, 0xD6, 0xCB,
+    0x0E, 0x31, 0x79, 0xAB, 0x10, 0x42, 0xA9, 0x5D,
+    0xCF, 0x6A, 0x94, 0x83, 0xB8, 0x4B, 0x4B, 0x36,
+    0xB3, 0x86, 0x1A, 0xA7, 0x25, 0x5E, 0x4C, 0x02,
+    0x78, 0xBA, 0x36, 0x04, 0x65, 0x0C, 0x10, 0xBE,
+    0x19, 0x48, 0x2F, 0x23, 0x17, 0x1B, 0x67, 0x1D,
+    0xF1, 0xCF, 0x3B, 0x96, 0x0C, 0x07, 0x43, 0x01,
+    0xCD, 0x93, 0xC1, 0xD1, 0x76, 0x03, 0xD1, 0x47,
+    0xDA, 0xE2, 0xAE, 0xF8, 0x37, 0xA6, 0x29, 0x64,
+    0xEF, 0x15, 0xE5, 0xFB, 0x4A, 0xAC, 0x0B, 0x8C,
+    0x1C, 0xCA, 0xA4, 0xBE, 0x75, 0x4A, 0xB5, 0x72,
+    0x8A, 0xE9, 0x13, 0x0C, 0x4C, 0x7D, 0x02, 0x88,
+    0x0A, 0xB9, 0x47, 0x2D, 0x45, 0x56, 0x55, 0x34,
+    0x7F, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF
+};
+
 static int test_dh_pkey_keygen(EVP_PKEY *params)
 {
     int err;
@@ -472,5 +543,104 @@ int test_dh_get_params(void *data)
     return err;
 }
 
+static int test_dh_krb5_keygen_ex(OSSL_LIB_CTX *libCtx)
+{
+    int err = 0;
+    EVP_PKEY *params = NULL;
+    EVP_PKEY *key = NULL;
+    EVP_PKEY_CTX *ctx = NULL;
+    OSSL_DECODER_CTX *dctx = NULL;
+    const unsigned char *inptr = dh_2048;
+    size_t inlen = sizeof(dh_2048);
+    unsigned char *spki = NULL;
+    size_t spki_len = 0;
+    unsigned char *der = NULL;
+    size_t der_len;
+
+    PRINT_MSG("Testing DH key generation with krb5 parameters");
+
+    /* Create decoder context for DH parameters */
+    dctx = OSSL_DECODER_CTX_new_for_pkey(&params, "DER", "type-specific", "DHX",
+        EVP_PKEY_KEY_PARAMETERS, libCtx, NULL);
+    err = dctx == NULL;
+    if (err == 0) {
+        /* Decode the parameters */
+        err = OSSL_DECODER_from_data(dctx, &inptr, &inlen) != 1;
+    }
+
+    if (err == 0) {
+        /* Create key generation context */
+        ctx = EVP_PKEY_CTX_new_from_pkey(libCtx, params, NULL);
+        err = ctx == NULL;
+    }
+    if (err == 0) {
+        err = EVP_PKEY_keygen_init(ctx) != 1;
+    }
+    if (err == 0) {
+        /* Generate key pair */
+        err = EVP_PKEY_keygen(ctx, &key) != 1;
+    }
+
+    if (err == 0) {
+        /* Get the size of the encoded public key */
+        err = i2d_PUBKEY(key, NULL) <= 0;
+    }
+    if (err == 0) {
+        der_len = i2d_PUBKEY(key, NULL);
+        der = OPENSSL_malloc(der_len);
+        err = der == NULL;
+    }
+    if (err == 0) {
+        unsigned char *p = der;
+        err = i2d_PUBKEY(key, &p) <= 0;
+    }
+    if (err == 0) {
+        spki = der;
+        der = NULL;
+        spki_len = der_len;
+    }
+    /* We were previously producing an empty subject public key info which was
+     * too short. We are still producing a dhKeyAgreement SPKI instead of a
+     * PKCS3 key, which should be fine for now but means we can't directly
+     * compare outputs with openssl. For now lets just make sure the SPKI
+     * encoding length is reasonable, about 260 for an empty SPKI */
+    if (err == 0 && spki_len < 300) {
+        PRINT_MSG("SPKI is too short");
+        err = 1;
+    }
+
+    if (der) {
+        OPENSSL_free(spki);
+    }
+    if (spki) {
+        OPENSSL_free(spki);
+    }
+    EVP_PKEY_free(key);
+    EVP_PKEY_free(params);
+    EVP_PKEY_CTX_free(ctx);
+    OSSL_DECODER_CTX_free(dctx);
+
+    return err;
+}
+
+/**
+ * Test DH key generation using the krb5 test parameters.
+ *
+ * @param [in] data  Unused.
+ * @return  1 on success.
+ * @return  0 on failure.
+ */
+int test_dh_krb5_keygen(void *data)
+{
+    int err = 0;
+
+    (void)data;
+
+    err = test_dh_krb5_keygen_ex(osslLibCtx);
+    if (err == 0) {
+        err = test_dh_krb5_keygen_ex(wpLibCtx);
+    }
+    return err;
+}
 
 #endif /* WP_HAVE_DH */

test/unit.c

@@ -152,6 +152,7 @@ TEST_CASE test_case[] = {
     TEST_DECL(test_dh_pgen_pkey, NULL),
     TEST_DECL(test_dh_pkey, NULL),
     TEST_DECL(test_dh_decode, NULL),
+    TEST_DECL(test_dh_krb5_keygen, NULL),
 #ifndef WOLFPROV_QUICKTEST
     TEST_DECL(test_dh_get_params, NULL),
 #endif

test/unit.h

@@ -256,6 +256,7 @@ int test_dh_pgen_pkey(void *data);
 int test_dh_pkey(void *data);
 int test_dh_decode(void *data);
 int test_dh_get_params(void *data);
+int test_dh_krb5_keygen(void *data);
 #endif /* WP_HAVE_DH */
 
 #ifdef WP_HAVE_ECC