commit de196121b05adea2f9b7e400271266495a40b0f4
author David Benjamin <davidben@google.com> Thu Jul 09 12:01:51 2020 -0400
committer CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org> Thu Jul 09 16:28:02 2020 +0000
tree e319c2343369185bd7a4aae24b7881b2b9260b0c
parent eda849d2e6e6a15a5a4dc728568ec12f21ebfb6d

Allow explicitly-encoded X.509v1 versions for now.

Sadly, we need to roll this one back for now, at least until we've
cleared all the test failures it causes. This retains the other checks
in https://boringssl-review.googlesource.com/c/boringssl/+/41746. We're
only rolling back enforcement of the DEFAULT v1 encoding.

Bug: 348, 364
Change-Id: I6a290311f5a5714ff4d5add3ae35ec4550398b32
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/42104
Commit-Queue: David Benjamin <davidben@google.com>
Commit-Queue: Adam Langley <agl@google.com>
Reviewed-by: Adam Langley <agl@google.com>

crypto/x509/x509_test.cc

diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 366e66e..38c9bc5 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -2440,7 +2440,10 @@
 // Test that the X.509 parser enforces versions are valid and match the fields
 // present.
 TEST(X509Test, InvalidVersion) {
-  EXPECT_FALSE(CertFromPEM(kExplicitDefaultVersionPEM));
+  // kExplicitDefaultVersionPEM is invalid but, for now, we accept it. See
+  // https://crbug.com/boringssl/364.
+  EXPECT_TRUE(CertFromPEM(kExplicitDefaultVersionPEM));
+
   EXPECT_FALSE(CertFromPEM(kNegativeVersionPEM));
   EXPECT_FALSE(CertFromPEM(kFutureVersionPEM));
   EXPECT_FALSE(CertFromPEM(kOverflowVersionPEM));

crypto/x509/x_x509.c

diff --git a/crypto/x509/x_x509.c b/crypto/x509/x_x509.c
index ff0dc34..cddceb8 100644
--- a/crypto/x509/x_x509.c
+++ b/crypto/x509/x_x509.c
@@ -115,12 +115,14 @@
         break;
 
     case ASN1_OP_D2I_POST: {
-        /* The version must be one of v1(0), v2(1), or v3(2). If the version is
-         * v1(0), it must be omitted because it is DEFAULT. */
+        /* The version must be one of v1(0), v2(1), or v3(2). */
         long version = 0;
         if (ret->cert_info->version != NULL) {
             version = ASN1_INTEGER_get(ret->cert_info->version);
-            if (version <= 0 || version > 2) {
+            /* TODO(https://crbug.com/boringssl/364): |version| = 0 should also
+             * be rejected. This means an explicitly-encoded X.509v1 version.
+             * v1 is DEFAULT, so DER requires it be omitted. */
+            if (version < 0 || version > 2) {
                 OPENSSL_PUT_ERROR(X509, X509_R_INVALID_VERSION);
                 return 0;
             }