diff options
author | Rich Salz <rsalz@akamai.com> | 2021-07-06 12:00:19 -0400 |
---|---|---|
committer | Pauli <pauli@openssl.org> | 2021-07-08 11:46:19 +1000 |
commit | 02db7354fe780c8bfb34b3f0ed73595cb9ac6570 (patch) | |
tree | deaa665d345edee5c99b008957c75afe33ccf70c | |
parent | 3bd5319b5d0df9ecf05c8baba2c401ad8e3ba130 (diff) |
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)
-rw-r--r-- | crypto/x509/t_x509.c | 10 |
1 files changed, 6 insertions, 4 deletions
diff --git a/crypto/x509/t_x509.c b/crypto/x509/t_x509.c index fdbdfd5b09..69b04e74eb 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; |