From: Rich Salz <rsalz@akamai.com>
Date: Tue, 6 Jul 2021 16:00:19 +0000 (-0400)
Subject: Fix bug in X509_print_ex
X-Git-Tag: openssl-3.0.0-beta2~100
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=02db7354fe7;p=thirdparty%2Fopenssl.git

Fix bug in X509_print_ex

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)
---

diff --git a/crypto/x509/t_x509.c b/crypto/x509/t_x509.c
index fdbdfd5b09d..69b04e74ebf 100644
--- a/crypto/x509/t_x509.c
+++ b/crypto/x509/t_x509.c
@@ -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;