3 files changed, 125 insertions, 22 deletions

diff --git a/src/lib/libcrypto/man/X509_NAME_get_index_by_NID.3 b/src/lib/libcrypto/man/X509_NAME_get_index_by_NID.3
index 20730fb52a..a2ceb10eb5 100644
--- a/src/lib/libcrypto/man/X509_NAME_get_index_by_NID.3
+++ b/src/lib/libcrypto/man/X509_NAME_get_index_by_NID.3
@@ -1,4 +1,4 @@
-.\"     $OpenBSD: X509_NAME_get_index_by_NID.3,v 1.15 2023/05/03 08:10:23 beck Exp $
+.\"     $OpenBSD: X509_NAME_get_index_by_NID.3,v 1.16 2023/05/29 11:54:50 beck Exp $
 .\"     OpenSSL aebb9aac Jul 19 09:27:53 2016 -0400
 .\"
 .\" This file was written by Dr. Stephen Henson <steve@openssl.org>.
@@ -49,7 +49,7 @@
 .\" ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
 .\" OF THE POSSIBILITY OF SUCH DAMAGE.
 .\"
-.Dd $Mdocdate: May 3 2023 $
+.Dd $Mdocdate: May 29 2023 $
 .Dt X509_NAME_GET_INDEX_BY_NID 3
 .Os
 .Sh NAME
@@ -136,22 +136,32 @@ run from 0 to
 .Fn X509_NAME_get_text_by_NID
 and
 .Fn X509_NAME_get_text_by_OBJ
-retrieve the "text" from the first entry in
+retrieve the bytes encoded as UTF-8 from the first entry in
 .Fa name
 which matches
 .Fa nid
 or
 .Fa obj .
-At most
-.Fa len
-bytes will be written and the text written to
-.Fa buf
-will be NUL terminated.
 If
 .Fa buf
 is
 .Dv NULL ,
 nothing is written, but the return value is calculated as usual.
+If
+.Fa buf
+is not
+.Dv NULL ,
+no more than
+.Fa len
+bytes will be written and the text written to
+.Fa buf
+will be NUL terminated.
+.Pp
+If
+.Fa len
+is not large enough to hold the NUL byte terminated UTF-8 encoding of
+the text, or if the UTF-8 encoding of the text would contains a NUL
+byte, no data will be written and the call will return failure.
 .Pp
 All relevant
 .Dv NID_*
@@ -189,8 +199,8 @@ if the index is invalid.
 .Fn X509_NAME_get_text_by_NID
 and
 .Fn X509_NAME_get_text_by_OBJ
-return the length of the output string written, not counting the
-terminating NUL, or -1 if no match is found.
+return the length of the output UTF-8 string written, not counting the
+terminating NUL, or -1 in the case of an error or no match being found.
 .Pp
 In some cases of failure of
 .Fn X509_NAME_get_index_by_NID
diff --git a/src/lib/libcrypto/x509/x509name.c b/src/lib/libcrypto/x509/x509name.c
index ecdf473ea9..d2df06ccc6 100644
--- a/src/lib/libcrypto/x509/x509name.c
+++ b/src/lib/libcrypto/x509/x509name.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509name.c,v 1.34 2023/05/03 08:10:23 beck Exp $ */
+/* $OpenBSD: x509name.c,v 1.35 2023/05/29 11:54:50 beck Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -66,6 +66,7 @@
 #include <openssl/stack.h>
 #include <openssl/x509.h>
 
+#include "bytestring.h"
 #include "x509_local.h"
 
 int
@@ -84,21 +85,38 @@ int
 X509_NAME_get_text_by_OBJ(X509_NAME *name, const ASN1_OBJECT *obj, char *buf,
     int len)
 {
-        int i;
+        unsigned char *text = NULL;
         ASN1_STRING *data;
+        int i, text_len;
+        int ret = -1;
+        CBS cbs;
 
         i = X509_NAME_get_index_by_OBJ(name, obj, -1);
         if (i < 0)
-                return (-1);
+                goto err;
         data = X509_NAME_ENTRY_get_data(X509_NAME_get_entry(name, i));
-        i = (data->length > (len - 1)) ? (len - 1) : data->length;
-        if (buf == NULL)
-                return (data->length);
-        if (i >= 0) {
-                memcpy(buf, data->data, i);
-                buf[i] = '\0';
+        /*
+         * Fail if we cannot encode as UTF-8, or if the UTF-8 encoding of the
+         * string contains a 0 byte, because mortal callers seldom handle the
+         * length difference correctly.
+         */
+        if ((text_len = ASN1_STRING_to_UTF8(&text, data)) < 0)
+                goto err;
+        CBS_init(&cbs, text, text_len);
+        if (CBS_contains_zero_byte(&cbs))
+                goto err;
+        /* We still support the "pass NULL to find out how much" API */
+        if (buf != NULL) {
+                if (len <= 0 || !CBS_write_bytes(&cbs, buf, len - 1, NULL))
+                        goto err;
+                /* It must be a C string */
+                buf[text_len] = '\0';
         }
