Make CMP server use same protection for response as for request

Also adds ossl_cmp_hdr_get_protection_nid() simplifying cmp_vfy.c

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11998)

diff --git a/crypto/cmp/cmp_hdr.c b/crypto/cmp/cmp_hdr.c
index 38b3bce3f5ddccc791f4580dc86fa37c8ae22073..4b98a598cd92cd6f8b1f65e44564b9853a0c46fa 100644 (file)
--- a/crypto/cmp/cmp_hdr.c
+++ b/crypto/cmp/cmp_hdr.c
@@ -41,6 +41,14 @@ int ossl_cmp_hdr_get_pvno(const OSSL_CMP_PKIHEADER *hdr)
     return (int)pvno;
 }
 
+int ossl_cmp_hdr_get_protection_nid(const OSSL_CMP_PKIHEADER *hdr)
+{
+    if (!ossl_assert(hdr != NULL)
+            || hdr->protectionAlg == NULL)
+        return NID_undef;
+    return OBJ_obj2nid(hdr->protectionAlg->algorithm);
+}
+
 ASN1_OCTET_STRING *OSSL_CMP_HDR_get0_transactionID(const
                                                    OSSL_CMP_PKIHEADER *hdr)
 {

diff --git a/crypto/cmp/cmp_local.h b/crypto/cmp/cmp_local.h
index 13981ce597fb639b5c96c3e2c4b7710f9ac3cddb..0d874ae78507a38ba2b1ba4e945b2c1bd8aa908d 100644 (file)
--- a/crypto/cmp/cmp_local.h
+++ b/crypto/cmp/cmp_local.h
@@ -796,6 +796,7 @@ int ossl_cmp_pkisi_check_pkifailureinfo(const OSSL_CMP_PKISI *si, int index);
 /* from cmp_hdr.c */
 int ossl_cmp_hdr_set_pvno(OSSL_CMP_PKIHEADER *hdr, int pvno);
 int ossl_cmp_hdr_get_pvno(const OSSL_CMP_PKIHEADER *hdr);
+int ossl_cmp_hdr_get_protection_nid(const OSSL_CMP_PKIHEADER *hdr);
 ASN1_OCTET_STRING *ossl_cmp_hdr_get0_senderNonce(const OSSL_CMP_PKIHEADER *hdr);
 int ossl_cmp_general_name_is_NULL_DN(GENERAL_NAME *name);
 int ossl_cmp_hdr_set1_sender(OSSL_CMP_PKIHEADER *hdr, const X509_NAME *nm);

diff --git a/crypto/cmp/cmp_protect.c b/crypto/cmp/cmp_protect.c
index 5d70c174eececb8db59c6482248281acc83f536e..880051d3dd6d94dfccba46336faf76f901f67974 100644 (file)
--- a/crypto/cmp/cmp_protect.c
+++ b/crypto/cmp/cmp_protect.c
@@ -259,10 +259,8 @@ int ossl_cmp_msg_protect(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
             goto err;
         }
 
-        if (msg->header->protectionAlg == NULL)
-            if ((msg->header->protectionAlg = X509_ALGOR_new()) == NULL)
-                goto err;
-
+        if ((msg->header->protectionAlg = X509_ALGOR_new()) == NULL)
+            goto err;
         if (!OBJ_find_sigid_by_algs(&algNID, ctx->digest,
                                     EVP_PKEY_id(ctx->pkey))) {
             CMPerr(0, CMP_R_UNSUPPORTED_KEY_TYPE);

diff --git a/crypto/cmp/cmp_server.c b/crypto/cmp/cmp_server.c
index c2f0e1a11338d94ee84f65b16a14ba0e53dd2a5f..8570885eedf141dd2c733a01b2bb5ebc30ec68ac 100644 (file)
--- a/crypto/cmp/cmp_server.c
+++ b/crypto/cmp/cmp_server.c
@@ -452,8 +452,10 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx,
                                            const OSSL_CMP_MSG *req)
 {
     OSSL_CMP_CTX *ctx;
+    ASN1_OCTET_STRING *backup_secret;
     OSSL_CMP_PKIHEADER *hdr;
     int req_type, rsp_type;
+    int res;
     OSSL_CMP_MSG *rsp = NULL;
 
     if (srv_ctx == NULL || srv_ctx->ctx == NULL
@@ -463,7 +465,12 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx,
         return 0;
     }
     ctx = srv_ctx->ctx;
+    backup_secret = ctx->secretValue;
 
+    /*
+     * Some things need to be done already before validating the message in
+     * order to be able to send an error message as far as needed and possible.
+     */
     if (hdr->sender->type != GEN_DIRNAME) {
         CMPerr(0, CMP_R_SENDER_GENERALNAME_TYPE_NOT_SUPPORTED);
         goto err;
@@ -506,8 +513,12 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx,
         }
     }
 
-    if (!ossl_cmp_msg_check_update(ctx, req, unprotected_exception,
-                                   srv_ctx->acceptUnprotected))
+    res = ossl_cmp_msg_check_update(ctx, req, unprotected_exception,
+                                    srv_ctx->acceptUnprotected);
+    if (ctx->secretValue != NULL && ctx->pkey != NULL
+            && ossl_cmp_hdr_get_protection_nid(hdr) != NID_id_PasswordBasedMAC)
+        ctx->secretValue = NULL; /* use MSG_SIG_ALG when protecting rsp */
+    if (!res)
         goto err;
 
     switch (req_type) {
@@ -573,15 +584,16 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx,
         }
 
         if ((si = OSSL_CMP_STATUSINFO_new(OSSL_CMP_PKISTATUS_rejection,
-                                          fail_info, NULL)) == NULL)
-            return 0;
-        if (err != 0 && (flags & ERR_TXT_STRING) != 0)
-            data = ERR_reason_error_string(err);
-        rsp = ossl_cmp_error_new(srv_ctx->ctx, si,
-                                 err != 0 ? ERR_GET_REASON(err) : -1,
-                                 data, srv_ctx->sendUnprotectedErrors);
-        OSSL_CMP_PKISI_free(si);
+                                          fail_info, NULL)) != NULL) {
+            if (err != 0 && (flags & ERR_TXT_STRING) != 0)
+                data = ERR_reason_error_string(err);
+            rsp = ossl_cmp_error_new(srv_ctx->ctx, si,
+                                     err != 0 ? ERR_GET_REASON(err) : -1,
+                                     data, srv_ctx->sendUnprotectedErrors);
+            OSSL_CMP_PKISI_free(si);
+        }
     }
