From ca68b301c7505e89918ca84314f26f74fdf649fa Mon Sep 17 00:00:00 2001 From: tb Date: Tue, 26 Mar 2024 11:09:37 +0000 Subject: [PATCH] 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 --- lib/libcrypto/x509/x509_set.c | 8 +++++++- lib/libcrypto/x509/x509cset.c | 8 +++++++- lib/libcrypto/x509/x509rset.c | 5 ++++- 3 files changed, 18 insertions(+), 3 deletions(-) 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)); } -- 2.20.1