bugfix in ossl_cmp_msg_protect(): set senderKID and extend extraCerts also for unprotected CMP requests

Reviewed-by: Tim Hudson <tjh@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/12822)

1 files changed, 12 insertions, 12 deletions

diff --git a/crypto/cmp/cmp_protect.c b/crypto/cmp/cmp_protect.c
index 2001e359c5..a6a0f9f9e0 100644
--- a/crypto/cmp/cmp_protect.c
+++ b/crypto/cmp/cmp_protect.c
@@ -179,7 +179,7 @@ int ossl_cmp_msg_add_extraCerts(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
                         X509_ADD_FLAG_UP_REF | X509_ADD_FLAG_NO_DUP))
         return 0;
 
-    /* if none was found avoid empty ASN.1 sequence */
+    /* in case extraCerts are empty list avoid empty ASN.1 sequence */
     if (sk_X509_num(msg->extraCerts) == 0) {
         sk_X509_free(msg->extraCerts);
         msg->extraCerts = NULL;
@@ -272,11 +272,11 @@ int ossl_cmp_msg_protect(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
     ASN1_BIT_STRING_free(msg->protection);
     msg->protection = NULL;
 
-    if (ctx->unprotectedSend)
-        return 1;
-
-    /* use PasswordBasedMac according to 5.1.3.1 if secretValue is given */
-    if (ctx->secretValue != NULL) {
+    if (ctx->unprotectedSend) {
+        if (!set_senderKID(ctx, msg, NULL))
+            goto err;
+    } else if (ctx->secretValue != NULL) {
+        /* use PasswordBasedMac according to 5.1.3.1 if secretValue is given */
         if (!set_pbmac_algor(ctx, &msg->header->protectionAlg))
             goto err;
         if (!set_senderKID(ctx, msg, NULL))
@@ -310,11 +310,12 @@ int ossl_cmp_msg_protect(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
         CMPerr(0, CMP_R_MISSING_KEY_INPUT_FOR_CREATING_PROTECTION);
         goto err;
     }
-    if ((msg->protection = ossl_cmp_calc_protection(ctx, msg)) == NULL)
+    if (!ctx->unprotectedSend
+            && ((msg->protection = ossl_cmp_calc_protection(ctx, msg)) == NULL))
         goto err;
 
     /*
-     * If present, add ctx->cert followed by its chain as far as possible.
+     * For signature-based protection add ctx->cert followed by its chain.
      * Finally add any additional certificates from ctx->extraCertsOut;
      * even if not needed to validate the protection
      * the option to do this might be handy for certain use cases.
@@ -327,11 +328,10 @@ int ossl_cmp_msg_protect(OSSL_CMP_CTX *ctx, OSSL_CMP_MSG *msg)
      * to the client it set to NULL-DN. In this case for identification at least
      * the senderKID must be set, where we took the referenceValue as fallback.
      */
-    if (ossl_cmp_general_name_is_NULL_DN(msg->header->sender)
-            && msg->header->senderKID == NULL)
-        CMPerr(0, CMP_R_MISSING_SENDER_IDENTIFICATION);
-    else
+    if (!(ossl_cmp_general_name_is_NULL_DN(msg->header->sender)
+          && msg->header->senderKID == NULL))
         return 1;
+    CMPerr(0, CMP_R_MISSING_SENDER_IDENTIFICATION);
 
  err:
     CMPerr(0, CMP_R_ERROR_PROTECTING_MESSAGE);