Check the X.509 version when parsing.
This checks the X.509 version is valid and consistent with fields new to
those versions. These checks are also implemented by Chromium's
certificate verifier and should be compatible.
Update-Note: The X.509 parser is now a bit stricter. This may break some
malformed certificates which were previously incorrectly accepted.
Bug: 348, 351
Change-Id: I56f35d768d5e72948d22a9546fba3d257a75f409
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/41746
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/err/x509.errordata b/crypto/err/x509.errordata
index 1426fbf..ffa4267 100644
--- a/crypto/err/x509.errordata
+++ b/crypto/err/x509.errordata
@@ -10,10 +10,12 @@
X509,108,IDP_MISMATCH
X509,109,INVALID_BIT_STRING_BITS_LEFT
X509,110,INVALID_DIRECTORY
+X509,139,INVALID_FIELD_FOR_VERSION
X509,111,INVALID_FIELD_NAME
X509,136,INVALID_PARAMETER
X509,112,INVALID_PSS_PARAMETERS
X509,113,INVALID_TRUST
+X509,140,INVALID_VERSION
X509,114,ISSUER_MISMATCH
X509,115,KEY_TYPE_MISMATCH
X509,116,KEY_VALUES_MISMATCH
diff --git a/crypto/x509/x509_test.cc b/crypto/x509/x509_test.cc
index 957c0b8..fa703dd 100644
--- a/crypto/x509/x509_test.cc
+++ b/crypto/x509/x509_test.cc
@@ -2312,3 +2312,133 @@
Verify(leaf.get(), {invalid_root.get()}, {intermediate.get()}, {}));
}
}
+
+// kExplicitDefaultVersionPEM is an X.509v1 certificate with the version number
+// encoded explicitly, rather than omitted as required by DER.
+static const char kExplicitDefaultVersionPEM[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBfTCCASSgAwIBAAIJANlMBNpJfb/rMAkGByqGSM49BAEwRTELMAkGA1UEBhMC\n"
+ "QVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdp\n"
+ "dHMgUHR5IEx0ZDAeFw0xNDA0MjMyMzIxNTdaFw0xNDA1MjMyMzIxNTdaMEUxCzAJ\n"
+ "BgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5l\n"
+ "dCBXaWRnaXRzIFB0eSBMdGQwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATmK2ni\n"
+ "v2Wfl74vHg2UikzVl2u3qR4NRvvdqakendy6WgHn1peoChj5w8SjHlbifINI2xYa\n"
+ "HPUdfvGULUvPciLBMAkGByqGSM49BAEDSAAwRQIhAPKgNV5ROjbDgnmb7idQhY5w\n"
+ "BnSVV9IpdAD0vhWHXcQHAiB8HnkUaiGD8Hp0aHlfFJmaaLTxy54VXuYfMlJhXnXJ\n"
+ "FA==\n"
+ "-----END CERTIFICATE-----\n";
+
+// kNegativeVersionPEM is an X.509 certificate with a negative version number.
+static const char kNegativeVersionPEM[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBfTCCASSgAwIB/wIJANlMBNpJfb/rMAkGByqGSM49BAEwRTELMAkGA1UEBhMC\n"
+ "QVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdp\n"
+ "dHMgUHR5IEx0ZDAeFw0xNDA0MjMyMzIxNTdaFw0xNDA1MjMyMzIxNTdaMEUxCzAJ\n"
+ "BgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5l\n"
+ "dCBXaWRnaXRzIFB0eSBMdGQwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATmK2ni\n"
+ "v2Wfl74vHg2UikzVl2u3qR4NRvvdqakendy6WgHn1peoChj5w8SjHlbifINI2xYa\n"
+ "HPUdfvGULUvPciLBMAkGByqGSM49BAEDSAAwRQIhAPKgNV5ROjbDgnmb7idQhY5w\n"
+ "BnSVV9IpdAD0vhWHXcQHAiB8HnkUaiGD8Hp0aHlfFJmaaLTxy54VXuYfMlJhXnXJ\n"
+ "FA==\n"
+ "-----END CERTIFICATE-----\n";
+
+// kFutureVersionPEM is an X.509 certificate with a version number value of
+// three, which is not defined. (v3 has value two).
+static const char kFutureVersionPEM[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBfTCCASSgAwIBAwIJANlMBNpJfb/rMAkGByqGSM49BAEwRTELMAkGA1UEBhMC\n"
+ "QVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdp\n"
+ "dHMgUHR5IEx0ZDAeFw0xNDA0MjMyMzIxNTdaFw0xNDA1MjMyMzIxNTdaMEUxCzAJ\n"
+ "BgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5l\n"
+ "dCBXaWRnaXRzIFB0eSBMdGQwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATmK2ni\n"
+ "v2Wfl74vHg2UikzVl2u3qR4NRvvdqakendy6WgHn1peoChj5w8SjHlbifINI2xYa\n"
+ "HPUdfvGULUvPciLBMAkGByqGSM49BAEDSAAwRQIhAPKgNV5ROjbDgnmb7idQhY5w\n"
+ "BnSVV9IpdAD0vhWHXcQHAiB8HnkUaiGD8Hp0aHlfFJmaaLTxy54VXuYfMlJhXnXJ\n"
+ "FA==\n"
+ "-----END CERTIFICATE-----\n";
+
+// kOverflowVersionPEM is an X.509 certificate with a version field which
+// overflows |uint64_t|.
+static const char kOverflowVersionPEM[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBoDCCAUegJgIkAP//////////////////////////////////////////////\n"
+ "AgkA2UwE2kl9v+swCQYHKoZIzj0EATBFMQswCQYDVQQGEwJBVTETMBEGA1UECAwK\n"
+ "U29tZS1TdGF0ZTEhMB8GA1UECgwYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMB4X\n"
+ "DTE0MDQyMzIzMjE1N1oXDTE0MDUyMzIzMjE1N1owRTELMAkGA1UEBhMCQVUxEzAR\n"
+ "BgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdpdHMgUHR5\n"
+ "IEx0ZDBZMBMGByqGSM49AgEGCCqGSM49AwEHA0IABOYraeK/ZZ+Xvi8eDZSKTNWX\n"
+ "a7epHg1G+92pqR6d3LpaAefWl6gKGPnDxKMeVuJ8g0jbFhoc9R1+8ZQtS89yIsEw\n"
+ "CQYHKoZIzj0EAQNIADBFAiEA8qA1XlE6NsOCeZvuJ1CFjnAGdJVX0il0APS+FYdd\n"
+ "xAcCIHweeRRqIYPwenRoeV8UmZpotPHLnhVe5h8yUmFedckU\n"
+ "-----END CERTIFICATE-----\n";
+
+// kV1WithExtensionsPEM is an X.509v1 certificate with extensions.
+static const char kV1WithExtensionsPEM[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIByjCCAXECCQDZTATaSX2/6zAJBgcqhkjOPQQBMEUxCzAJBgNVBAYTAkFVMRMw\n"
+ "EQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0\n"
+ "eSBMdGQwHhcNMTQwNDIzMjMyMTU3WhcNMTQwNTIzMjMyMTU3WjBFMQswCQYDVQQG\n"
+ "EwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50ZXJuZXQgV2lk\n"
+ "Z2l0cyBQdHkgTHRkMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE5itp4r9ln5e+\n"
+ "Lx4NlIpM1Zdrt6keDUb73ampHp3culoB59aXqAoY+cPEox5W4nyDSNsWGhz1HX7x\n"
+ "lC1Lz3IiwaNQME4wHQYDVR0OBBYEFKuE0qyrlfCCThZ4B1VXX+QmjYLRMB8GA1Ud\n"
+ "IwQYMBaAFKuE0qyrlfCCThZ4B1VXX+QmjYLRMAwGA1UdEwQFMAMBAf8wCQYHKoZI\n"
+ "zj0EAQNIADBFAiEA8qA1XlE6NsOCeZvuJ1CFjnAGdJVX0il0APS+FYddxAcCIHwe\n"
+ "eRRqIYPwenRoeV8UmZpotPHLnhVe5h8yUmFedckU\n"
+ "-----END CERTIFICATE-----\n";
+
+// kV2WithExtensionsPEM is an X.509v2 certificate with extensions.
+static const char kV2WithExtensionsPEM[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBzzCCAXagAwIBAQIJANlMBNpJfb/rMAkGByqGSM49BAEwRTELMAkGA1UEBhMC\n"
+ "QVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoMGEludGVybmV0IFdpZGdp\n"
+ "dHMgUHR5IEx0ZDAeFw0xNDA0MjMyMzIxNTdaFw0xNDA1MjMyMzIxNTdaMEUxCzAJ\n"
+ "BgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5l\n"
+ "dCBXaWRnaXRzIFB0eSBMdGQwWTATBgcqhkjOPQIBBggqhkjOPQMBBwNCAATmK2ni\n"
+ "v2Wfl74vHg2UikzVl2u3qR4NRvvdqakendy6WgHn1peoChj5w8SjHlbifINI2xYa\n"
+ "HPUdfvGULUvPciLBo1AwTjAdBgNVHQ4EFgQUq4TSrKuV8IJOFngHVVdf5CaNgtEw\n"
+ "HwYDVR0jBBgwFoAUq4TSrKuV8IJOFngHVVdf5CaNgtEwDAYDVR0TBAUwAwEB/zAJ\n"
+ "BgcqhkjOPQQBA0gAMEUCIQDyoDVeUTo2w4J5m+4nUIWOcAZ0lVfSKXQA9L4Vh13E\n"
+ "BwIgfB55FGohg/B6dGh5XxSZmmi08cueFV7mHzJSYV51yRQ=\n"
+ "-----END CERTIFICATE-----\n";
+
+// kV1WithIssuerUniqueIDPEM is an X.509v1 certificate with an issuerUniqueID.
+static const char kV1WithIssuerUniqueIDPEM[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBgzCCASoCCQDZTATaSX2/6zAJBgcqhkjOPQQBMEUxCzAJBgNVBAYTAkFVMRMw\n"
+ "EQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0\n"
+ "eSBMdGQwHhcNMTQwNDIzMjMyMTU3WhcNMTQwNTIzMjMyMTU3WjBFMQswCQYDVQQG\n"
+ "EwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50ZXJuZXQgV2lk\n"
+ "Z2l0cyBQdHkgTHRkMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE5itp4r9ln5e+\n"
+ "Lx4NlIpM1Zdrt6keDUb73ampHp3culoB59aXqAoY+cPEox5W4nyDSNsWGhz1HX7x\n"
+ "lC1Lz3IiwYEJAAEjRWeJq83vMAkGByqGSM49BAEDSAAwRQIhAPKgNV5ROjbDgnmb\n"
+ "7idQhY5wBnSVV9IpdAD0vhWHXcQHAiB8HnkUaiGD8Hp0aHlfFJmaaLTxy54VXuYf\n"
+ "MlJhXnXJFA==\n"
+ "-----END CERTIFICATE-----\n";
+
+// kV1WithSubjectUniqueIDPEM is an X.509v1 certificate with an issuerUniqueID.
+static const char kV1WithSubjectUniqueIDPEM[] =
+ "-----BEGIN CERTIFICATE-----\n"
+ "MIIBgzCCASoCCQDZTATaSX2/6zAJBgcqhkjOPQQBMEUxCzAJBgNVBAYTAkFVMRMw\n"
+ "EQYDVQQIDApTb21lLVN0YXRlMSEwHwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0\n"
+ "eSBMdGQwHhcNMTQwNDIzMjMyMTU3WhcNMTQwNTIzMjMyMTU3WjBFMQswCQYDVQQG\n"
+ "EwJBVTETMBEGA1UECAwKU29tZS1TdGF0ZTEhMB8GA1UECgwYSW50ZXJuZXQgV2lk\n"
+ "Z2l0cyBQdHkgTHRkMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAE5itp4r9ln5e+\n"
+ "Lx4NlIpM1Zdrt6keDUb73ampHp3culoB59aXqAoY+cPEox5W4nyDSNsWGhz1HX7x\n"
+ "lC1Lz3IiwYIJAAEjRWeJq83vMAkGByqGSM49BAEDSAAwRQIhAPKgNV5ROjbDgnmb\n"
+ "7idQhY5wBnSVV9IpdAD0vhWHXcQHAiB8HnkUaiGD8Hp0aHlfFJmaaLTxy54VXuYf\n"
+ "MlJhXnXJFA==\n"
+ "-----END CERTIFICATE-----\n";
+
+// Test that the X.509 parser enforces versions are valid and match the fields
+// present.
+TEST(X509Test, InvalidVersion) {
+ EXPECT_FALSE(CertFromPEM(kExplicitDefaultVersionPEM));
+ EXPECT_FALSE(CertFromPEM(kNegativeVersionPEM));
+ EXPECT_FALSE(CertFromPEM(kFutureVersionPEM));
+ EXPECT_FALSE(CertFromPEM(kOverflowVersionPEM));
+ EXPECT_FALSE(CertFromPEM(kV1WithExtensionsPEM));
+ EXPECT_FALSE(CertFromPEM(kV2WithExtensionsPEM));
+ EXPECT_FALSE(CertFromPEM(kV1WithIssuerUniqueIDPEM));
+ EXPECT_FALSE(CertFromPEM(kV1WithSubjectUniqueIDPEM));
+}
diff --git a/crypto/x509/x_x509.c b/crypto/x509/x_x509.c
index 9ece062..010b625 100644
--- a/crypto/x509/x_x509.c
+++ b/crypto/x509/x_x509.c
@@ -115,11 +115,37 @@
ret->buf = NULL;
break;
- case ASN1_OP_D2I_POST:
- if (ret->name != NULL)
- OPENSSL_free(ret->name);
+ 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. */
+ long version = 0;
+ if (ret->cert_info->version != NULL) {
+ version = ASN1_INTEGER_get(ret->cert_info->version);
+ if (version <= 0 || version > 2) {
+ OPENSSL_PUT_ERROR(X509, X509_R_INVALID_VERSION);
+ return 0;
+ }
+ }
+
+ /* Per RFC5280, section 4.1.2.8, these fields require v2 or v3. */
+ if (version == 0 && (ret->cert_info->issuerUID != NULL ||
+ ret->cert_info->subjectUID != NULL)) {
+ OPENSSL_PUT_ERROR(X509, X509_R_INVALID_FIELD_FOR_VERSION);
+ return 0;
+ }
+
+ /* Per RFC5280, section 4.1.2.9, extensions require v3. */
+ if (version != 2 && ret->cert_info->extensions != NULL) {
+ OPENSSL_PUT_ERROR(X509, X509_R_INVALID_FIELD_FOR_VERSION);
+ return 0;
+ }
+
+ /* TODO(davidben): Remove this field once the few external accesses are
+ * removed. */
+ OPENSSL_free(ret->name);
ret->name = X509_NAME_oneline(ret->cert_info->subject, NULL, 0);
break;
+ }
case ASN1_OP_FREE_POST:
CRYPTO_MUTEX_cleanup(&ret->lock);
diff --git a/include/openssl/x509.h b/include/openssl/x509.h
index 252946f..3a0391a 100644
--- a/include/openssl/x509.h
+++ b/include/openssl/x509.h
@@ -1204,5 +1204,7 @@
#define X509_R_INVALID_PARAMETER 136
#define X509_R_SIGNATURE_ALGORITHM_MISMATCH 137
#define X509_R_DELTA_CRL_WITHOUT_CRL_NUMBER 138
+#define X509_R_INVALID_FIELD_FOR_VERSION 139
+#define X509_R_INVALID_VERSION 140
#endif
