Fix read length in ASN1_STRING_print_ex/to_UTF8. Refactor freeing logic in wolfSSL_X509_get0_extensions. Only read GENERAL_NAME types with strings in wolfSSL_X509V3_EXT_print. Properly handle NULLs inside of DNs in X509_NAME_oneline.

doc/dox_comments/header_files/asn_public.h

@@ -3635,7 +3635,7 @@ int wc_GetSubjectPubKeyInfoDerFromCert(const byte* certDer,
     mistaken for a separator (e.g. a CN of "a/b" is written "/CN=a\\/b").
     The format is similar to, but not byte-identical with, OpenSSL's
     X509_NAME_oneline(): wolfSSL also escapes "\\", does not hex-escape
-    bytes outside 0x20-0x7E (OpenSSL writes them as "\\xHH"), and always
+    bytes outside 0x20-0x7E other than NUL (OpenSSL writes them as "\\xHH"), and always
     separates attributes with "/", even within a multi-valued RDN (OpenSSL
     uses "+"). Compare against strings produced by this library rather
     than by OpenSSL or by hand. The output is NOT
@@ -3677,7 +3677,7 @@ int wc_GetDecodedCertSubject(const struct DecodedCert* cert,
     mistaken for a separator (e.g. a CN of "a/b" is written "/CN=a\\/b").
     The format is similar to, but not byte-identical with, OpenSSL's
     X509_NAME_oneline(): wolfSSL also escapes "\\", does not hex-escape
-    bytes outside 0x20-0x7E (OpenSSL writes them as "\\xHH"), and always
+    bytes outside 0x20-0x7E other than NUL (OpenSSL writes them as "\\xHH"), and always
     separates attributes with "/", even within a multi-valued RDN (OpenSSL
     uses "+"). Compare against strings produced by this library rather
     than by OpenSSL or by hand. The output is NOT

doc/dox_comments/header_files/ssl.h

@@ -6525,7 +6525,7 @@ long wolfSSL_BIO_get_mem_ptr(WOLFSSL_BIO *bio, WOLFSSL_BUF_MEM **m);
     for a separator (e.g. a CN of "a/b" is written "/CN=a\\/b"). The format
     is similar to, but not byte-identical with, OpenSSL's
     X509_NAME_oneline(): wolfSSL also escapes "\\", does not hex-escape
-    bytes outside 0x20-0x7E (OpenSSL writes them as "\\xHH"), and always
+    bytes outside 0x20-0x7E other than NUL (OpenSSL writes them as "\\xHH"), and always
     separates attributes with "/", even within a multi-valued RDN (OpenSSL
     uses "+").
 

src/ssl_asn1.c

@@ -3749,10 +3749,12 @@ static int wolfssl_asn1_string_print_esc_2253(WOLFSSL_BIO *bio,
     WOLFSSL_ASN1_STRING *str)
 {
     char* p;
+    int i;
     int str_len = 0;
 
-    /* Write all of string character by character. */
-    for (p = str->data; (*p) != '\0'; p++) {
+    /* Write all of string character by character.
+     * Use length as data is not necessarily NUL terminated. */
+    for (p = str->data, i = 0; i < str->length; p++, i++) {
         /* Check if character needs escaping. */
         if (wolfssl_check_esc_char(*p)){
             /* Update count of written characters. */

src/x509.c

@@ -406,12 +406,14 @@ static WOLFSSL_STACK* generateExtStack(const WOLFSSL_X509 *x)
 }
 
 /**
+ * The returned stack is owned by the X509 and stays valid until the X509 is
+ * freed or its DER encoding is replaced (e.g. by wolfSSL_X509_sign).
+ *
  * @param x Certificate to extract extensions from
  * @return STACK_OF(X509_EXTENSION)*
  */
 const WOLFSSL_STACK *wolfSSL_X509_get0_extensions(const WOLFSSL_X509 *x)
 {
-    int numOfExt;
     WOLFSSL_X509 *x509 = (WOLFSSL_X509*)x;
     WOLFSSL_ENTER("wolfSSL_X509_get0_extensions");
 
@@ -420,10 +422,9 @@ const WOLFSSL_STACK *wolfSSL_X509_get0_extensions(const WOLFSSL_X509 *x)
         return NULL;
     }
 
-    numOfExt = wolfSSL_X509_get_ext_count(x509);
-
-    if (numOfExt != wolfSSL_sk_num(x509->ext_sk_full)) {
-        wolfSSL_sk_pop_free(x509->ext_sk_full, NULL);
+    /* Only build the stack once. Freeing and rebuilding it here would leave
+     * the caller of a previous call holding a dangling pointer. */
+    if (x509->ext_sk_full == NULL) {
         x509->ext_sk_full = generateExtStack(x);
     }
 
@@ -1155,15 +1156,13 @@ WOLFSSL_X509_EXTENSION* wolfSSL_X509_set_ext(WOLFSSL_X509* x509, int loc)
         tmpIdx = idx + length;
 
         /* Get CRITICAL. If not present, defaults to false.
-         * It present, must be a valid TRUE */
+         * Accept any one byte value like the certificate parser does. */
         if ((tmpIdx < (word32)sz) &&
             (input[tmpIdx] == ASN_BOOLEAN))
         {
             if (((tmpIdx + 2) >= (word32)sz) ||
                 /* Check bool length */
-                (input[tmpIdx+1] != 1) ||
-                /* Assert true if CRITICAL present */
-                (input[tmpIdx+2] != 0xff))
+                (input[tmpIdx+1] != 1))
             {
                 WOLFSSL_MSG("Error decoding unknown extension data");
                 wolfSSL_X509_EXTENSION_free(ext);
@@ -1172,7 +1171,7 @@ WOLFSSL_X509_EXTENSION* wolfSSL_X509_set_ext(WOLFSSL_X509* x509, int loc)
                 return NULL;
             }
 
-            ext->crit = 1;
+            ext->crit = (input[tmpIdx+2] != 0);
             tmpIdx += 3;
         }
 
@@ -1189,25 +1188,24 @@ WOLFSSL_X509_EXTENSION* wolfSSL_X509_set_ext(WOLFSSL_X509* x509, int loc)
 
         tmpIdx++;
 
-        if (GetLength(input, &tmpIdx, &length, (word32)sz) <= 0) {
+        /* An empty OCTET STRING is accepted by the certificate parser. */
+        if (GetLength(input, &tmpIdx, &length, (word32)sz) < 0) {
             WOLFSSL_MSG("Error: Invalid Input Length.");
             wolfSSL_X509_EXTENSION_free(ext);
             FreeDecodedCert(cert);
             WC_FREE_VAR_EX(cert, NULL, DYNAMIC_TYPE_DCERT);
             return NULL;
         }
-        ext->value.data = (char*)XMALLOC(length, NULL,
-            DYNAMIC_TYPE_ASN1);
-        ext->value.isDynamic = 1;
-        if (ext->value.data == NULL) {
-            WOLFSSL_MSG("Failed to malloc ASN1_STRING data");
+        /* Copy with wolfSSL_ASN1_STRING_set() so the data is NUL terminated
+         * like every other ASN1_STRING. */
+        if (wolfSSL_ASN1_STRING_set(&ext->value, input + tmpIdx, length)
+                != 1) {
+            WOLFSSL_MSG("Failed to set ASN1_STRING data");
             wolfSSL_X509_EXTENSION_free(ext);
             FreeDecodedCert(cert);
             WC_FREE_VAR_EX(cert, NULL, DYNAMIC_TYPE_DCERT);
             return NULL;
         }
-        XMEMCPY(ext->value.data,input+tmpIdx,length);
-        ext->value.length = length;
 
         break; /* Got the Extension. Now exit while loop. */
 
@@ -1737,8 +1735,31 @@ int wolfSSL_X509V3_EXT_print(WOLFSSL_BIO *out, WOLFSSL_X509_EXTENSION *ext,
             sk = ext->ext_sk;
             while (sk != NULL) {
                 if (sk->type == STACK_TYPE_GEN_NAME && sk->data.gn) {
+                    /* Only the string backed GENERAL_NAME types hold a
+                     * WOLFSSL_ASN1_STRING in the union. otherName, dirName
+                     * and registeredID carry other objects there. */
+                    switch (sk->data.gn->type) {
+                        case WOLFSSL_GEN_EMAIL:
+                        case WOLFSSL_GEN_DNS:
+                        case WOLFSSL_GEN_URI:
+                        case WOLFSSL_GEN_IPADD:
+                        case WOLFSSL_GEN_IA5:
+                            break;
+                        default:
+                            WOLFSSL_MSG("Unsupported GENERAL_NAME type");
+                            sk = sk->next;
+                            continue;
+                    }
                     /* str is GENERAL_NAME for subject alternative name ext */
                     str = sk->data.gn->d.ia5;
+                    if (str == NULL) {
+                        /* Skip it like an unsupported type above: a name
+                         * built through the API can carry the type without
+                         * the string. */
+                        WOLFSSL_MSG("NULL GENERAL_NAME string");
+                        sk = sk->next;
+                        continue;
+                    }
                     len = str->length + 2; /* + 2 for NULL char and "," */
                     if (len > tmpSz) {
                         WOLFSSL_MSG("len greater than buffer size");
@@ -14174,6 +14195,12 @@ static int CertFromX509(Cert* cert, WOLFSSL_X509* x509)
          * or in the names they were set from, and not where they were. */
         ClearX509NameRaw(&x509->issuer);
         ClearX509NameRaw(&x509->subject);
+    #endif
+    #if defined(WOLFSSL_QT) || defined(OPENSSL_ALL) || defined(OPENSSL_EXTRA)
+        /* Extension stack cached by wolfSSL_X509_get0_extensions() was built
+         * from the old encoding. */
+        wolfSSL_sk_pop_free(x509->ext_sk_full, NULL);
+        x509->ext_sk_full = NULL;
     #endif
         type = CERT_TYPE;
     #ifdef WOLFSSL_CERT_REQ
@@ -15875,7 +15902,7 @@ WOLFSSL_ASN1_OBJECT* wolfSSL_X509_NAME_ENTRY_get_object(
 
                 data = wolfSSL_ASN1_STRING_data(e->value);
                 if (data != NULL) {
-                    sz = (int)XSTRLEN((const char*)data);
+                    sz = wolfSSL_ASN1_STRING_length(e->value);
                     if ((fullNameSz - (word32)*idx) >=
                             X509CertEscapeName(NULL, (const char*)data,
                                 (word32)sz)) {
@@ -15926,10 +15953,12 @@ WOLFSSL_ASN1_OBJECT* wolfSSL_X509_NAME_ENTRY_get_object(
                 if (data == NULL) {
                     continue;
                 }
+                if (wolfSSL_ASN1_STRING_length(e->value) < 0)
+                    return BUFFER_E;
                 /* Certain characters need to be escaped, so ask for the
                  * length of the value once escaped. */
                 intermediateVal = X509CertEscapeName(NULL, data,
-                    (word32)XSTRLEN(data));
+                    (word32)wolfSSL_ASN1_STRING_length(e->value));
                 if (intermediateVal > (word32)INT_MAX - 1 - totalLen)
                     return BUFFER_E;
                 totalLen += intermediateVal;
@@ -18591,6 +18620,10 @@ static int regenX509REQDerBuffer(WOLFSSL_X509* x509)
         ClearX509NameRaw(&x509->issuer);
         ClearX509NameRaw(&x509->subject);
     #endif
+        /* Extension stack cached by wolfSSL_X509_get0_extensions() was built
+         * from the old encoding. */
+        wolfSSL_sk_pop_free(x509->ext_sk_full, NULL);
+        x509->ext_sk_full = NULL;
         if (AllocDer(&x509->derCert, (word32)derSz, CERT_TYPE,
                                                              x509->heap) == 0) {
             XMEMCPY(x509->derCert->buffer, der, derSz);

tests/api/test_ossl_asn1.c

@@ -1668,8 +1668,9 @@ int test_wolfSSL_ASN1_STRING_print_ex(void)
     /* RFC2253 Escape */
     XMEMSET(rbuf, 0, 255);
     flags = ASN1_STRFLGS_ESC_2253;
-    ExpectIntEQ(p_len = wolfSSL_ASN1_STRING_print_ex(bio, esc_str, flags), 9);
-    ExpectIntEQ(BIO_read(bio, (void*)rbuf, 9), 9);
+    /* 6 bytes of data (incl. NUL) plus 4 escape characters. */
+    ExpectIntEQ(p_len = wolfSSL_ASN1_STRING_print_ex(bio, esc_str, flags), 10);
+    ExpectIntEQ(BIO_read(bio, (void*)rbuf, 10), 10);
     ExpectStrEQ((char*)rbuf, "a\\+\\;\\<\\>");
     ExpectIntEQ(BIO_set_write_buf_size(fixed, 1), 1);
     /* Ensure there is 0 bytes available to write into. */

tests/api/test_ossl_x509_ext.c

@@ -750,6 +750,74 @@ int test_wolfSSL_X509_stack_extensions(void)
     return EXPECT_RESULT();
 }
 
+/* wolfSSL_X509_get0_extensions() returns a stack the X509 owns, so a later
+ * call must hand back the same objects: an application holding an
+ * X509_EXTENSION from an earlier call still reads through it. The encodings
+ * the compat layer could not convert - here an explicit "critical FALSE",
+ * which the certificate parser accepts - used to make the entry count differ
+ * from X509_get_ext_count() and the getter rebuild the stack every call. */
+int test_wolfSSL_X509_get0_extensions_stable(void)
+{
+    EXPECT_DECLS;
+#if defined(OPENSSL_EXTRA) && !defined(NO_CERTS) && !defined(NO_FILESYSTEM) && \
+    defined(HAVE_ECC)
+    /* keyUsage extension: OID 2.5.29.15 then BOOLEAN TRUE. */
+    static const byte keyUsageCrit[] = {
+        0x06, 0x03, 0x55, 0x1d, 0x0f, 0x01, 0x01, 0xff
+    };
+    WOLFSSL_X509* x509 = NULL;
+    const WOLFSSL_STACK* sk1 = NULL;
+    const WOLFSSL_STACK* sk2 = NULL;
+    WOLFSSL_X509_EXTENSION* ext = NULL;
+    const unsigned char* p = NULL;
+    byte* der = NULL;
+    word32 derSz = 0;
+    word32 i;
+    int found = 0;
+    XFILE f = XBADFILE;
+
+    ExpectTrue((f = XFOPEN("./certs/server-ecc.der", "rb")) != XBADFILE);
+    ExpectNotNull(der = (byte*)XMALLOC(FOURK_BUF, NULL,
+        DYNAMIC_TYPE_TMP_BUFFER));
+    if ((f != XBADFILE) && (der != NULL)) {
+        derSz = (word32)XFREAD(der, 1, FOURK_BUF, f);
+    }
+    if (f != XBADFILE)
+        XFCLOSE(f);
+    ExpectIntGT(derSz, sizeof(keyUsageCrit));
+
+    /* Turn the critical flag into an explicit FALSE. */
+    for (i = 0; (der != NULL) && (i + sizeof(keyUsageCrit) <= derSz); i++) {
+        if (XMEMCMP(der + i, keyUsageCrit, sizeof(keyUsageCrit)) == 0) {
+            der[i + sizeof(keyUsageCrit) - 1] = 0x00;
+            found = 1;
+            break;
+        }
+    }
+    ExpectIntEQ(found, 1);
+
+    p = der;
+    ExpectNotNull(x509 = wolfSSL_d2i_X509(NULL, &p, (int)derSz));
+
+    ExpectNotNull(sk1 = wolfSSL_X509_get0_extensions(x509));
+    ExpectIntEQ(X509v3_get_ext_count(sk1),
+        wolfSSL_X509_get_ext_count(x509));
+    /* The extension that was turned into an explicit FALSE is still there. */
+    ExpectIntGE(X509v3_get_ext_by_NID(sk1, NID_key_usage, -1), 0);
+    ExpectNotNull(ext = X509v3_get_ext(sk1, 0));
+
+    /* Second call must not release what the first one returned. */
+    ExpectNotNull(sk2 = wolfSSL_X509_get0_extensions(x509));
+    ExpectPtrEq(sk1, sk2);
+    ExpectNotNull(wolfSSL_X509_EXTENSION_get_object(ext));
+    ExpectPtrEq(X509v3_get_ext(sk2, 0), ext);
+
+    wolfSSL_X509_free(x509);
+    XFREE(der, NULL, DYNAMIC_TYPE_TMP_BUFFER);
+#endif
+    return EXPECT_RESULT();
+}
+
 int test_wolfSSL_X509_EXTENSION_new(void)
 {
     EXPECT_DECLS;
@@ -824,6 +892,7 @@ int test_wolfSSL_X509_EXTENSION_get_data(void)
     WOLFSSL_ASN1_STRING* str = NULL;
     XFILE file = XBADFILE;
 #ifndef WOLFSSL_OLD_EXTDATA_FMT
+    unsigned char* utf8 = NULL;
     const byte ext_data[] = {
         0x04, 0x14, 0xB3, 0x11, 0x32, 0xC9, 0x92, 0x98,
         0x84, 0xE2, 0xC9, 0xF8, 0xD0, 0x3B, 0x6E, 0x03,
@@ -843,6 +912,11 @@ int test_wolfSSL_X509_EXTENSION_get_data(void)
 #ifndef WOLFSSL_OLD_EXTDATA_FMT
     ExpectIntEQ(str->length, sizeof (ext_data));
     ExpectBufEQ(str->data, ext_data, sizeof (ext_data));
+    /* Data is NUL terminated like any other ASN1_STRING. */
+    ExpectIntEQ(str->data[str->length], '\0');
+    ExpectIntEQ(wolfSSL_ASN1_STRING_to_UTF8(&utf8, str), sizeof (ext_data));
+    ExpectBufEQ(utf8, ext_data, sizeof (ext_data));
+    XFREE(utf8, NULL, DYNAMIC_TYPE_OPENSSL);
 #endif
 
     wolfSSL_X509_free(x509);
@@ -1798,6 +1872,37 @@ int test_wolfSSL_X509V3_EXT_print(void)
         X509_free(x509);
     }
 
+#ifdef WOLFSSL_FPKI
+    {
+        /* rid-cert.der SAN: otherName (UPN), registeredID, DNS, URI and a
+         * second otherName. With WOLFSSL_FPKI the otherName entries are put
+         * in the GENERAL_NAME stack and must not be read as an ASN1_STRING. */
+        const char* ridCert = "./certs/rid-cert.der";
+        X509* x509 = NULL;
+        BIO* bio = NULL;
+        X509_EXTENSION* ext = NULL;
+        char buf[256];
+        int loc = 0;
+        int len = 0;
+
+        ExpectNotNull(x509 = wolfSSL_X509_load_certificate_file(ridCert,
+            WOLFSSL_FILETYPE_ASN1));
+        ExpectNotNull(bio = BIO_new(BIO_s_mem()));
+
+        ExpectIntGT(loc = X509_get_ext_by_NID(x509, NID_subject_alt_name, -1),
+            -1);
+        ExpectNotNull(ext = X509_get_ext(x509, loc));
+        ExpectIntEQ(X509V3_EXT_print(bio, ext, 0, 0), 1);
+
+        XMEMSET(buf, 0, sizeof(buf));
+        ExpectIntGT(len = BIO_read(bio, buf, sizeof(buf) - 1), 0);
+        ExpectNotNull(XSTRSTR(buf, "www.example.org"));
+
+        BIO_free(bio);
+        X509_free(x509);
+    }
+#endif /* WOLFSSL_FPKI */
+
     {
         BIO* bio = NULL;
         X509_EXTENSION* ext = NULL;

tests/api/test_ossl_x509_ext.h

@@ -33,6 +33,7 @@ int test_wolfSSL_X509_add_ext(void);
 int test_wolfSSL_X509_add_ext_dirname_san_rejected(void);
 int test_wolfSSL_X509_get_ext_count(void);
 int test_wolfSSL_X509_stack_extensions(void);
+int test_wolfSSL_X509_get0_extensions_stable(void);
 int test_wolfSSL_X509_EXTENSION_new(void);
 int test_wolfSSL_X509_EXTENSION_dup(void);
 int test_wolfSSL_X509_EXTENSION_get_object(void);
@@ -71,6 +72,8 @@ int test_wolfSSL_X509_set_ext_oid_collision(void);
                       test_wolfSSL_X509_add_ext_dirname_san_rejected),         \
     TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_get_ext_count),         \
     TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_stack_extensions),      \
+    TEST_DECL_GROUP("ossl_x509_ext",                                       \
+        test_wolfSSL_X509_get0_extensions_stable),                          \
     TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_EXTENSION_new),         \
     TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_EXTENSION_dup),         \
     TEST_DECL_GROUP("ossl_x509_ext", test_wolfSSL_X509_EXTENSION_get_object),  \