From: schwarze <schwarze@openbsd.org>
Date: Tue, 6 Jul 2021 11:26:25 +0000 (+0000)
Subject: Fix a bug in X509_print_ex(3).
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=64d49c38248d035f69fcf5922002cce30427815f;p=openbsd

Fix a bug in X509_print_ex(3).
If the user set nmflags == X509_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 == X509_FLAG_COMPAT
and by returning -1 if nmflags != X509_FLAG_COMPAT.
That's definitely atrocious API design (witnessed by the
complexity of the code needed for correct error checking),
but changing the API contract and becoming incompatible
with OpenSSL would make matters even worse.

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 != X509_FLAG_COMPAT
2. indent == 0 (which X509_print_ex(3) does use in some cases)
3. the name object is NULL or empty

I found the bug by code inspection and proposed an incomplete patch,
then jsing@ proposed this improved version of the patch.
OK jsing@.
---

diff --git a/lib/libcrypto/asn1/t_x509.c b/lib/libcrypto/asn1/t_x509.c
index 73a0491c003..1cef35dfca6 100644
--- a/lib/libcrypto/asn1/t_x509.c
+++ b/lib/libcrypto/asn1/t_x509.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: t_x509.c,v 1.32 2020/04/10 07:05:24 tb Exp $ */
+/* $OpenBSD: t_x509.c,v 1.33 2021/07/06 11:26:25 schwarze Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -180,7 +180,7 @@ X509_print_ex(BIO *bp, X509 *x, unsigned long nmflags, unsigned long cflag)
 		if (BIO_printf(bp, "        Issuer:%c", mlch) <= 0)
 			goto err;
 		if (X509_NAME_print_ex(bp, X509_get_issuer_name(x),
-		    nmindent, nmflags) < 0)
+		    nmindent, nmflags) < (nmflags == X509_FLAG_COMPAT ? 1 : 0))
 			goto err;
 		if (BIO_write(bp, "\n", 1) <= 0)
 			goto err;
@@ -203,7 +203,7 @@ X509_print_ex(BIO *bp, X509 *x, unsigned long nmflags, unsigned long cflag)
 		if (BIO_printf(bp, "        Subject:%c", mlch) <= 0)
 			goto err;
 		if (X509_NAME_print_ex(bp, X509_get_subject_name(x),
-		    nmindent, nmflags) < 0)
+		    nmindent, nmflags) < (nmflags == X509_FLAG_COMPAT ? 1 : 0))
 			goto err;
 		if (BIO_write(bp, "\n", 1) <= 0)
 			goto err;