CMP: Improve reporting of error codes and related strings via 'error' msg

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

6 files changed, 34 insertions, 21 deletions

diff --git a/crypto/cmp/cmp_client.c b/crypto/cmp/cmp_client.c
index ea6ca39fb3..c7a846f03e 100644
--- a/crypto/cmp/cmp_client.c
+++ b/crypto/cmp/cmp_client.c
@@ -216,14 +216,14 @@ static int send_receive_check(OSSL_CMP_CTX *ctx, const OSSL_CMP_MSG *req,
                                                   sizeof(buf)) != NULL)
             ERR_add_error_data(1, buf);
         if (emc->errorCode != NULL
-                && BIO_snprintf(buf, sizeof(buf), "; errorCode: %ld",
+                && BIO_snprintf(buf, sizeof(buf), "; errorCode: %08lX",
                                 ASN1_INTEGER_get(emc->errorCode)) > 0)
             ERR_add_error_data(1, buf);
         if (emc->errorDetails != NULL) {
             char *text = ossl_sk_ASN1_UTF8STRING2text(emc->errorDetails, ", ",
                                                       OSSL_CMP_PKISI_BUFLEN - 1);
 
-            if (text != NULL)
+            if (text != NULL && *text != '\0')
                 ERR_add_error_data(2, "; errorDetails: ", text);
             OPENSSL_free(text);
         }
diff --git a/crypto/cmp/cmp_local.h b/crypto/cmp/cmp_local.h
index e63150bcf6..1cca238916 100644
--- a/crypto/cmp/cmp_local.h
+++ b/crypto/cmp/cmp_local.h
@@ -882,7 +882,7 @@ OSSL_CMP_MSG *ossl_cmp_genm_new(OSSL_CMP_CTX *ctx);
 OSSL_CMP_MSG *ossl_cmp_genp_new(OSSL_CMP_CTX *ctx,
                                 const STACK_OF(OSSL_CMP_ITAV) *itavs);
 OSSL_CMP_MSG *ossl_cmp_error_new(OSSL_CMP_CTX *ctx, const OSSL_CMP_PKISI *si,
-                                 int errorCode, const char *details,
+                                 int64_t errorCode, const char *details,
                                  int unprotected);
 int ossl_cmp_certstatus_set0_certHash(OSSL_CMP_CERTSTATUS *certStatus,
                                       ASN1_OCTET_STRING *hash);
diff --git a/crypto/cmp/cmp_msg.c b/crypto/cmp/cmp_msg.c
index ee50144726..fe4b64d575 100644
--- a/crypto/cmp/cmp_msg.c
+++ b/crypto/cmp/cmp_msg.c
@@ -727,10 +727,11 @@ OSSL_CMP_MSG *ossl_cmp_genp_new(OSSL_CMP_CTX *ctx,
 }
 
 OSSL_CMP_MSG *ossl_cmp_error_new(OSSL_CMP_CTX *ctx, const OSSL_CMP_PKISI *si,
-                                 int errorCode, const char *details,
+                                 int64_t errorCode, const char *details,
                                  int unprotected)
 {
     OSSL_CMP_MSG *msg = NULL;
+    const char *lib = NULL, *reason = NULL;
     OSSL_CMP_PKIFREETEXT *ft;
 
     if (!ossl_assert(ctx != NULL && si != NULL))
@@ -743,17 +744,26 @@ OSSL_CMP_MSG *ossl_cmp_error_new(OSSL_CMP_CTX *ctx, const OSSL_CMP_PKISI *si,
     if ((msg->body->value.error->pKIStatusInfo = OSSL_CMP_PKISI_dup(si))
         == NULL)
         goto err;
-    if (errorCode >= 0) {
-        if ((msg->body->value.error->errorCode = ASN1_INTEGER_new()) == NULL)
-            goto err;
-        if (!ASN1_INTEGER_set(msg->body->value.error->errorCode, errorCode))
-            goto err;
+    if ((msg->body->value.error->errorCode = ASN1_INTEGER_new()) == NULL)
+        goto err;
+    if (!ASN1_INTEGER_set_int64(msg->body->value.error->errorCode, errorCode))
+        goto err;
+    if (errorCode > 0 && errorCode < (ERR_SYSTEM_FLAG << 1)) {
+        lib = ERR_lib_error_string((unsigned long)errorCode);
+        reason = ERR_reason_error_string((unsigned long)errorCode);
     }
-    if (details != NULL) {
+    if (lib != NULL || reason != NULL || details != NULL) {
         if ((ft = sk_ASN1_UTF8STRING_new_null()) == NULL)
             goto err;
         msg->body->value.error->errorDetails = ft;
-        if (!ossl_cmp_sk_ASN1_UTF8STRING_push_str(ft, details))
+        if (lib != NULL && *lib != '\0'
+                && !ossl_cmp_sk_ASN1_UTF8STRING_push_str(ft, lib))
+            goto err;
+        if (reason != NULL && *reason != '\0'
+                && !ossl_cmp_sk_ASN1_UTF8STRING_push_str(ft, reason))
+            goto err;
+        if (details != NULL
+                && !ossl_cmp_sk_ASN1_UTF8STRING_push_str(ft, details))
             goto err;
     }
 
diff --git a/crypto/cmp/cmp_server.c b/crypto/cmp/cmp_server.c
index 345f312df4..593c074f8d 100644
--- a/crypto/cmp/cmp_server.c
+++ b/crypto/cmp/cmp_server.c
@@ -562,7 +562,7 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx,
  err:
     if (rsp == NULL) {
         /* on error, try to respond with CMP error message to client */
-        const char *data = NULL;
+        const char *data = NULL, *reason = NULL;
         int flags = 0;
         unsigned long err = ERR_peek_error_data(&data, &flags);
         int fail_info = 1 << OSSL_CMP_PKIFAILUREINFO_badRequest;
@@ -574,12 +574,12 @@ OSSL_CMP_MSG *OSSL_CMP_SRV_process_request(OSSL_CMP_SRV_CTX *srv_ctx,
             (void)ossl_cmp_ctx_set1_recipNonce(ctx, hdr->senderNonce);
         }
 
+        if ((flags & ERR_TXT_STRING) == 0 || *data == '\0')
+            data = NULL;
+        reason = ERR_reason_error_string(err);
         if ((si = OSSL_CMP_STATUSINFO_new(OSSL_CMP_PKISTATUS_rejection,
-                                          fail_info, data)) != 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,
+                                          fail_info, reason)) != NULL) {
+            rsp = ossl_cmp_error_new(srv_ctx->ctx, si, err,
                                      data, srv_ctx->sendUnprotectedErrors);
             OSSL_CMP_PKISI_free(si);
         }
diff --git a/doc/internal/man3/ossl_cmp_certreq_new.pod b/doc/internal/man3/ossl_cmp_certreq_new.pod
index ced9f01fc7..05530dafae 100644
--- a/doc/internal/man3/ossl_cmp_certreq_new.pod
+++ b/doc/internal/man3/ossl_cmp_certreq_new.pod
@@ -38,7 +38,7 @@ ossl_cmp_error_new
  OSSL_CMP_MSG *ossl_cmp_genm_new(OSSL_CMP_CTX *ctx);
  OSSL_CMP_MSG *ossl_cmp_genp_new(OSSL_CMP_CTX *ctx);
  OSSL_CMP_MSG *ossl_cmp_error_new(OSSL_CMP_CTX *ctx, const OSSL_CMP_PKISI *si,
-                                  int errorCode, const char *details,
+                                  int64_t errorCode, const char *details,
                                   int unprotected);
 
 =head1 DESCRIPTION
@@ -141,8 +141,11 @@ ossl_cmp_genm_new() creates a new General Message with an empty ITAV stack.
 ossl_cmp_genp_new() creates a new General Response with an empty ITAV stack.
 
 ossl_cmp_error_new() creates a new Error Message with the given contents
-with the given I<si>, I<errorCode> (if nonnegative), and optional I<details>.
-It does not protect the message if I<unprotectedErrors> is nonzero.
+I<si>, I<errorCode>, and optional I<details>.
+If I<errorCode> is positive and in the range of an OpenSSL error code,
+the library and reason strings are included in the B<errorDetails> field.
+If given, the I<details> are added to the contents of the B<errorDetails> field.
+The function does not protect the message if I<unprotectedErrors> is nonzero.
 
 =head1 NOTES
 
diff --git a/test/cmp_server_test.c b/test/cmp_server_test.c
index 8583c6f608..9f20d27ac9 100644
--- a/test/cmp_server_test.c
+++ b/test/cmp_server_test.c
@@ -98,7 +98,7 @@ static int execute_test_handle_request(CMP_SRV_TEST_FIXTURE *fixture)
                             OSSL_CMP_PKIBODY_ERROR)
             || !TEST_ptr(errorContent = rsp->body->value.error)
             || !TEST_int_eq(ASN1_INTEGER_get(errorContent->errorCode),
-                            dummy_errorCode))
+                            ERR_PACK(ERR_LIB_CMP, 0, dummy_errorCode)))
         goto end;
 
     res = 1;