Accept invalid "v3" CSRs. Although not defined, older versions of certbot use numeric value 2, which would be a hypothetical v3(2). See https://github.com/certbot/certbot/pull/9334. Accept these for compatibility. Bug: 467 Change-Id: I47cc64503569992595bdb42baa6333058d560242 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/53229 Reviewed-by: Bob Beck <bbe@google.com> Reviewed-by: Adam Langley <agl@google.com>

diff --git a/crypto/x509/t_req.c b/crypto/x509/t_req.c
index b9090b9..154fb75 100644
--- a/crypto/x509/t_req.c
+++ b/crypto/x509/t_req.c

@@ -105,7 +105,9 @@
   }
   if (!(cflag & X509_FLAG_NO_VERSION)) {
     l = X509_REQ_get_version(x);
-    assert(l == X509_REQ_VERSION_1);
+    // Only zero, |X509_REQ_VERSION_1|, is valid but our parser accepts some
+    // invalid values for compatibility.
+    assert(0 <= l && l <= 2);
     if (BIO_printf(bio, "%8sVersion: %ld (0x%lx)\n", "", l + 1,
                    (unsigned long)l) <= 0) {
       goto err;

diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index fe4a775..425d745 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc

@@ -2936,6 +2936,17 @@
 -----END CERTIFICATE REQUEST-----
 )";
 
+// kV3CSRPEM is a v3 CSR. CSR versions only go up to v1.
+static const char kV3CSRPEM[] = R"(
+-----BEGIN CERTIFICATE REQUEST-----
+MIHJMHECAQIwDzENMAsGA1UEAwwEVGVzdDBZMBMGByqGSM49AgEGCCqGSM49AwEH
+A0IABJjsayyAQod1J7UJYNT8AH4WWxLdKV0ozhrIz6hCzBAze7AqXWOSH8G+1EWC
+pSfL3oMQNtBdJS0kpXXaUqEAgTSgADAKBggqhkjOPQQDAgNIADBFAiAUXVaEYATg
+4Cc917T73KBImxh6xyhsA5pKuYpq1S4m9wIhAK+G93HR4ur7Ghel6+zUTvIAsj9e
+rsn4lSYsqI4OI4ei
+-----END CERTIFICATE REQUEST-----
+)";
+
 // Test that the library enforces versions are valid and match the fields
 // present.
 TEST(X509Test, InvalidVersion) {
@@ -2955,6 +2966,10 @@
   EXPECT_FALSE(CRLFromPEM(kV3CRLPEM));
   EXPECT_FALSE(CSRFromPEM(kV2CSRPEM));
 
+  // kV3CSRPEM is invalid but, for now, we accept it. See
+  // https://github.com/certbot/certbot/pull/9334
+  EXPECT_TRUE(CSRFromPEM(kV3CSRPEM));
+
   bssl::UniquePtr<X509> x509(X509_new());
   ASSERT_TRUE(x509);
   EXPECT_FALSE(X509_set_version(x509.get(), -1));

diff --git a/crypto/x509/x_req.c b/crypto/x509/x_req.c
index 4ddeaea..f7faf39 100644
--- a/crypto/x509/x_req.c
+++ b/crypto/x509/x_req.c

@@ -82,7 +82,11 @@
   }
 
   if (operation == ASN1_OP_D2I_POST) {
-    if (ASN1_INTEGER_get(rinf->version) != X509_REQ_VERSION_1) {
+    // The only defined CSR version is v1(0). For compatibility, we also accept
+    // a hypothetical v3(2). Although not defined, older versions of certbot
+    // use it. See https://github.com/certbot/certbot/pull/9334.
+    long version = ASN1_INTEGER_get(rinf->version);
+    if (version != X509_REQ_VERSION_1 && version != 2) {
       OPENSSL_PUT_ERROR(X509, X509_R_INVALID_VERSION);
       return 0;
     }

diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 945355f..526765f 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h

@@ -396,7 +396,9 @@
 #define X509_REQ_VERSION_1 0
 
 // X509_REQ_get_version returns the numerical value of |req|'s version. This
-// will always be |X509_REQ_VERSION_1|.
+// will always be |X509_REQ_VERSION_1| for valid CSRs. For compatibility,
+// |d2i_X509_REQ| also accepts some invalid version numbers, in which case this
+// function may return other values.
 OPENSSL_EXPORT long X509_REQ_get_version(const X509_REQ *req);
 
 // X509_REQ_get_subject_name returns |req|'s subject name. Note this function is