Fix bug in X509_print_ex
authorRich Salz <rsalz@akamai.com>
Tue, 6 Jul 2021 16:00:19 +0000 (12:00 -0400)
committerPauli <pauli@openssl.org>
Thu, 8 Jul 2021 01:46:19 +0000 (11:46 +1000)
If the user set nmflags == XN_FLAG_COMPAT and X509_NAME_print_ex(3)
failed, the error return value of 0 was misinterpreted as an indicator
of success, causing X509_print_ex(3) to ignore the error, continue
printing, and potentially return successfully even though not all
the content of the certificate was printed.

The X509_NAME_print_ex(3) manual page explains that this function
indicates failure by returning 0 if nmflags == XN_FLAG_COMPAT
and by returning -1 if nmflags != XN_FLAG_COMPAT.

Note that just checking for <= 0 in all cases would not be correct
either because X509_NAME_print_ex(3) returns 0 to indicate that it
successfully printed zero bytes in some cases, for example when all
three of the following conditions hold:
1. nmflags != XN_FLAG_COMPAT
2. indent == 0 (which X509_print_ex(3) does use in some cases)
3. the name object is NULL or empty

Thanks to Ingo Schwarze <schwarze@openbsd.org> for finding the bug,
and Joel Sing <jsing@openbsd.org> for contributing an idea for the
fix.

Reviewed-by: Ben Kaduk <kaduk@mit.edu>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16009)

crypto/x509/t_x509.c

index fdbdfd5b09da32c4763db1c69193070f048c2686..69b04e74ebffe8a3af79a93c8377dbf94d520218 100644 (file)
@@ -51,7 +51,7 @@ int X509_print_ex(BIO *bp, X509 *x, unsigned long nmflags,
     long l;
     int ret = 0, i;
     char *m = NULL, mlch = ' ';
-    int nmindent = 0;
+    int nmindent = 0, printok = 0;
     EVP_PKEY *pkey = NULL;
     const char *neg;
 
@@ -60,8 +60,10 @@ int X509_print_ex(BIO *bp, X509 *x, unsigned long nmflags,
         nmindent = 12;
     }
 
-    if (nmflags == X509_FLAG_COMPAT)
+    if (nmflags == X509_FLAG_COMPAT) {
         nmindent = 16;
+        printok = 1;
+    }
 
     if (!(cflag & X509_FLAG_NO_HEADER)) {
         if (BIO_write(bp, "Certificate:\n", 13) <= 0)
@@ -130,7 +132,7 @@ int X509_print_ex(BIO *bp, X509 *x, unsigned long nmflags,
         if (BIO_printf(bp, "        Issuer:%c", mlch) <= 0)
             goto err;
         if (X509_NAME_print_ex(bp, X509_get_issuer_name(x), nmindent, nmflags)
-            < 0)
+            < printok)
             goto err;
         if (BIO_write(bp, "\n", 1) <= 0)
             goto err;
@@ -153,7 +155,7 @@ int X509_print_ex(BIO *bp, X509 *x, unsigned long nmflags,
         if (BIO_printf(bp, "        Subject:%c", mlch) <= 0)
             goto err;
         if (X509_NAME_print_ex
-            (bp, X509_get_subject_name(x), nmindent, nmflags) < 0)
+            (bp, X509_get_subject_name(x), nmindent, nmflags) < printok)
             goto err;
         if (BIO_write(bp, "\n", 1) <= 0)
             goto err;