Also accept incorrectly-encoded X509 v1 version fields again

This reverts another part of
https://boringssl-review.googlesource.com/c/boringssl/+/82087

Sadly this hit another real-world input. We'll need to use our new
parser to wire in a X509_parse_with_options or something of the sort, so
that the impacted code can pass in arguments to reject a handful of
non-certificate variations.

Bug: b:449478579, 42290225
Change-Id: I1e39c8b6ed0ab454bf61c85edffe6631e8c7c3df
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/82747
Auto-Submit: 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 063ae8e..de67e41 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -3840,7 +3840,9 @@
 // Test that the library 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/42290225.
+  EXPECT_TRUE(CertFromPEM(kExplicitDefaultVersionPEM));
   EXPECT_FALSE(CRLFromPEM(kExplicitDefaultVersionCRLPEM));
   EXPECT_FALSE(CertFromPEM(kNegativeVersionPEM));
   EXPECT_FALSE(CertFromPEM(kFutureVersionPEM));
@@ -8988,6 +8990,9 @@
       "crypto/x509/test/unusual_tbs_uid_both.pem",
       "crypto/x509/test/unusual_tbs_uid_issuer.pem",
       "crypto/x509/test/unusual_tbs_uid_subject.pem",
+      // A v1 version is explicit encoded instead of omitted as DEFAULT.
+      // TODO(crbug.com/42290225): The parser should reject this.
+      "crypto/x509/test/unusual_tbs_v1_not_omitted.pem",
       // Within a RelativeDistinguishedName, attributes should be sorted in
       // canonical SET OF order. These are inverted.
       // TODO(crbug.com/42290219): The parser should reject this.
@@ -8999,18 +9004,6 @@
     ASSERT_TRUE(cert);
     EXPECT_TRUE(X509_verify(cert.get(), key.get()));
   }
-
-  // The following inputs were once accepted, and thus preserved in signature
-  // verification, but we no longer parse them at all.
-  const char *kInvalidPaths[] = {
-      // A v1 version is explicit encoded instead of omitted as DEFAULT.
-      "crypto/x509/test/unusual_tbs_v1_not_omitted.pem",
-  };
-  for (const char *path : kInvalidPaths) {
-    SCOPED_TRACE(path);
-    bssl::UniquePtr<X509> cert = CertFromPEM(GetTestData(path));
-    EXPECT_FALSE(cert);
-  }
 }
 
 TEST(X509Test, TrailingDataX509) {

crypto/x509/x_x509.cc

diff --git a/crypto/x509/x_x509.cc b/crypto/x509/x_x509.cc
index a572042..bf5ac24 100644
--- a/crypto/x509/x_x509.cc
+++ b/crypto/x509/x_x509.cc
@@ -137,9 +137,11 @@
       OPENSSL_PUT_ERROR(ASN1, ASN1_R_DECODE_ERROR);
       return nullptr;
     }
-    // Versions v1, v2, and v3 are defined. v1 is DEFAULT, so cannot be encoded
-    // explicitly.
-    if (version != X509_VERSION_2 && version != X509_VERSION_3) {
+    // The version must be one of v1(0), v2(1), or v3(2).
+    // TODO(https://crbug.com/42290225): Also reject |X509_VERSION_1|. v1 is
+    // DEFAULT, so DER requires it be omitted.
+    if (version != X509_VERSION_1 && version != X509_VERSION_2 &&
+        version != X509_VERSION_3) {
       OPENSSL_PUT_ERROR(X509, X509_R_INVALID_VERSION);
       return nullptr;
     }