]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
Fix ERR_print_errors so that it matches the documented format in doc/man3/ERR_error_s...
authorShane Lontis <shane.lontis@oracle.com>
Tue, 26 May 2020 02:44:36 +0000 (12:44 +1000)
committerShane Lontis <shane.lontis@oracle.com>
Tue, 26 May 2020 02:44:36 +0000 (12:44 +1000)
Fixes #11743

The ouput format had 2 issues that caused it not to match the expected documented format:
(1) At some point the thread id printing was changed to use the OPENSSL_hex2str method which puts ':' between hex bytes.
    An internal function that skips the seperator has been added.
(2) The error code no longer exists. So this was completely removed from the string. It is now replaced by ::

As an example:
  00:77:6E:52:14:7F:00:00:error:asn1 encoding routines:asn1_check_tlen:wrong tag:crypto/asn1/tasn_dec.c:1135:
Is now:
  00776E52147F0000:error::asn1 encoding routines:asn1_check_tlen:wrong tag:crypto/asn1/tasn_dec.c:1135:

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/11789)

crypto/err/err_prn.c
crypto/o_str.c
include/internal/cryptlib.h
test/build.info
test/errtest.c
test/hexstr_test.c [new file with mode: 0644]
test/recipes/04-test_hexstring.t [new file with mode: 0644]

index 5c4ebcbddd2749289be6825f793ceb4dee7d627d..80cc0ecf1a3d6fce68e8fa763f535d941b9a99ab 100644 (file)
@@ -35,8 +35,9 @@ void ERR_print_errors_cb(int (*cb) (const char *str, size_t len, void *u),
             func = "unknown function";
         if ((flags & ERR_TXT_STRING) == 0)
             data = "";
-        hex = OPENSSL_buf2hexstr((const unsigned char *)&tid, sizeof(tid));
-        BIO_snprintf(buf, sizeof(buf), "%s:error:%s:%s:%s:%s:%d:%s\n",
+        hex = openssl_buf2hexstr_sep((const unsigned char *)&tid, sizeof(tid),
+                                     '\0');
+        BIO_snprintf(buf, sizeof(buf), "%s:error::%s:%s:%s:%s:%d:%s\n",
                      hex == NULL ? "<null>" : hex, lib, func, reason, file,
                      line, data);
         OPENSSL_free(hex);
index 6780188cda273459cee3df6ee2f16e70eb41c256..6578857b9495a4eb2c6e135335381c343cfe23a1 100644 (file)
@@ -12,6 +12,9 @@
 #include <openssl/crypto.h>
 #include "internal/cryptlib.h"
 
+#define DEFAULT_SEPARATOR ':'
+#define CH_ZERO '\0'
+
 char *CRYPTO_strdup(const char *str, const char* file, int line)
 {
     char *ret;
@@ -37,7 +40,7 @@ char *CRYPTO_strndup(const char *str, size_t s, const char* file, int line)
     ret = CRYPTO_malloc(maxlen + 1, file, line);
     if (ret) {
         memcpy(ret, str, maxlen);
-        ret[maxlen] = '\0';
+        ret[maxlen] = CH_ZERO;
     }
     return ret;
 }
@@ -61,7 +64,7 @@ size_t OPENSSL_strnlen(const char *str, size_t maxlen)
 {
     const char *p;
 
-    for (p = str; maxlen-- != 0 && *p != '\0'; ++p) ;
+    for (p = str; maxlen-- != 0 && *p != CH_ZERO; ++p) ;
 
     return p - str;
 }
@@ -74,7 +77,7 @@ size_t OPENSSL_strlcpy(char *dst, const char *src, size_t size)
         l++;
     }
     if (size)
-        *dst = '\0';
+        *dst = CH_ZERO;
     return l + strlen(src);
 }
 
@@ -129,11 +132,8 @@ int OPENSSL_hexchar2int(unsigned char c)
     return -1;
 }
 