-        return (i);
+        ret = text_len;
+
+ err:
+        free(text);
+        return (ret);
 }
 LCRYPTO_ALIAS(X509_NAME_get_text_by_OBJ);
 
diff --git a/src/regress/lib/libcrypto/x509/x509_asn1.c b/src/regress/lib/libcrypto/x509/x509_asn1.c
index d96a51880e..4c038c9bbf 100644
--- a/src/regress/lib/libcrypto/x509/x509_asn1.c
+++ b/src/regress/lib/libcrypto/x509/x509_asn1.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_asn1.c,v 1.18 2023/05/03 08:10:23 beck Exp $ */
+/* $OpenBSD: x509_asn1.c,v 1.19 2023/05/29 11:54:50 beck Exp $ */
 /*
  * Copyright (c) 2023 Job Snijders <job@openbsd.org>
  *
@@ -512,13 +512,88 @@ test_x509_req_setters(void)
         return failed;
 }
 
-int main(void)
+static const struct testcase {
+        char *data;
+        int len;
+        int len_to_pass;
+        int encode_type;
+        int expected_result;
+        char *expected_string;
+} testCases[] = {
+        /* should work */
+        {"fozzie", 6, 80, MBSTRING_ASC, 6, "fozzie"},
+        /* should work */
+        {"fozzie", 6, -1, MBSTRING_ASC, 6, ""},
+        /* should fail, truncation */
+        {"muppet", 6, 5, MBSTRING_ASC, -1, ""},
+        /* should fail, contains 0 byte */
+        {"g\0nzo", 5, 80, MBSTRING_ASC, -1, ""},
+        /* should fail, can't encode as utf-8 */
+        {"\x30\x00", 2, 80, V_ASN1_SEQUENCE, -1, ""},
+};
+
+#define NUM_TEST_CASES (sizeof(testCases) / sizeof(testCases[0]))
+
+static int
+test_x509_name_get(void)
+{
+        int failed = 0;
+        size_t i;
+
+        for (i = 0; i < NUM_TEST_CASES; i++) {
+                const struct testcase *test = testCases + i;
+                X509_NAME_ENTRY *entry = NULL;
+                X509_NAME *name = NULL;
+                char textbuf[80];
+                int result;
+
+                textbuf[0] = '\0';
+                if ((name = X509_NAME_new()) == NULL)
+                        err(1, "X509_NAME_new");
+                if ((entry = X509_NAME_ENTRY_new()) == NULL)
+                        err(1, "X509_NAME_ENTRY_new");
+                if (!X509_NAME_ENTRY_set_object(entry,
+                    OBJ_nid2obj(NID_commonName)))
+                        err(1, "X509_NAME_ENTRY_set_object");
+                if (!X509_NAME_ENTRY_set_data(entry, test->encode_type,
+                    test->data, test->len))
+                        err(1, "X509_NAME_ENTRY_set_data");
+                if (!X509_NAME_add_entry(name, entry, -1, 0))
+                        err(1, "X509_NAME_add_entry");
+                if (test->len_to_pass == -1)
+                        result = X509_NAME_get_text_by_NID(name, NID_commonName,
+                            NULL, 0);
+                else
+                        result = X509_NAME_get_text_by_NID(name, NID_commonName,
+                            textbuf, test->len_to_pass);
+                if (result != test->expected_result) {
+                        fprintf(stderr,
+                            "Test %zu X509_GET_text_by_NID returned %d,"
+                            "expected %d\n", i, result, test->expected_result);
+                        failed++;
+                }
+                if (result != -1 &&
+                    strcmp(test->expected_string, textbuf) != 0) {
+                        fprintf(stderr,
+                            "Test %zu, X509_GET_text_by_NID returned bytes do"
+                            "not match \n", i);
+                        failed++;
+                }
+                X509_NAME_ENTRY_free(entry);
+                X509_NAME_free(name);
+        }
+        return failed;
+}
+
+int
+main(void)
 {
         int failed = 0;
 
         failed |= test_x509_setters();
         /* failed |= */ test_x509_crl_setters();
         /* failed |= */ test_x509_req_setters();
+        failed |= test_x509_name_get();
 
         OPENSSL_cleanup();