From: Dr. David von Oheimb <David.von.Oheimb@siemens.com>
Date: Tue, 7 Dec 2021 12:11:27 +0000 (+0100)
Subject: CMP check_msg_find_cert(): improve diagnostics on transactionID mismatch
X-Git-Tag: openssl-3.2.0-alpha1~3239
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a3ea35c2936acbe6a53b1d52d2d7addbfb6bbd5a;p=thirdparty%2Fopenssl.git

CMP check_msg_find_cert(): improve diagnostics on transactionID mismatch

On this occasion, make use of i2s_ASN1_OCTET_STRING() wherever possible

Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/17224)
---

diff --git a/crypto/cmp/cmp_hdr.c b/crypto/cmp/cmp_hdr.c
index 8c553af61a5..e970e6cbd70 100644
--- a/crypto/cmp/cmp_hdr.c
+++ b/crypto/cmp/cmp_hdr.c
@@ -276,8 +276,7 @@ int ossl_cmp_hdr_set_transactionID(OSSL_CMP_CTX *ctx, OSSL_CMP_PKIHEADER *hdr)
         if (!set_random(&ctx->transactionID, ctx,
                         OSSL_CMP_TRANSACTIONID_LENGTH))
             return 0;
-        tid = OPENSSL_buf2hexstr(ctx->transactionID->data,
-                                 ctx->transactionID->length);
+        tid = i2s_ASN1_OCTET_STRING(NULL, ctx->transactionID);
         if (tid != NULL)
             ossl_cmp_log1(DEBUG, ctx,
                           "Starting new transaction with ID=%s", tid);
diff --git a/crypto/cmp/cmp_server.c b/crypto/cmp/cmp_server.c
index 7ce4662aee5..c32737d0e3b 100644
--- a/crypto/cmp/cmp_server.c
+++ b/crypto/cmp/cmp_server.c
@@ -481,10 +481,8 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx,
     case OSSL_CMP_PKIBODY_GENM:
     case OSSL_CMP_PKIBODY_ERROR:
         if (ctx->transactionID != NULL) {
-            char *tid;
+            char *tid = i2s_ASN1_OCTET_STRING(NULL, ctx->transactionID);
 
-            tid = OPENSSL_buf2hexstr(ctx->transactionID->data,
-                                     ctx->transactionID->length);
             if (tid != NULL)
                 ossl_cmp_log1(WARN, ctx,
                               "Assuming that last transaction with ID=%s got aborted",
diff --git a/crypto/cmp/cmp_vfy.c b/crypto/cmp/cmp_vfy.c
index b9d6fc2bdd1..d3d9cca0d41 100644
--- a/crypto/cmp/cmp_vfy.c
+++ b/crypto/cmp/cmp_vfy.c
@@ -186,7 +186,7 @@ static int check_kid(const OSSL_CMP_CTX *ctx,
         ossl_cmp_warn(ctx, "missing Subject Key Identifier in certificate");
         return 0;
     }
-    str = OPENSSL_buf2hexstr(ckid->data, ckid->length);
+    str = i2s_ASN1_OCTET_STRING(NULL, ckid);
     if (ASN1_OCTET_STRING_cmp(ckid, skid) == 0) {
         if (str != NULL)
             ossl_cmp_log1(INFO, ctx, " subjectKID matches senderKID: %s", str);
@@ -197,7 +197,7 @@ static int check_kid(const OSSL_CMP_CTX *ctx,
     if (str != NULL)
         ossl_cmp_log1(INFO, ctx, " cert Subject Key Identifier = %s", str);
     OPENSSL_free(str);
-    if ((str = OPENSSL_buf2hexstr(skid->data, skid->length)) != NULL)
+    if ((str = i2s_ASN1_OCTET_STRING(NULL, skid)) != NULL)
         ossl_cmp_log1(INFO, ctx, " does not match senderKID    = %s", str);
     OPENSSL_free(str);
     return 0;
@@ -500,8 +500,7 @@ static int check_msg_find_cert(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg)
     (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);
+    skid_str = skid == NULL ? NULL : i2s_ASN1_OCTET_STRING(NULL, skid);
     if (ctx->log_cb != NULL) {
         ossl_cmp_info(ctx, "trying to verify msg signature with a valid cert that..");
         if (sname != NULL)
@@ -747,7 +746,17 @@ int ossl_cmp_msg_check_update(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *msg,
                 || ASN1_OCTET_STRING_cmp(ctx->transactionID,
                                          hdr->transactionID) != 0)) {
 #ifndef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
-        ERR_raise(ERR_LIB_CMP, CMP_R_TRANSACTIONID_UNMATCHED);
+        char *ctx_str, *hdr_str;
+
+        ctx_str = i2s_ASN1_OCTET_STRING(NULL, ctx->transactionID);
+        hdr_str = hdr->transactionID == NULL ? "(none)"
+            : i2s_ASN1_OCTET_STRING(NULL, hdr->transactionID);
+        ERR_raise_data(ERR_LIB_CMP, CMP_R_TRANSACTIONID_UNMATCHED,
+                       "expected = %s, actual = %s",
+                       ctx_str == NULL ? "?" : ctx_str,
+                       hdr_str == NULL ? "?" : hdr_str);
+        OPENSSL_free(ctx_str);
+        OPENSSL_free(hdr_str);
         return 0;
 #endif
     }
diff --git a/crypto/x509/v3_akid.c b/crypto/x509/v3_akid.c
index 59ea439eddf..2a993dd5bcb 100644
--- a/crypto/x509/v3_akid.c
+++ b/crypto/x509/v3_akid.c
@@ -44,7 +44,7 @@ static STACK_OF(CONF_VALUE) *i2v_AUTHORITY_KEYID(X509V3_EXT_METHOD *method,
     STACK_OF(CONF_VALUE) *origextlist = extlist, *tmpextlist;
 
     if (akeyid->keyid) {
-        tmp = OPENSSL_buf2hexstr(akeyid->keyid->data, akeyid->keyid->length);
+        tmp = i2s_ASN1_OCTET_STRING(NULL, akeyid->keyid);
         if (tmp == NULL) {
             ERR_raise(ERR_LIB_X509V3, ERR_R_MALLOC_FAILURE);
             return NULL;
@@ -66,7 +66,7 @@ static STACK_OF(CONF_VALUE) *i2v_AUTHORITY_KEYID(X509V3_EXT_METHOD *method,
         extlist = tmpextlist;
     }
     if (akeyid->serial) {
-        tmp = OPENSSL_buf2hexstr(akeyid->serial->data, akeyid->serial->length);
+        tmp = i2s_ASN1_OCTET_STRING(NULL, akeyid->serial);
         if (tmp == NULL) {
             ERR_raise(ERR_LIB_X509V3, ERR_R_MALLOC_FAILURE);
             goto err;