-/*
- * Give a string of hex digits convert to a buffer
- */
-int OPENSSL_hexstr2buf_ex(unsigned char *buf, size_t buf_n, size_t *buflen,
-                          const char *str)
+static int hexstr2buf_sep(unsigned char *buf, size_t buf_n, size_t *buflen,
+                          const char *str, const char sep)
 {
     unsigned char *q;
     unsigned char ch, cl;
@@ -143,26 +143,24 @@ int OPENSSL_hexstr2buf_ex(unsigned char *buf, size_t buf_n, size_t *buflen,
 
     for (p = (const unsigned char *)str, q = buf, cnt = 0; *p; ) {
         ch = *p++;
-        if (ch == ':')
+        /* A separator of CH_ZERO means there is no separator */
+        if (ch == sep && sep != CH_ZERO)
             continue;
         cl = *p++;
         if (!cl) {
-            CRYPTOerr(CRYPTO_F_OPENSSL_HEXSTR2BUF_EX,
-                      CRYPTO_R_ODD_NUMBER_OF_DIGITS);
+            CRYPTOerr(0, CRYPTO_R_ODD_NUMBER_OF_DIGITS);
             return 0;
         }
         cli = OPENSSL_hexchar2int(cl);
         chi = OPENSSL_hexchar2int(ch);
         if (cli < 0 || chi < 0) {
-            CRYPTOerr(CRYPTO_F_OPENSSL_HEXSTR2BUF_EX,
-                      CRYPTO_R_ILLEGAL_HEX_DIGIT);
+            CRYPTOerr(0, CRYPTO_R_ILLEGAL_HEX_DIGIT);
             return 0;
         }
         cnt++;
         if (q != NULL) {
             if (cnt > buf_n) {
-                CRYPTOerr(CRYPTO_F_OPENSSL_HEXSTR2BUF_EX,
-                          CRYPTO_R_TOO_SMALL_BUFFER);
+                CRYPTOerr(0, CRYPTO_R_TOO_SMALL_BUFFER);
                 return 0;
             }
             *q++ = (unsigned char)((chi << 4) | cli);
@@ -174,21 +172,31 @@ int OPENSSL_hexstr2buf_ex(unsigned char *buf, size_t buf_n, size_t *buflen,
     return 1;
 }
 
-unsigned char *OPENSSL_hexstr2buf(const char *str, long *buflen)
+/*
+ * Given a string of hex digits convert to a buffer
+ */
+int OPENSSL_hexstr2buf_ex(unsigned char *buf, size_t buf_n, size_t *buflen,
+                          const char *str)
+{
+    return hexstr2buf_sep(buf, buf_n, buflen, str, DEFAULT_SEPARATOR);
+}
+
+unsigned char *openssl_hexstr2buf_sep(const char *str, long *buflen,
+                                      const char sep)
 {
     unsigned char *buf;
     size_t buf_n, tmp_buflen;
 
     buf_n = strlen(str) >> 1;
     if ((buf = OPENSSL_malloc(buf_n)) == NULL) {
-        CRYPTOerr(CRYPTO_F_OPENSSL_HEXSTR2BUF, ERR_R_MALLOC_FAILURE);
+        CRYPTOerr(0, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
 
     if (buflen != NULL)
         *buflen = 0;
     tmp_buflen = 0;
-    if (OPENSSL_hexstr2buf_ex(buf, buf_n, &tmp_buflen, str)) {
+    if (hexstr2buf_sep(buf, buf_n, &tmp_buflen, str, sep)) {
         if (buflen != NULL)
             *buflen = (long)tmp_buflen;
         return buf;
@@ -197,21 +205,29 @@ unsigned char *OPENSSL_hexstr2buf(const char *str, long *buflen)
     return NULL;
 }
 
-int OPENSSL_buf2hexstr_ex(char *str, size_t str_n, size_t *strlen,
-                          const unsigned char *buf, size_t buflen)
+unsigned char *OPENSSL_hexstr2buf(const char *str, long *buflen)
+{
+    return openssl_hexstr2buf_sep(str, buflen, DEFAULT_SEPARATOR);
+}
+
+static int buf2hexstr_sep(char *str, size_t str_n, size_t *strlen,
+                          const unsigned char *buf, size_t buflen,
+                          const char sep)
 {
     static const char hexdig[] = "0123456789ABCDEF";
     const unsigned char *p;
     char *q;
     size_t i;
+    int has_sep = (sep != CH_ZERO);
+    size_t len = has_sep ? buflen * 3 : 1 + buflen * 2;
 
     if (strlen != NULL)
-        *strlen = buflen * 3;
+        *strlen = len;
     if (str == NULL)
         return 1;
 
-    if (str_n < (unsigned long)buflen * 3) {
-        CRYPTOerr(CRYPTO_F_OPENSSL_BUF2HEXSTR_EX, CRYPTO_R_TOO_SMALL_BUFFER);
+    if (str_n < (unsigned long)len) {
+        CRYPTOerr(0, CRYPTO_R_TOO_SMALL_BUFFER);
         return 0;
     }
 
@@ -219,21 +235,26 @@ int OPENSSL_buf2hexstr_ex(char *str, size_t str_n, size_t *strlen,
     for (i = 0, p = buf; i < buflen; i++, p++) {
         *q++ = hexdig[(*p >> 4) & 0xf];
         *q++ = hexdig[*p & 0xf];
-        *q++ = ':';
+        if (has_sep)
+            *q++ = sep;
     }
-    q[-1] = 0;
+    if (has_sep)
+        --q;
+    *q = CH_ZERO;
+
 #ifdef CHARSET_EBCDIC
     ebcdic2ascii(str, str, q - str - 1);
 #endif
     return 1;
 }
 
-/*
- * Given a buffer of length 'len' return a OPENSSL_malloc'ed string with its
- * hex representation @@@ (Contents of buffer are always kept in ASCII, also
- * on EBCDIC machines)
- */
-char *OPENSSL_buf2hexstr(const unsigned char *buf, long buflen)
+int OPENSSL_buf2hexstr_ex(char *str, size_t str_n, size_t *strlen,
+                          const unsigned char *buf, size_t buflen)
+{
+    return buf2hexstr_sep(str, str_n, strlen, buf, buflen, DEFAULT_SEPARATOR);
+}
+
+char *openssl_buf2hexstr_sep(const unsigned char *buf, long buflen, char sep)
 {
     char *tmp;
     size_t tmp_n;
@@ -241,18 +262,29 @@ char *OPENSSL_buf2hexstr(const unsigned char *buf, long buflen)
     if (buflen == 0)
         return OPENSSL_zalloc(1);
 
-    tmp_n = buflen * 3;
+    tmp_n = (sep != CH_ZERO) ? buflen * 3 : 1 + buflen * 2;
     if ((tmp = OPENSSL_malloc(tmp_n)) == NULL) {
-        CRYPTOerr(CRYPTO_F_OPENSSL_BUF2HEXSTR, ERR_R_MALLOC_FAILURE);
+        CRYPTOerr(0, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
 
-    if (OPENSSL_buf2hexstr_ex(tmp, tmp_n, NULL, buf, buflen))
+    if (buf2hexstr_sep(tmp, tmp_n, NULL, buf, buflen, sep))
         return tmp;
     OPENSSL_free(tmp);
     return NULL;
 }
 
+
+/*
+ * Given a buffer of length 'len' return a OPENSSL_malloc'ed string with its
+ * hex representation @@@ (Contents of buffer are always kept in ASCII, also
+ * on EBCDIC machines)
+ */
+char *OPENSSL_buf2hexstr(const unsigned char *buf, long buflen)
+{
+    return openssl_buf2hexstr_sep(buf, buflen, ':');
+}
+
 int openssl_strerror_r(int errnum, char *buf, size_t buflen)
 {
 #if defined(_MSC_VER) && _MSC_VER>=1400
index b479b58a84887f397a93cae96f14154c95794e0b..a4f18a5d3f5520bf93b7ca0a7a0e729a731cf3c7 100644 (file)
@@ -241,4 +241,8 @@ char *sk_ASN1_UTF8STRING2text(STACK_OF(ASN1_UTF8STRING) *text, const char *sep,
                               size_t max_len);
 char *ipaddr_to_asc(unsigned char *p, int len);
 
+char *openssl_buf2hexstr_sep(const unsigned char *buf, long buflen, char sep);
+unsigned char *openssl_hexstr2buf_sep(const char *str, long *buflen,
+                                      const char sep);
+
 #endif
index 112b68c22f81fb2790ec4d95abe2c0d727324a1c..9697e55f12f5b87c688a7b8ff1a2517a793d0737 100644 (file)
@@ -54,7 +54,7 @@ IF[{- !$disabled{tests} -}]
           http_test servername_test ocspapitest fatalerrtest tls13ccstest \
           sysdefaulttest errtest ssl_ctx_test gosttest \
           context_internal_test aesgcmtest params_test evp_pkey_dparams_test \
-          keymgmt_internal_test
+          keymgmt_internal_test hexstr_test
 
   SOURCE[confdump]=confdump.c
   INCLUDE[confdump]=../include ../apps/include
@@ -736,6 +736,11 @@ IF[{- !$disabled{tests} -}]
   INCLUDE[params_test]=.. ../include ../apps/include
   DEPEND[params_test]=../libcrypto.a libtestutil.a
 
+  PROGRAMS{noinst}=hexstr_test
+  SOURCE[hexstr_test]=hexstr_test.c
+  INCLUDE[hexstr_test]=.. ../include ../apps/include
+  DEPEND[hexstr_test]=../libcrypto.a libtestutil.a
+
   PROGRAMS{noinst}=namemap_internal_test
   SOURCE[namemap_internal_test]=namemap_internal_test.c
   INCLUDE[namemap_internal_test]=.. ../include ../apps/include
index 179c338c4503f0d2ecd8f30e6c6ae81e3dd468dc..cc2f6612d144ee6145ede291f5be86bae5788671 100644 (file)
@@ -7,6 +7,7 @@
  * https://www.openssl.org/source/license.html
  */
 
+#include <string.h>
 #include <openssl/opensslconf.h>
 #include <openssl/err.h>
 
 # include <errno.h>
 #endif
 
+#ifndef OPENSSL_NO_DEPRECATED_3_0
+# define IS_HEX(ch) ((ch >= '0' && ch <='9') || (ch >= 'A' && ch <='F'))
+
+static int test_print_error_format(void)
+{
+    static const char expected[] =
+        ":error::system library:test_print_error_format:Operation not permitted:"
+# ifndef OPENSSL_NO_FILENAMES
+        "errtest.c:30:";
+# else
+        ":0:";
+# endif
+    char *out = NULL, *p = NULL;
+    int ret = 0, len;
+    BIO *bio = NULL;
+
+    if (!TEST_ptr(bio = BIO_new(BIO_s_mem())))
+        return 0;
+
+    ERR_PUT_error(ERR_LIB_SYS, 0, 1, "errtest.c", 30);
+    ERR_print_errors(bio);
+
+    if (!TEST_int_gt(len = BIO_get_mem_data(bio, &out), 0))
+        goto err;
+    /* Skip over the variable thread id at the start of the string */
+    for (p = out; *p != ':' && *p != 0; ++p) {
+        if (!TEST_true(IS_HEX(*p)))
+            goto err;
+    }
+    if (!TEST_true(*p != 0)
+        || !TEST_strn_eq(expected, p, strlen(expected)))
+        goto err;
+
+    ret = 1;
+err:
+    BIO_free(bio);
+    return ret;
+}
+#endif
+
 /* Test that querying the error queue preserves the OS error. */
 static int preserves_system_error(void)
 {
@@ -79,5 +120,8 @@ int setup_tests(void)
     ADD_TEST(preserves_system_error);
     ADD_TEST(vdata_appends);
     ADD_TEST(raised_error);
+#ifndef OPENSSL_NO_DEPRECATED_3_0
+    ADD_TEST(test_print_error_format);
+#endif
     return 1;
 }
diff --git a/test/hexstr_test.c b/test/hexstr_test.c
new file mode 100644 (file)
index 0000000..c4f13b6
--- /dev/null
@@ -0,0 +1,133 @@
+/*
+ * Copyright 2020 The OpenSSL Project Authors. All Rights Reserved.
+ *
+ * Licensed under the Apache License 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * https://www.openssl.org/source/license.html
+ * or in the file LICENSE in the source distribution.
+ */
+
+/*
+ * This program tests the use of OSSL_PARAM, currently in raw form.
+ */
+
+#include "internal/nelem.h"
+#include "internal/cryptlib.h"
+#include "testutil.h"
+
+struct testdata
+{
+    const char *in;
+    const unsigned char *expected;
+    size_t expected_len;
+    const char sep;
+};
+
+static const unsigned char test_1[] = { 0xAB, 0xCD, 0xEF, 0xF1 };
+static const unsigned char test_2[] = { 0xAB, 0xCD, 0xEF, 0x76, 0x00 };
+
+static struct testdata tbl_testdata[] = {
+    {
+        "AB:CD:EF:F1",
+        test_1, sizeof(test_1),
+        ':',
+    },
+    {
+        "AB:CD:EF:76:00",
+        test_2, sizeof(test_2),
+        ':',
+    },
+    {
+        "AB_CD_EF_F1",
+        test_1, sizeof(test_1),
+        '_',
+    },
+    {
+        "AB_CD_EF_76_00",
+        test_2, sizeof(test_2),
+        '_',
+    },
+    {
+        "ABCDEFF1",
+        test_1, sizeof(test_1),
+        '\0',
+    },
+    {
+        "ABCDEF7600",
+        test_2, sizeof(test_2),
+        '\0',
+    },
+};
+
+static int test_hexstr_sep_to_from(int test_index)
+{
+    int ret = 0;
+    long len = 0;
+    unsigned char *buf = NULL;
+    char *out = NULL;
+    struct testdata *test = &tbl_testdata[test_index];
+
+    if (!TEST_ptr(buf = openssl_hexstr2buf_sep(test->in, &len, test->sep))
+        || !TEST_mem_eq(buf, len, test->expected, test->expected_len)
+        || !TEST_ptr(out = openssl_buf2hexstr_sep(buf, len, test->sep))
+        || !TEST_str_eq(out, test->in))
+       goto err;
+
+    ret = 1;
+err:
+    OPENSSL_free(buf);
+    OPENSSL_free(out);
+    return ret;
+}
+
+static int test_hexstr_to_from(int test_index)
+{
+    int ret = 0;
+    long len = 0;
+    unsigned char *buf = NULL;
+    char *out = NULL;
+    struct testdata *test = &tbl_testdata[test_index];
+
+    if (test->sep != '_') {
+        if (!TEST_ptr(buf = OPENSSL_hexstr2buf(test->in, &len))
+            || !TEST_mem_eq(buf, len, test->expected, test->expected_len)
+            || !TEST_ptr(out = OPENSSL_buf2hexstr(buf, len)))
+           goto err;
+        if (test->sep == ':') {
+            if (!TEST_str_eq(out, test->in))
+                goto err;
+        } else if (!TEST_str_ne(out, test->in)) {
+            goto err;
+        }
+    } else {
+        if (!TEST_ptr_null(buf = OPENSSL_hexstr2buf(test->in, &len)))
+            goto err;
+    }
+    ret = 1;
+err:
+    OPENSSL_free(buf);
+    OPENSSL_free(out);
+    return ret;
+}
+
+static int test_hexstr_ex_to_from(int test_index)
+{
+    size_t len = 0;
+    char out[64];
+    unsigned char buf[64];
+    struct testdata *test = &tbl_testdata[test_index];
+
+    return TEST_true(OPENSSL_hexstr2buf_ex(buf, sizeof(buf), &len, test->in))
+           && TEST_mem_eq(buf, len, test->expected, test->expected_len)
+           && TEST_true(OPENSSL_buf2hexstr_ex(out, sizeof(out), NULL, buf, len))
+           && TEST_str_eq(out, test->in);
+}
+
+int setup_tests(void)
+{
+    ADD_ALL_TESTS(test_hexstr_sep_to_from, OSSL_NELEM(tbl_testdata));
+    ADD_ALL_TESTS(test_hexstr_to_from, OSSL_NELEM(tbl_testdata));
+    ADD_ALL_TESTS(test_hexstr_ex_to_from, 2);
+    return 1;
+}
diff --git a/test/recipes/04-test_hexstring.t b/test/recipes/04-test_hexstring.t
new file mode 100644 (file)
index 0000000..664868f
--- /dev/null
@@ -0,0 +1,15 @@
+#! /usr/bin/env perl
+# Copyright 2020 The OpenSSL Project Authors. All Rights Reserved.
+#
+# Licensed under the Apache License 2.0 (the "License").  You may not use
+# this file except in compliance with the License.  You can obtain a copy
+# in the file LICENSE in the source distribution or at
+# https://www.openssl.org/source/license.html
+
+use strict;
+use OpenSSL::Test;
+use OpenSSL::Test::Simple;
+
+setup("test_hexstring");
+
+simple_test("test_hexstring", "hexstr_test");