Correct error reason of verify_signature() in cmp_vfy.c
[openssl.git] / crypto / cmp / cmp_vfy.c
index 437bc32..f73a0a0 100644 (file)
@@ -22,6 +22,8 @@
 #include <openssl/x509.h>
 #include "crypto/x509.h"
 
+DEFINE_STACK_OF(X509)
+
 /*
  * Verify a message protected by signature according to section 5.1.3.3
  * (sha1+RSA/DSA or any other algorithm supported by OpenSSL).
@@ -32,7 +34,7 @@ static int verify_signature(const OSSL_CMP_CTX *cmp_ctx,
                             const OSSL_CMP_MSG *msg, X509 *cert)
 {
     EVP_MD_CTX *ctx = NULL;
-    CMP_PROTECTEDPART prot_part;
+    OSSL_CMP_PROTECTEDPART prot_part;
     int digest_nid, pk_nid;
     const EVP_MD *digest = NULL;
     EVP_PKEY *pubkey = NULL;
@@ -62,7 +64,7 @@ static int verify_signature(const OSSL_CMP_CTX *cmp_ctx,
     prot_part.header = msg->header;
     prot_part.body = msg->body;
 
-    len = i2d_CMP_PROTECTEDPART(&prot_part, &prot_part_der);
+    len = i2d_OSSL_CMP_PROTECTEDPART(&prot_part, &prot_part_der);
     if (len < 0 || prot_part_der == NULL)
         goto end;
     prot_part_der_len = (size_t) len;
@@ -93,7 +95,7 @@ static int verify_signature(const OSSL_CMP_CTX *cmp_ctx,
 
  sig_err:
     res = x509_print_ex_brief(bio, cert, X509_FLAG_NO_EXTENSIONS);
-    CMPerr(0, CMP_R_ERROR_VALIDATING_PROTECTION);
+    CMPerr(0, CMP_R_ERROR_VALIDATING_SIGNATURE);
     if (res)
         ERR_add_error_mem_bio("\n", bio);
     res = 0;
@@ -167,6 +169,8 @@ int OSSL_CMP_validate_cert_path(OSSL_CMP_CTX *ctx, X509_STORE *trusted_store,
         CMPerr(0, CMP_R_POTENTIALLY_INVALID_CERTIFICATE);
 
  err:
+    /* directly output any fresh errors, needed for check_msg_find_cert() */
+    OSSL_CMP_CTX_print_errors(ctx);
     X509_STORE_CTX_free(csc);
     return valid;
 }