+    ctx->secretValue = backup_secret;
 
     /* possibly close the transaction */
     rsp_type =

diff --git a/crypto/cmp/cmp_vfy.c b/crypto/cmp/cmp_vfy.c
index 6a25ce0f78c33f71b3f2e5d99dacf71820dfd138..9eccb71d1b2eee11fe56e1df14c99dfc9893b953 100644 (file)
--- a/crypto/cmp/cmp_vfy.c
+++ b/crypto/cmp/cmp_vfy.c
@@ -70,7 +70,7 @@ static int verify_signature(const OSSL_CMP_CTX *cmp_ctx,
     prot_part_der_len = (size_t) len;
 
     /* verify signature of protected part */
-    if (!OBJ_find_sigid_algs(OBJ_obj2nid(msg->header->protectionAlg->algorithm),
+    if (!OBJ_find_sigid_algs(ossl_cmp_hdr_get_protection_nid(msg->header),
                              &digest_nid, &pk_nid)
             || digest_nid == NID_undef || pk_nid == NID_undef
             || (digest = EVP_get_digestbynid(digest_nid)) == NULL) {
@@ -574,9 +574,6 @@ static int check_msg_find_cert(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
  */
 int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
 {
-    X509_ALGOR *alg;
-    int nid = NID_undef, pk_nid = NID_undef;
-    const ASN1_OBJECT *algorOID = NULL;
     X509 *scrt;
     const X509_NAME *expected_sender;
 
@@ -605,17 +602,13 @@ int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
         return 0;
     /* Note: if recipient was NULL-DN it could be learned here if needed */
 
-    if ((alg = msg->header->protectionAlg) == NULL /* unprotected message */
+    if (msg->header->protectionAlg == NULL /* unprotected message */
             || msg->protection == NULL || msg->protection->data == NULL) {
         CMPerr(0, CMP_R_MISSING_PROTECTION);
         return 0;
     }
 
-    /* determine the nid for the used protection algorithm */
-    X509_ALGOR_get0(&algorOID, NULL, NULL, alg);
-    nid = OBJ_obj2nid(algorOID);
-
-    switch (nid) {
+    switch (ossl_cmp_hdr_get_protection_nid(msg->header)) {
         /* 5.1.3.1.  Shared Secret Information */
     case NID_id_PasswordBasedMAC:
         if (ctx->secretValue == 0) {
@@ -665,11 +658,6 @@ int OSSL_CMP_validate_msg(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
          * 5.1.3.3.  Signature
          */
     default:
-        if (!OBJ_find_sigid_algs(OBJ_obj2nid(alg->algorithm), NULL, &pk_nid)
-                || pk_nid == NID_undef) {
-            CMPerr(0, CMP_R_UNKNOWN_ALGORITHM_ID);
-            break;
-        }
         scrt = ctx->srvCert;
         if (scrt == NULL) {
             if (check_msg_find_cert(ctx, msg))
@@ -727,7 +715,7 @@ int ossl_cmp_msg_check_update(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
         return 0;
 
     /* validate message protection */
-    if (msg->header->protectionAlg != 0) {
+    if (msg->header->protectionAlg != NULL) {
         /* detect explicitly permitted exceptions for invalid protection */
         if (!OSSL_CMP_validate_msg(ctx, msg)
                 && (cb == NULL || (*cb)(ctx, msg, 1, cb_arg) <= 0)) {

diff --git a/doc/internal/man3/ossl_cmp_hdr_init.pod b/doc/internal/man3/ossl_cmp_hdr_init.pod
index cf7f5965512ffd93e6193d0ba59948eb9faf9165..0c6405054f3894aae5bf7940364d070676709e3d 100644 (file)
--- a/doc/internal/man3/ossl_cmp_hdr_init.pod
+++ b/doc/internal/man3/ossl_cmp_hdr_init.pod
@@ -4,6 +4,7 @@
 
 ossl_cmp_hdr_set_pvno,
 ossl_cmp_hdr_get_pvno,
+ossl_cmp_hdr_get_protection_nid,
 ossl_cmp_hdr_get0_sendernonce,
 ossl_cmp_general_name_is_NULL_DN,
 ossl_cmp_hdr_set1_sender,
@@ -25,6 +26,7 @@ ossl_cmp_hdr_init
 
   int ossl_cmp_hdr_set_pvno(OSSL_CMP_PKIHEADER *hdr, int pvno);
   int ossl_cmp_hdr_get_pvno(const OSSL_CMP_PKIHEADER *hdr);
+  int ossl_cmp_hdr_get_protection_nid(const OSSL_CMP_PKIHEADER *hdr);
   ASN1_OCTET_STRING
      *ossl_cmp_hdr_get0_sendernonce(const OSSL_CMP_PKIHEADER *hdr);
   int ossl_cmp_general_name_is_NULL_DN(GENERAL_NAME *name);
@@ -52,6 +54,9 @@ ossl_cmp_hdr_set_pvno() sets hdr->pvno to the given B<pvno>.
 
 ossl_cmp_hdr_get_pvno() returns the pvno of the given B<hdr> or -1 on error.
 
+ossl_cmp_hdr_get_protection_nid returns the NID of the protection algorithm
+in B<hdr> or NID_undef on error.
+
 ossl_cmp_hdr_get0_sendernonce() returns the sender nonce of the given PKIHeader.
 
 ossl_cmp_general_name_is_NULL_DN() determines if the given GENERAL_NAME
@@ -110,7 +115,9 @@ CMP is defined in RFC 4210 (and CRMF in RFC 4211).
 
 ossl_cmp_hdr_get_pvno() returns the pvno of the given B<hdr> or -1 on error.
 
-ossl_cmp_hdr_get0_sendernonce() returns the respective nonce.
+ossl_cmp_hdr_get_protection_nid returns the respective NID, NID_undef on error.
+
+ossl_cmp_hdr_get0_sendernonce() returns the respective nonce, or NULL.
 
 ossl_cmp_general_name_is_NULL_DN() returns 1 given a NULL-DN, else 0.