From: tb <tb@openbsd.org>
Date: Tue, 26 Mar 2024 11:09:37 +0000 (+0000)
Subject: Reject setting invalid versions for certs, CRLs and CSRs
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=ca68b301c7505e89918ca84314f26f74fdf649fa;p=openbsd

Reject setting invalid versions for certs, CRLs and CSRs

The toolkit aspect bites again. Lots of invalid CRLs and CSRs are produced
because people neither read the RFCs nor does the toolkit check anything it
is fed. Reviewers apparently also aren't capable of remembering that they
have three copy-pasted versions of the same API and that adding a version
check to one of the might suggest adding one for the other two.

This requires ruby-openssl 20240326p0 to pass

ok beck job jsing
---

diff --git a/lib/libcrypto/x509/x509_set.c b/lib/libcrypto/x509/x509_set.c
index b56d30aec59..684a899781f 100644
--- a/lib/libcrypto/x509/x509_set.c
+++ b/lib/libcrypto/x509/x509_set.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509_set.c,v 1.26 2023/06/23 08:00:28 tb Exp $ */
+/* $OpenBSD: x509_set.c,v 1.27 2024/03/26 11:09:37 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -84,6 +84,12 @@ X509_set_version(X509 *x, long version)
 {
 	if (x == NULL)
 		return (0);
+	/*
+	 * RFC 5280, 4.1: versions 1 - 3 are specified as follows.
+	 * Version  ::=  INTEGER  {  v1(0), v2(1), v3(2) }
+	 */
+	if (version < 0 || version > 2)
+		return (0);
 	if (x->cert_info->version == NULL) {
 		if ((x->cert_info->version = ASN1_INTEGER_new()) == NULL)
 			return (0);
diff --git a/lib/libcrypto/x509/x509cset.c b/lib/libcrypto/x509/x509cset.c
index 7904a7d6700..a80d5f21d3b 100644
--- a/lib/libcrypto/x509/x509cset.c
+++ b/lib/libcrypto/x509/x509cset.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509cset.c,v 1.19 2023/02/16 08:38:17 tb Exp $ */
+/* $OpenBSD: x509cset.c,v 1.20 2024/03/26 11:09:37 tb Exp $ */
 /* Written by Dr Stephen N Henson (steve@openssl.org) for the OpenSSL
  * project 2001.
  */
@@ -78,6 +78,12 @@ X509_CRL_set_version(X509_CRL *x, long version)
 {
 	if (x == NULL)
 		return (0);
+	/*
+	 * RFC 5280, 4.1: versions 1 - 3 are specified as follows.
+	 * Version  ::=  INTEGER  {  v1(0), v2(1), v3(2) }
+	 */
+	if (version < 0 || version > 1)
+		return (0);
 	if (x->crl->version == NULL) {
 		if ((x->crl->version = ASN1_INTEGER_new()) == NULL)
 			return (0);
diff --git a/lib/libcrypto/x509/x509rset.c b/lib/libcrypto/x509/x509rset.c
index b05b2a1c91c..6ac64f199dc 100644
--- a/lib/libcrypto/x509/x509rset.c
+++ b/lib/libcrypto/x509/x509rset.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: x509rset.c,v 1.14 2024/03/25 12:10:57 jsing Exp $ */
+/* $OpenBSD: x509rset.c,v 1.15 2024/03/26 11:09:37 tb Exp $ */
 /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com)
  * All rights reserved.
  *
@@ -70,6 +70,9 @@ X509_REQ_set_version(X509_REQ *x, long version)
 {
 	if (x == NULL)
 		return (0);
+	/* RFC 2986 section 4.1 only specifies version 1, encoded as a 0. */
+	if (version != 0)
+		return (0);
 	x->req_info->enc.modified = 1;
 	return (ASN1_INTEGER_set(x->req_info->version, version));
 }