@@ -250,17 +254,22 @@ static int cert_acceptable(OSSL_CMP_CTX *ctx,
                            const OSSL_CMP_MSG *msg)
 {
     X509_STORE *ts = ctx->trusted;
-    char *sub, *iss;
+    int self_issued = X509_check_issued(cert, cert) == X509_V_OK;
+    char *str;
     X509_VERIFY_PARAM *vpm = ts != NULL ? X509_STORE_get0_param(ts) : NULL;
     int time_cmp;
 
-    ossl_cmp_log2(INFO, ctx, " considering %s %s with..", desc1, desc2);
-    if ((sub = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0)) != NULL)
-        ossl_cmp_log1(INFO, ctx, "  subject = %s", sub);
-    if ((iss = X509_NAME_oneline(X509_get_issuer_name(cert), NULL, 0)) != NULL)
-        ossl_cmp_log1(INFO, ctx, "  issuer  = %s", iss);
-    OPENSSL_free(iss);
-    OPENSSL_free(sub);
+    ossl_cmp_log3(INFO, ctx, " considering %s%s %s with..",
+                  self_issued ? "self-issued ": "", desc1, desc2);
+    if ((str = X509_NAME_oneline(X509_get_subject_name(cert), NULL, 0)) != NULL)
+        ossl_cmp_log1(INFO, ctx, "  subject = %s", str);
+    OPENSSL_free(str);
+    if (!self_issued) {
+        str = X509_NAME_oneline(X509_get_issuer_name(cert), NULL, 0);
+        if (str != NULL)
+            ossl_cmp_log1(INFO, ctx, "  issuer  = %s", str);
+        OPENSSL_free(str);
+    }
 
     if (already_checked(cert, already_checked1)
             || already_checked(cert, already_checked2)) {
@@ -284,7 +293,7 @@ static int cert_acceptable(OSSL_CMP_CTX *ctx,
     if (!check_kid(ctx, cert, msg->header->senderKID))
         return 0;
     /* acceptable also if there is no senderKID in msg header */
-    ossl_cmp_info(ctx, " cert is acceptable");
+    ossl_cmp_info(ctx, " cert seems acceptable");
     return 1;
 }
 
@@ -295,38 +304,49 @@ static int check_msg_valid_cert(OSSL_CMP_CTX *ctx, X509_STORE *store,
         ossl_cmp_warn(ctx, "msg signature verification failed");
         return 0;
     }
-    if (!OSSL_CMP_validate_cert_path(ctx, store, scrt)) {
-        ossl_cmp_warn(ctx, "cert path validation failed");
-        return 0;
-    }
-    return 1;
+    if (OSSL_CMP_validate_cert_path(ctx, store, scrt))
+        return 1;
+
+    ossl_cmp_warn(ctx,
+                  "msg signature validates but cert path validation failed");
+    return 0;
 }
 
 /*
  * Exceptional handling for 3GPP TS 33.310 [3G/LTE Network Domain Security
- * (NDS); Authentication Framework (AF)], only to use for IP and if the ctx
- * option is explicitly set: use self-issued certificates from extraCerts as
- * trust anchor to validate sender cert and msg -
+ * (NDS); Authentication Framework (AF)], only to use for IP messages
+ * and if the ctx option is explicitly set: use self-issued certificates
+ * from extraCerts as trust anchor to validate sender cert and msg -
  * provided it also can validate the newly enrolled certificate
  */
 static int check_msg_valid_cert_3gpp(OSSL_CMP_CTX *ctx, X509 *scrt,
                                      const OSSL_CMP_MSG *msg)
 {
     int valid = 0;
-    X509_STORE *store = X509_STORE_new();
+    X509_STORE *store;
 
-    if (store != NULL /* store does not include CRLs */
-            && ossl_cmp_X509_STORE_add1_certs(store, msg->extraCerts,
-                                              1 /* self-issued only */))
-        valid = check_msg_valid_cert(ctx, store, scrt, msg);
-    if (valid) {
+    if (!ctx->permitTAInExtraCertsForIR)
+        return 0;
+
+    if ((store = X509_STORE_new()) == NULL
+            || !ossl_cmp_X509_STORE_add1_certs(store, msg->extraCerts,
+                                               1 /* self-issued only */))
+        goto err;
+
+    /* store does not include CRLs */
+    valid = OSSL_CMP_validate_cert_path(ctx, store, scrt);
+    if (!valid) {
+        ossl_cmp_warn(ctx,
+                      "also exceptional 3GPP mode cert path validation failed");
+    } else {
         /*
-         * verify that the newly enrolled certificate (which is assumed to have
-         * rid == 0) can also be validated with the same trusted store
+         * verify that the newly enrolled certificate (which assumed rid ==
+         * OSSL_CMP_CERTREQID) can also be validated with the same trusted store
          */
         EVP_PKEY *privkey = OSSL_CMP_CTX_get0_newPkey(ctx, 1);
         OSSL_CMP_CERTRESPONSE *crep =
-            ossl_cmp_certrepmessage_get0_certresponse(msg->body->value.ip, 0);
+            ossl_cmp_certrepmessage_get0_certresponse(msg->body->value.ip,
+                                                      OSSL_CMP_CERTREQID);
         X509 *newcrt = ossl_cmp_certresponse_get1_certificate(privkey, crep);
         /*
          * maybe better use get_cert_status() from cmp_client.c, which catches
@@ -335,6 +355,8 @@ static int check_msg_valid_cert_3gpp(OSSL_CMP_CTX *ctx, X509 *scrt,
         valid = OSSL_CMP_validate_cert_path(ctx, store, newcrt);
         X509_free(newcrt);
     }
+
+ err:
     X509_STORE_free(store);
     return valid;
 }
@@ -393,8 +415,13 @@ static int check_msg_all_certs(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
 {
     int ret = 0;
 
+    if (mode_3gpp
+            && ((!ctx->permitTAInExtraCertsForIR
+                     || ossl_cmp_msg_get_bodytype(msg) != OSSL_CMP_PKIBODY_IP)))
+        return 0;
+
     ossl_cmp_info(ctx,
-                  mode_3gpp ? "failed; trying now 3GPP mode trusting extraCerts"
+                  mode_3gpp ? "normal mode failed; trying now 3GPP mode trusting extraCerts"
                             : "trying first normal mode using trust store");
     if (check_msg_with_certs(ctx, msg->extraCerts, "extraCerts",
                              NULL, NULL, msg, mode_3gpp))
@@ -418,6 +445,12 @@ static int check_msg_all_certs(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
     return ret;
 }
 
+static int no_log_cb(const char *func, const char *file, int line,
+                     OSSL_CMP_severity level, const char *msg)
+{
+    return 1;
+}
+
 /* verify message signature with any acceptable and valid candidate cert */
 static int check_msg_find_cert(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
 {
@@ -426,7 +459,7 @@ static int check_msg_find_cert(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
     char *sname = NULL;
     char *skid_str = NULL;
     const ASN1_OCTET_STRING *skid = msg->header->senderKID;
-    OSSL_cmp_log_cb_t backup_log_cb = ctx->log_cb;
+    OSSL_CMP_log_cb_t backup_log_cb = ctx->log_cb;
     int res = 0;
 
     if (sender == NULL || msg->body == NULL)
@@ -436,49 +469,52 @@ static int check_msg_find_cert(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
         return 0;
     }
 
+    /* dump any hitherto errors to avoid confusion when printing further ones */
+    OSSL_CMP_CTX_print_errors(ctx);
+
     /*
      * try first cached scrt, used successfully earlier in same transaction,
      * for validating this and any further msgs where extraCerts may be left out
      */
-    (void)ERR_set_mark();
-    if (scrt != NULL
-            && cert_acceptable(ctx, "previously validated", "sender cert", scrt,
-                               NULL, NULL, msg)
-            && (check_msg_valid_cert(ctx, ctx->trusted, scrt, msg)
+    if (scrt != NULL) {
+        (void)ERR_set_mark();
+        ossl_cmp_info(ctx,
+                      "trying to verify msg signature with previously validated cert");
+        if (cert_acceptable(ctx, "previously validated", "sender cert", scrt,
+                            NULL, NULL, msg)
+                && (check_msg_valid_cert(ctx, ctx->trusted, scrt, msg)
                     || check_msg_valid_cert_3gpp(ctx, scrt, msg))) {
+            (void)ERR_pop_to_mark();
+            return 1;
+        }
         (void)ERR_pop_to_mark();
-        return 1;
+        /* cached sender cert has shown to be no more successfully usable */
+        (void)ossl_cmp_ctx_set0_validatedSrvCert(ctx, NULL);
     }
-    (void)ERR_pop_to_mark();
-
-    /* release any cached sender cert that proved no more successfully usable */
-    (void)ossl_cmp_ctx_set0_validatedSrvCert(ctx, NULL);
 
     /* enable clearing irrelevant errors in attempts to validate sender certs */
     (void)ERR_set_mark();
-    ctx->log_cb = NULL; /* temporarily disable logging diagnostic info */
-
-    if (check_msg_all_certs(ctx, msg, 0 /* using ctx->trusted */)
-            || check_msg_all_certs(ctx, msg, 1 /* 3gpp */)) {
-        /* discard any diagnostic info on trying to use certs */
-        ctx->log_cb = backup_log_cb; /* restore any logging */
+    ctx->log_cb = no_log_cb; /* temporarily disable logging */
+    res = check_msg_all_certs(ctx, msg, 0 /* using ctx->trusted */)
+            || check_msg_all_certs(ctx, msg, 1 /* 3gpp */);
+    ctx->log_cb = backup_log_cb;
+    if (res) {
+        /* discard any diagnostic information on trying to use certs */
         (void)ERR_pop_to_mark();
-        res = 1;
         goto end;
     }
     /* failed finding a sender cert that verifies the message signature */
-    ctx->log_cb = backup_log_cb; /* restore any logging */
     (void)ERR_clear_last_mark();
 
     sname = X509_NAME_oneline(sender->d.directoryName, NULL, 0);
     skid_str = skid == NULL ? NULL
                             : OPENSSL_buf2hexstr(skid->data, skid->length);
     if (ctx->log_cb != NULL) {
-        ossl_cmp_info(ctx, "verifying msg signature with valid cert that..");
+        ossl_cmp_info(ctx, "trying to verify msg signature with a valid cert that..");
         if (sname != NULL)
-            ossl_cmp_log1(INFO, ctx, "matches msg sender name = %s", sname);
+            ossl_cmp_log1(INFO, ctx, "matches msg sender    = %s", sname);
         if (skid_str != NULL)
-            ossl_cmp_log1(INFO, ctx, "matches msg senderKID   = %s", skid_str);
+            ossl_cmp_log1(INFO, ctx, "matches msg senderKID = %s", skid_str);
         else
             ossl_cmp_info(ctx, "while msg header does not contain senderKID");
         /* re-do the above checks (just) for adding diagnostic information */
@@ -523,6 +559,7 @@ int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
     int nid = NID_undef, pk_nid = NID_undef;
     const ASN1_OBJECT *algorOID = NULL;
     X509 *scrt;
+    const X509_NAME *expected_sender;
 
     if (ctx == NULL || msg == NULL
             || msg->header == NULL || msg->body == NULL) {
@@ -530,6 +567,25 @@ int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
         return 0;
     }
 
+    /* validate sender name of received msg */
+    if (msg->header->sender->type != GEN_DIRNAME) {
+        CMPerr(0, CMP_R_SENDER_GENERALNAME_TYPE_NOT_SUPPORTED);
+        return 0; /* TODO FR#42: support for more than X509_NAME */
+    }
+    /*
+     * Compare actual sender name of response with expected sender name.
+     * Mitigates risk to accept misused PBM secret
+     * or misused certificate of an unauthorized entity of a trusted hierarchy.
+     */
+    expected_sender = ctx->expected_sender;
+    if (expected_sender == NULL && ctx->srvCert != NULL)
+        expected_sender = X509_get_subject_name(ctx->srvCert);
+    if (!check_name(ctx, "sender DN field",
+                    msg->header->sender->d.directoryName,
+                    "expected sender", expected_sender))
+        return 0;
+    /* Note: if recipient was NULL-DN it could be learned here if needed */
+
     if ((alg = msg->header->protectionAlg) == NULL /* unprotected message */
             || msg->protection == NULL || msg->protection->data == NULL) {
         CMPerr(0, CMP_R_MISSING_PROTECTION);
@@ -543,6 +599,11 @@ int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
     switch (nid) {
         /* 5.1.3.1.  Shared Secret Information */
     case NID_id_PasswordBasedMAC:
+        if (ctx->secretValue == 0) {
+            CMPerr(0, CMP_R_CHECKING_PBM_NO_SECRET_AVAILABLE);
+            break;
+        }
+
         if (verify_PBMAC(msg, ctx->secretValue)) {
             /*
              * RFC 4210, 5.3.2: 'Note that if the PKI Message Protection is
@@ -590,23 +651,6 @@ int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
             CMPerr(0, CMP_R_UNKNOWN_ALGORITHM_ID);
             break;
         }
-        /* validate sender name of received msg */
-        if (msg->header->sender->type != GEN_DIRNAME) {
-            CMPerr(0, CMP_R_SENDER_GENERALNAME_TYPE_NOT_SUPPORTED);
-            break; /* FR#42: support for more than X509_NAME */
-        }
-        /*
-         * Compare actual sender name of response with expected sender name.
-         * Expected name can be set explicitly or the subject of ctx->srvCert.
-         * Mitigates risk to accept misused certificate of an unauthorized
-         * entity of a trusted hierarchy.
-         */
-        if (!check_name(ctx, "sender DN field",
-                        msg->header->sender->d.directoryName,
-                        "expected sender", ctx->expected_sender))
-            break;
-        /* Note: if recipient was NULL-DN it could be learned here if needed */
-
         scrt = ctx->srvCert;
         if (scrt == NULL) {
             if (check_msg_find_cert(ctx, msg))
@@ -633,8 +677,8 @@ int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
  *
  * Ensures that:
  * it has a valid body type
- * its protection is valid or absent (allowed only if callback function is
- *    present and function yields non-zero result using also supplied argument)
+ * its protection is valid (or invalid/absent, but only if a callback function
+ *     is present and yields a positive result using also the supplied argument)
  * its transaction ID matches the previous transaction ID stored in ctx (if any)
  * its recipNonce matches the previous senderNonce stored in the ctx (if any)
  *
@@ -660,33 +704,35 @@ int ossl_cmp_msg_check_received(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
     if (msg->header->protectionAlg != 0) {
         /* detect explicitly permitted exceptions for invalid protection */
         if (!OSSL_CMP_validate_msg(ctx, msg)
-                && (cb == NULL || !(*cb)(ctx, msg, 1, cb_arg))) {
+                && (cb == NULL || (*cb)(ctx, msg, 1, cb_arg) <= 0)) {
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
             CMPerr(0, CMP_R_ERROR_VALIDATING_PROTECTION);
             return -1;
+#endif
         }
     } else {
         /* detect explicitly permitted exceptions for missing protection */
-        if (cb == NULL || !(*cb)(ctx, msg, 0, cb_arg)) {
+        if (cb == NULL || (*cb)(ctx, msg, 0, cb_arg) <= 0) {
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
             CMPerr(0, CMP_R_MISSING_PROTECTION);
             return -1;
+#endif
         }
     }
 
-    /*
-     * Store any provided extraCerts in ctx for future use,
-     * such that they are available to ctx->certConf_cb and
-     * the peer does not need to send them again in the same transaction.
-     * For efficiency, the extraCerts are prepended so they get used first.
-     */
-    if (!ossl_cmp_sk_X509_add1_certs(ctx->untrusted_certs, msg->extraCerts,
-                                     0 /* this allows self-issued certs */,
-                                     1 /* no_dups */, 1 /* prepend */))
-        return -1;
-
     /* check CMP version number in header */
     if (ossl_cmp_hdr_get_pvno(OSSL_CMP_MSG_get0_header(msg)) != OSSL_CMP_PVNO) {
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
         CMPerr(0, CMP_R_UNEXPECTED_PVNO);
         return -1;
+#endif
+    }
+
+    if ((rcvd_type = ossl_cmp_msg_get_bodytype(msg)) < 0) {
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+        CMPerr(0, CMP_R_PKIBODY_ERROR);
+        return -1;
+#endif
     }
 
     /* compare received transactionID with the expected one in previous msg */
@@ -694,8 +740,10 @@ int ossl_cmp_msg_check_received(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
             && (msg->header->transactionID == NULL
                 || ASN1_OCTET_STRING_cmp(ctx->transactionID,
                                          msg->header->transactionID) != 0)) {
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
         CMPerr(0, CMP_R_TRANSACTIONID_UNMATCHED);
         return -1;
+#endif
     }
 
     /* compare received nonce with the one we sent */
@@ -703,8 +751,10 @@ int ossl_cmp_msg_check_received(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
             && (msg->header->recipNonce == NULL
                 || ASN1_OCTET_STRING_cmp(ctx->senderNonce,
                                          msg->header->recipNonce) != 0)) {
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
         CMPerr(0, CMP_R_RECIPNONCE_UNMATCHED);
         return -1;
+#endif
     }
 
     /*
@@ -720,10 +770,17 @@ int ossl_cmp_msg_check_received(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
         && !OSSL_CMP_CTX_set1_transactionID(ctx, msg->header->transactionID))
         return -1;
 
-    if ((rcvd_type = ossl_cmp_msg_get_bodytype(msg)) < 0) {
-        CMPerr(0, CMP_R_PKIBODY_ERROR);
+    /*
+     * Store any provided extraCerts in ctx for future use,
+     * such that they are available to ctx->certConf_cb and
+     * the peer does not need to send them again in the same transaction.
+     * For efficiency, the extraCerts are prepended so they get used first.
+     */
+    if (!ossl_cmp_sk_X509_add1_certs(ctx->untrusted_certs, msg->extraCerts,
+                                     0 /* this allows self-issued certs */,
+                                     1 /* no_dups */, 1 /* prepend */))
         return -1;
-    }
+
     return rcvd_type;
 }
 
@@ -736,19 +793,27 @@ int ossl_cmp_verify_popo(const OSSL_CMP_MSG *msg, int accept_RAVerified)
         {
             X509_REQ *req = msg->body->value.p10cr;
 
-            if (X509_REQ_verify(req, X509_REQ_get0_pubkey(req)) > 0)
-                return 1;
-            CMPerr(0, CMP_R_REQUEST_NOT_ACCEPTED);
-            return 0;
+            if (X509_REQ_verify(req, X509_REQ_get0_pubkey(req)) <= 0) {
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+                CMPerr(0, CMP_R_REQUEST_NOT_ACCEPTED);
+                return 0;
+#endif
+            }
         }
+        break;
     case OSSL_CMP_PKIBODY_IR:
     case OSSL_CMP_PKIBODY_CR:
     case OSSL_CMP_PKIBODY_KUR:
-        return OSSL_CRMF_MSGS_verify_popo(msg->body->value.ir,
-                                          OSSL_CMP_CERTREQID,
-                                          accept_RAVerified);
+        if (!OSSL_CRMF_MSGS_verify_popo(msg->body->value.ir, OSSL_CMP_CERTREQID,
+                                        accept_RAVerified)) {
+#ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
+            return 0;
+#endif
+        }
+        break;
     default:
         CMPerr(0, CMP_R_PKIBODY_ERROR);
         return 0;
     }
+    return 1;
 }