Merge pull request #376 from padelsbach/wp-kdf-type-strcmp

Fix KDF type string compare

Author: ColtonWilley

src/wp_dh_exch.c

@@ -481,7 +481,7 @@ static int wp_dh_set_param_kdf(wp_DhCtx* ctx, const OSSL_PARAM params[])
         if (kdf[0] == '\0') {
             ctx->kdfType = WP_KDF_NONE;
         }
-        else if (XSTRNCMP(kdf, OSSL_KDF_NAME_X942KDF_ASN1, XSTRLEN(kdf)) == 0) {
+        else if (XSTRCMP(kdf, OSSL_KDF_NAME_X942KDF_ASN1) == 0) {
             /* Only support the non ASN1 variant. */
             ctx->kdfType = WP_KDF_X963;
         }

src/wp_ecdh_exch.c

@@ -438,7 +438,7 @@ static int wp_ecdh_set_param_kdf(wp_EcdhCtx* ctx, const OSSL_PARAM params[])
         if (kdf[0] == '\0') {
             ctx->kdfType = WP_KDF_NONE;
         }
-        else if (XSTRNCMP(kdf, OSSL_KDF_NAME_X942KDF_ASN1, XSTRLEN(kdf)) == 0) {
+        else if (XSTRCMP(kdf, OSSL_KDF_NAME_X942KDF_ASN1) == 0) {
             /* Only support the non ASN1 variant. */
             ctx->kdfType = WP_KDF_X963;
         }

test/test_dh.c

@@ -360,6 +360,54 @@ int test_dh_pkey(void *data)
     return err;
 }
 
+int test_dh_invalid_kdf_strings(void *data)
+{
+    int err = 0;
+    EVP_PKEY_CTX *ctx = NULL;
+    EVP_PKEY *key = NULL;
+    const unsigned char *p = dh_der;
+    char *invalidKdfs[] = {
+        (char *)"X",
+        (char *)"X942",
+        (char *)"X942KDF",
+        (char *)"X942KDF-AS"
+    };
+    size_t i;
+
+    (void)data;
+
+    PRINT_MSG("Reject invalid DH KDF type strings");
+
+    key = d2i_PrivateKey_ex(EVP_PKEY_DH, NULL, &p, sizeof(dh_der), wpLibCtx,
+        NULL);
+    err = key == NULL;
+    if (err == 0) {
+        ctx = EVP_PKEY_CTX_new_from_pkey(wpLibCtx, key, NULL);
+        err = ctx == NULL;
+    }
+    if (err == 0) {
+        err = EVP_PKEY_derive_init(ctx) != 1;
+    }
+    for (i = 0; (err == 0) && (i < (sizeof(invalidKdfs) / sizeof(*invalidKdfs)));
+            i++) {
+        OSSL_PARAM params[2];
+
+        params[0] = OSSL_PARAM_construct_utf8_string(
+            OSSL_EXCHANGE_PARAM_KDF_TYPE, invalidKdfs[i], 0);
+        params[1] = OSSL_PARAM_construct_end();
+
+        err = EVP_PKEY_CTX_set_params(ctx, params) > 0;
+        if (err != 0) {
+            PRINT_ERR_MSG("Accepted invalid DH KDF type: %s", invalidKdfs[i]);
+        }
+    }
+
+    EVP_PKEY_CTX_free(ctx);
+    EVP_PKEY_free(key);
+
+    return err;
+}
+
 int test_dh_decode(void *data)
 {
     int err = 0;

test/test_ecc.c

@@ -851,6 +851,56 @@ static int test_ecdh(const unsigned char *privKey, size_t len,
     return err;
 }
 
+#ifdef WP_HAVE_EC_P256
+int test_ecdh_invalid_kdf_strings(void *data)
+{
+    int err = 0;
+    EVP_PKEY_CTX *ctx = NULL;
+    EVP_PKEY *key = NULL;
+    const unsigned char *p = ecc_key_der_256;
+    char *invalidKdfs[] = {
+        (char *)"X",
+        (char *)"X942",
+        (char *)"X942KDF",
+        (char *)"X942KDF-AS"
+    };
+    size_t i;
+
+    (void)data;
+
+    PRINT_MSG("Reject invalid ECDH KDF type strings");
+
+    key = d2i_PrivateKey_ex(EVP_PKEY_EC, NULL, &p, sizeof(ecc_key_der_256),
+        wpLibCtx, NULL);
+    err = key == NULL;
+    if (err == 0) {
+        ctx = EVP_PKEY_CTX_new_from_pkey(wpLibCtx, key, NULL);
+        err = ctx == NULL;
+    }
+    if (err == 0) {
+        err = EVP_PKEY_derive_init(ctx) != 1;
+    }
+    for (i = 0; (err == 0) && (i < (sizeof(invalidKdfs) / sizeof(*invalidKdfs)));
+            i++) {
+        OSSL_PARAM params[2];
+
+        params[0] = OSSL_PARAM_construct_utf8_string(
+            OSSL_EXCHANGE_PARAM_KDF_TYPE, invalidKdfs[i], 0);
+        params[1] = OSSL_PARAM_construct_end();
+
+        err = EVP_PKEY_CTX_set_params(ctx, params) > 0;
+        if (err != 0) {
+            PRINT_ERR_MSG("Accepted invalid ECDH KDF type: %s", invalidKdfs[i]);
+        }
+    }
+
+    EVP_PKEY_CTX_free(ctx);
+    EVP_PKEY_free(key);
+
+    return err;
+}
+#endif /* WP_HAVE_EC_P256 */
+
 #ifdef WP_HAVE_EC_P192
 int test_ecdh_p192(void *data)
 {

test/unit.c

@@ -285,6 +285,7 @@ TEST_CASE test_case[] = {
 #ifdef WP_HAVE_DH
     TEST_DECL(test_dh_pgen_pkey, NULL),
     TEST_DECL(test_dh_pkey, NULL),
+    TEST_DECL(test_dh_invalid_kdf_strings, NULL),
     TEST_DECL(test_dh_decode, NULL),
     TEST_DECL(test_dh_krb5_keygen, NULL),
 #ifndef WOLFPROV_QUICKTEST
@@ -352,6 +353,7 @@ TEST_CASE test_case[] = {
     #ifdef WP_HAVE_ECKEYGEN
         TEST_DECL(test_ecdh_p256_keygen, NULL),
     #endif
+        TEST_DECL(test_ecdh_invalid_kdf_strings, NULL),
         TEST_DECL(test_ecdh_p256, NULL),
     #endif
     #ifdef WP_HAVE_ECDSA

test/unit.h

@@ -285,6 +285,7 @@ int test_rsa_null_init(void* data);
 #ifdef WP_HAVE_DH
 int test_dh_pgen_pkey(void *data);
 int test_dh_pkey(void *data);
+int test_dh_invalid_kdf_strings(void *data);
 int test_dh_decode(void *data);
 int test_dh_get_params(void *data);
 int test_dh_krb5_keygen(void *data);
@@ -361,6 +362,7 @@ int test_ecdh_p192(void *data);
 int test_ecdh_p224(void *data);
 #endif /* WP_HAVE_EC_P224 */
 #ifdef WP_HAVE_EC_P256
+int test_ecdh_invalid_kdf_strings(void *data);
 int test_ecdh_p256(void *data);
 #endif /* WP_HAVE_EC_P256 */
 #ifdef WP_HAVE_EC_P384