OSSL_CMP_validate_msg(): make sure to reject protection type mismatch

Do not accept password-based if expected signature-based and no secret is available and do not accept signature-based if expected password-based and no trust anchors available. Reviewed-by: Tomas Mraz <tomas@openssl.org> Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: David von Oheimb <david.von.oheimb@siemens.com> (Merged from #19729)
openssl · Dec 8, 2022 · fc93335 · fc93335
1 parent 318a9df
commit fc93335
Show file tree

Hide file tree

Showing 6 changed files with 70 additions and 37 deletions.
diff --git a/crypto/cmp/cmp_err.c b/crypto/cmp/cmp_err.c
@@ -1,6 +1,6 @@
 /*
  * Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 1995-2021 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2022 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -98,8 +98,11 @@ static const ERR_STRING_DATA CMP_str_reasons[] = {
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MISSING_PROTECTION), "missing protection"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MISSING_REFERENCE_CERT),
     "missing reference cert"},
+    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MISSING_SECRET), "missing secret"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MISSING_SENDER_IDENTIFICATION),
     "missing sender identification"},
+    {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MISSING_TRUST_ANCHOR),
+    "missing trust anchor"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MISSING_TRUST_STORE),
     "missing trust store"},
     {ERR_PACK(ERR_LIB_CMP, 0, CMP_R_MULTIPLE_REQUESTS_NOT_SUPPORTED),

diff --git a/crypto/cmp/cmp_vfy.c b/crypto/cmp/cmp_vfy.c
@@ -565,8 +565,9 @@ int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
         /* 5.1.3.1.  Shared Secret Information */
     case NID_id_PasswordBasedMAC:
         if (ctx->secretValue == NULL) {
-            ossl_cmp_warn(ctx, "no secret available for verifying PBM-based CMP message protection");
-            return 1;
+            ossl_cmp_info(ctx, "no secret available for verifying PBM-based CMP message protection");
+            ERR_raise(ERR_LIB_CMP, CMP_R_MISSING_SECRET);
+            return 0;
         }
         if (verify_PBMAC(ctx, msg)) {
             /*
@@ -616,8 +617,9 @@ int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
         scrt = ctx->srvCert;
         if (scrt == NULL) {
             if (ctx->trusted == NULL) {
-                ossl_cmp_warn(ctx, "no trust store nor pinned server cert available for verifying signature-based CMP message protection");
-                return 1;
+                ossl_cmp_info(ctx, "no trust store nor pinned server cert available for verifying signature-based CMP message protection");
+                ERR_raise(ERR_LIB_CMP, CMP_R_MISSING_TRUST_ANCHOR);
+                return 0;
             }
             if (check_msg_find_cert(ctx, msg)) {
                 ossl_cmp_debug(ctx,

diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt
@@ -242,7 +242,9 @@ CMP_R_MISSING_PBM_SECRET:166:missing pbm secret
 CMP_R_MISSING_PRIVATE_KEY:131:missing private key
 CMP_R_MISSING_PROTECTION:143:missing protection
 CMP_R_MISSING_REFERENCE_CERT:168:missing reference cert
+CMP_R_MISSING_SECRET:178:missing secret
 CMP_R_MISSING_SENDER_IDENTIFICATION:111:missing sender identification
+CMP_R_MISSING_TRUST_ANCHOR:179:missing trust anchor
 CMP_R_MISSING_TRUST_STORE:144:missing trust store
 CMP_R_MULTIPLE_REQUESTS_NOT_SUPPORTED:161:multiple requests not supported
 CMP_R_MULTIPLE_RESPONSES_NOT_SUPPORTED:170:multiple responses not supported

diff --git a/doc/man3/OSSL_CMP_validate_msg.pod b/doc/man3/OSSL_CMP_validate_msg.pod
@@ -19,8 +19,11 @@ This is the API for validating the protection of CMP messages,
 which includes validating CMP message sender certificates and their paths
 while optionally checking the revocation status of the certificates(s).
 
-OSSL_CMP_validate_msg() validates the protection of the given I<msg>
-using either password-based mac (PBM) or a signature algorithm.
+OSSL_CMP_validate_msg() validates the protection of the given I<msg>,
+which must be signature-based or using password-based MAC (PBM).
+In the former case a suitable trust anchor must be given in the CMP context
+I<ctx>, and in the latter case the matching secret must have been set there
+using L<OSSL_CMP_CTX_set1_secretValue(3)>.
 
 In case of signature algorithm, the certificate to use for the signature check
 is preferably the one provided by a call to L<OSSL_CMP_CTX_set1_srvCert(3)>.
@@ -61,7 +64,9 @@ return 1 on success, 0 on error or validation failed.
 
 =head1 SEE ALSO
 
-L<OSSL_CMP_CTX_new(3)>, L<OSSL_CMP_exec_certreq(3)>
+L<OSSL_CMP_CTX_new(3)>, L<OSSL_CMP_exec_certreq(3)>,
+L<OSSL_CMP_CTX_set1_secretValue(3)>, L<OSSL_CMP_CTX_set1_srvCert(3)>,
+L<OSSL_CMP_CTX_set1_untrusted(3)>, L<OSSL_CMP_CTX_set0_trusted(3)>
 
 =head1 HISTORY
 

diff --git a/include/openssl/cmperr.h b/include/openssl/cmperr.h
@@ -1,6 +1,6 @@
 /*
  * Generated by util/mkerr.pl DO NOT EDIT
- * Copyright 1995-2021 The OpenSSL Project Authors. All Rights Reserved.
+ * Copyright 1995-2022 The OpenSSL Project Authors. All Rights Reserved.
  *
  * Licensed under the Apache License 2.0 (the "License").  You may not use
  * this file except in compliance with the License.  You can obtain a copy
@@ -69,7 +69,9 @@
 #  define CMP_R_MISSING_PRIVATE_KEY                        131
 #  define CMP_R_MISSING_PROTECTION                         143
 #  define CMP_R_MISSING_REFERENCE_CERT                     168
+#  define CMP_R_MISSING_SECRET                             178
 #  define CMP_R_MISSING_SENDER_IDENTIFICATION              111
+#  define CMP_R_MISSING_TRUST_ANCHOR                       179
 #  define CMP_R_MISSING_TRUST_STORE                        144
 #  define CMP_R_MULTIPLE_REQUESTS_NOT_SUPPORTED            161
 #  define CMP_R_MULTIPLE_RESPONSES_NOT_SUPPORTED           170

diff --git a/test/cmp_vfy_test.c b/test/cmp_vfy_test.c
@@ -83,6 +83,12 @@ static X509 *insta_cert = NULL, *instaca_cert = NULL;
 static unsigned char rand_data[OSSL_CMP_TRANSACTIONID_LENGTH];
 static OSSL_CMP_MSG *ir_unprotected, *ir_rmprotection;
 
+/* secret value used for IP_waitingStatus_PBM.der */
+static const unsigned char sec_1[] = {
+    '9', 'p', 'p', '8', '-', 'b', '3', '5', 'i', '-', 'X', 'd', '3',
+    'Q', '-', 'u', 'd', 'N', 'R'
+};
+
 static int flip_bit(ASN1_BIT_STRING *bitstr)
 {
     int bit_num = 7;
@@ -147,20 +153,15 @@ static int execute_validate_cert_path_test(CMP_VFY_TEST_FIXTURE *fixture)
     return res;
 }
 
-static int test_validate_msg_mac_alg_protection(void)
+static int test_validate_msg_mac_alg_protection(int miss, int wrong)
 {
-    /* secret value belonging to cmp-test/CMP_IP_waitingStatus_PBM.der */
-    const unsigned char sec_1[] = {
-        '9', 'p', 'p', '8', '-', 'b', '3', '5', 'i', '-', 'X', 'd', '3',
-        'Q', '-', 'u', 'd', 'N', 'R'
-    };
-
     SETUP_TEST_FIXTURE(CMP_VFY_TEST_FIXTURE, set_up);
     fixture->cert = NULL;
 
-    fixture->expected = 1;
-    if (!TEST_true(OSSL_CMP_CTX_set1_secretValue(fixture->cmp_ctx, sec_1,
-                                                 sizeof(sec_1)))
+    fixture->expected = !miss && !wrong;
+    if (!TEST_true(miss ? OSSL_CMP_CTX_set0_trusted(fixture->cmp_ctx, NULL)
+                   : OSSL_CMP_CTX_set1_secretValue(fixture->cmp_ctx, sec_1,
+                                                   wrong ? 4 : sizeof(sec_1)))
             || !TEST_ptr(fixture->msg = load_pkimsg(ip_waiting_f, libctx))) {
         tear_down(fixture);
         fixture = NULL;
@@ -169,6 +170,21 @@ static int test_validate_msg_mac_alg_protection(void)
     return result;
 }
 
+static int test_validate_msg_mac_alg_protection_ok(void)
+{
+    return test_validate_msg_mac_alg_protection(0, 0);
+}
+
+static int test_validate_msg_mac_alg_protection_missing(void)
+{
+    return test_validate_msg_mac_alg_protection(1, 0);
+}
+
+static int test_validate_msg_mac_alg_protection_wrong(void)
+{
+    return test_validate_msg_mac_alg_protection(0, 1);
+}
+
 #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
 static int test_validate_msg_mac_alg_protection_bad(void)
 {
@@ -240,44 +256,44 @@ static int test_validate_msg_signature_trusted_expired(void)
 }
 #endif
 
-static int test_validate_msg_signature_srvcert_wrong(void)
+static int test_validate_msg_signature_srvcert(int bad_sig, int miss, int wrong)
 {
     SETUP_TEST_FIXTURE(CMP_VFY_TEST_FIXTURE, set_up);
-    fixture->expected = 0;
+    fixture->cert = srvcert;
+    fixture->expected = !bad_sig && !wrong && !miss;
     if (!TEST_ptr(fixture->msg = load_pkimsg(ir_protected_f, libctx))
-        || !TEST_true(OSSL_CMP_CTX_set1_srvCert(fixture->cmp_ctx, clcert))) {
+        || !TEST_true(miss ? OSSL_CMP_CTX_set1_secretValue(fixture->cmp_ctx,
+                                                           sec_1, sizeof(sec_1))
+                      :  OSSL_CMP_CTX_set1_srvCert(fixture->cmp_ctx,
+                                                   wrong? clcert : srvcert))
+        || (bad_sig && !flip_bit(fixture->msg->protection))) {
         tear_down(fixture);
         fixture = NULL;
     }
     EXECUTE_TEST(execute_validate_msg_test, tear_down);
     return result;
 }
 
-static int test_validate_msg_signature_srvcert(int bad_sig)
+static int test_validate_msg_signature_srvcert_missing(void)
 {
-    SETUP_TEST_FIXTURE(CMP_VFY_TEST_FIXTURE, set_up);
-    fixture->cert = srvcert;
-    fixture->expected = !bad_sig;
-    if (!TEST_ptr(fixture->msg = load_pkimsg(ir_protected_f, libctx))
-        || !TEST_true(OSSL_CMP_CTX_set1_srvCert(fixture->cmp_ctx, srvcert))
-        || (bad_sig && !flip_bit(fixture->msg->protection))) {
-        tear_down(fixture);
-        fixture = NULL;
-    }
-    EXECUTE_TEST(execute_validate_msg_test, tear_down);
-    return result;
+    return test_validate_msg_signature_srvcert(0, 1, 0);
+}
+
+static int test_validate_msg_signature_srvcert_wrong(void)
+{
+    return test_validate_msg_signature_srvcert(0, 0, 1);
 }
 
 #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
 static int test_validate_msg_signature_bad(void)
 {
-    return test_validate_msg_signature_srvcert(1);
+    return test_validate_msg_signature_srvcert(1, 0, 0);
 }
 #endif
 
 static int test_validate_msg_signature_sender_cert_srvcert(void)
 {
-    return test_validate_msg_signature_srvcert(0);
+    return test_validate_msg_signature_srvcert(0, 0, 0);
 }
 
 static int test_validate_msg_signature_sender_cert_untrusted(void)
@@ -650,6 +666,7 @@ int setup_tests(void)
     ADD_TEST(test_validate_msg_signature_trusted_ok);
 #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
     ADD_TEST(test_validate_msg_signature_trusted_expired);
+    ADD_TEST(test_validate_msg_signature_srvcert_missing);
 #endif
     ADD_TEST(test_validate_msg_signature_srvcert_wrong);
 #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
@@ -667,8 +684,10 @@ int setup_tests(void)
 #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
     ADD_TEST(test_validate_msg_unprotected_request);
 #endif
-    ADD_TEST(test_validate_msg_mac_alg_protection);
+    ADD_TEST(test_validate_msg_mac_alg_protection_ok);
 #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+    ADD_TEST(test_validate_msg_mac_alg_protection_missing);
+    ADD_TEST(test_validate_msg_mac_alg_protection_wrong);
     ADD_TEST(test_validate_msg_mac_alg_protection_bad);
 #endif