Remove remnants of Netscape Server Gated Crypto from the new verifier
Per the bug, we were waiting for the launch to stick. It has now stuck,
so finish the job.
Also fix the rebase-errors.py scripts. They stopped working after the
migration.
Update-Note: SHA-1 certificates with the Netscape SGC OID will no longer
skip their EKU check in the new verifier. By default, SHA-1
certificates are rejected, in which case this only impacts error
reporting, not which certificates are ultimately accepted.
Fixed: chromium:843735
Change-Id: I44df6a0ee80625c50e0bdf03fb775df9913fd141
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/65054
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
diff --git a/pki/common_cert_errors.cc b/pki/common_cert_errors.cc
index b13bc94..00f5743 100644
--- a/pki/common_cert_errors.cc
+++ b/pki/common_cert_errors.cc
@@ -39,9 +39,6 @@
"than TBSCertificate.signature");
DEFINE_CERT_ERROR_ID(kEkuLacksServerAuth,
"The extended key usage does not include server auth");
-DEFINE_CERT_ERROR_ID(kEkuLacksServerAuthButHasGatedCrypto,
- "The extended key usage does not include server auth but "
- "instead includes Netscape Server Gated Crypto");
DEFINE_CERT_ERROR_ID(kEkuLacksServerAuthButHasAnyEKU,
"The extended key usage does not include server auth but "
"instead includes anyExtendeKeyUsage");
diff --git a/pki/common_cert_errors.h b/pki/common_cert_errors.h
index 43f2408..d2b37ba 100644
--- a/pki/common_cert_errors.h
+++ b/pki/common_cert_errors.h
@@ -108,10 +108,6 @@
// What constitutes as "acceptable" is determined by the verification delegate.
OPENSSL_EXPORT extern const CertErrorId kUnacceptablePublicKey;
-// The certificate's EKU is missing serverAuth. However Netscape Server Gated
-// Crypto is present instead.
-OPENSSL_EXPORT extern const CertErrorId kEkuLacksServerAuthButHasGatedCrypto;
-
// The certificate's EKU is missing serverAuth. However EKU ANY is present
// instead.
OPENSSL_EXPORT extern const CertErrorId kEkuLacksServerAuthButHasAnyEKU;
diff --git a/pki/extended_key_usage.h b/pki/extended_key_usage.h
index f3b9d02..ae37650 100644
--- a/pki/extended_key_usage.h
+++ b/pki/extended_key_usage.h
@@ -68,12 +68,6 @@
inline constexpr uint8_t kOCSPSigning[] = {0x2b, 0x06, 0x01, 0x05,
0x05, 0x07, 0x03, 0x09};
-// Netscape Server Gated Crypto (2.16.840.1.113730.4.1) is a deprecated OID
-// which in some situations is considered equivalent to the serverAuth key
-// purpose.
-inline constexpr uint8_t kNetscapeServerGatedCrypto[] = {
- 0x60, 0x86, 0x48, 0x01, 0x86, 0xf8, 0x42, 0x04, 0x01};
-
// Parses |extension_value|, which contains the extnValue field of an X.509v3
// Extended Key Usage extension, and populates |eku_oids| with the list of
// DER-encoded OID values (that is, without tag and length). Returns false if
diff --git a/pki/test_helpers.cc b/pki/test_helpers.cc
index b6e6385..3b1b256 100644
--- a/pki/test_helpers.cc
+++ b/pki/test_helpers.cc
@@ -479,7 +479,7 @@
<< "ACTUAL:\n\n"
<< actual_errors_str << "\n"
<< "===> Use "
- "testdata/verify_certificate_chain_unittest/"
+ "pki/testdata/verify_certificate_chain_unittest/"
"rebase-errors.py to rebaseline.\n";
}
}
@@ -497,7 +497,7 @@
<< "ACTUAL:\n\n"
<< actual_errors_str << "\n"
<< "===> Use "
- "testdata/parse_certificate_unittest/"
+ "pki/testdata/parse_certificate_unittest/"
"rebase-errors.py to rebaseline.\n";
}
}
diff --git a/pki/testdata/parse_certificate_unittest/rebase-errors.py b/pki/testdata/parse_certificate_unittest/rebase-errors.py
index 412c985..0f3d920 100755
--- a/pki/testdata/parse_certificate_unittest/rebase-errors.py
+++ b/pki/testdata/parse_certificate_unittest/rebase-errors.py
@@ -11,8 +11,8 @@
To use this run the affected tests, and then pass the input to this script
(either via stdin, or as the first argument). For instance:
- $ ./out/Release/net_unittests --gtest_filter="*ParseCertificate*" | \
- net/data/parse_certificate_unittest/rebase-errors.py
+ $ ./build/pki_test --gtest_filter="*ParseCertificate*:*ParsedCertificate*:" | \
+ pki/testdata/parse_certificate_unittest/rebase-errors.py
The script works by scanning the stdout looking for gtest failures having a
particular format. The C++ test side should have been instrumented to dump
@@ -21,17 +21,10 @@
This script will then update the corresponding .pem file
"""
-import sys
+import base64
import os
-
-script_dir = os.path.dirname(os.path.realpath(__file__))
-sys.path += [os.path.join(script_dir, '..')]
-
-import gencerts
-
-import os
-import sys
import re
+import sys
# Regular expression to find the failed errors in test stdout.
# * Group 1 of the match is file path (relative to //src) where the
@@ -46,7 +39,7 @@
ACTUAL:
((?:.|\n)*?)
-===> Use net/data/parse_certificate_unittest/rebase-errors.py to rebaseline.
+===> Use pki/testdata/parse_certificate_unittest/rebase-errors.py to rebaseline.
""", re.MULTILINE)
@@ -81,11 +74,18 @@
return original[0:start] + replacement + original[end:]
+def text_data_to_pem(block_header, text_data):
+ # b64encode takes in bytes and returns bytes.
+ pem_data = base64.b64encode(text_data.encode('utf8')).decode('utf8')
+ return '%s\n-----BEGIN %s-----\n%s\n-----END %s-----\n' % (
+ text_data, block_header, pem_data, block_header)
+
+
def fixup_pem_file(path, actual_errors):
"""Updates the ERRORS block in the test .pem file"""
contents = read_file_to_string(path)
- errors_block_text = '\n' + gencerts.text_data_to_pem('ERRORS', actual_errors)
+ errors_block_text = '\n' + text_data_to_pem('ERRORS', actual_errors)
# Strip the trailing newline.
errors_block_text = errors_block_text[:-1]
@@ -102,15 +102,17 @@
def get_src_root():
- """Returns the path to the enclosing //src directory. This assumes the
+ """Returns the path to BoringSSL source tree. This assumes the
current script is inside the source tree."""
cur_dir = os.path.dirname(os.path.realpath(__file__))
while True:
- parent_dir, dirname = os.path.split(cur_dir)
- # Check if it looks like the src/ root.
- if dirname == "src" and os.path.isdir(os.path.join(cur_dir, "net")):
+ # Check if it looks like the BoringSSL root.
+ if os.path.isdir(os.path.join(cur_dir, "crypto")) and \
+ os.path.isdir(os.path.join(cur_dir, "pki")) and \
+ os.path.isdir(os.path.join(cur_dir, "ssl")):
return cur_dir
+ parent_dir, _ = os.path.split(cur_dir)
if not parent_dir or parent_dir == cur_dir:
break
cur_dir = parent_dir
@@ -138,7 +140,7 @@
test_stdout = sys.stdin.read()
for m in failed_test_regex.finditer(test_stdout):
- src_relative_errors_path = m.group(1)
+ src_relative_errors_path = "pki/" + m.group(1)
errors_path = get_abs_path(src_relative_errors_path)
actual_errors = m.group(2)
diff --git a/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha1-eku-serverAuth-strict.test b/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha1-eku-serverAuth-strict.test
index b6a8a00..c6a7146 100644
--- a/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha1-eku-serverAuth-strict.test
+++ b/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha1-eku-serverAuth-strict.test
@@ -4,6 +4,5 @@
key_purpose: SERVER_AUTH_STRICT
expected_errors:
----- Certificate i=1 (CN=Intermediate) -----
-WARNING: The extended key usage does not include server auth but instead includes Netscape Server Gated Crypto
ERROR: The extended key usage does not include server auth
diff --git a/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha1-eku-serverAuth.test b/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha1-eku-serverAuth.test
index 682897d..4afad8b 100644
--- a/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha1-eku-serverAuth.test
+++ b/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha1-eku-serverAuth.test
@@ -4,5 +4,5 @@
key_purpose: SERVER_AUTH
expected_errors:
----- Certificate i=1 (CN=Intermediate) -----
-WARNING: The extended key usage does not include server auth but instead includes Netscape Server Gated Crypto
+ERROR: The extended key usage does not include server auth
diff --git a/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha256-eku-serverAuth-strict.test b/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha256-eku-serverAuth-strict.test
index acd23e6..75ec64f 100644
--- a/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha256-eku-serverAuth-strict.test
+++ b/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha256-eku-serverAuth-strict.test
@@ -4,6 +4,5 @@
key_purpose: SERVER_AUTH_STRICT
expected_errors:
----- Certificate i=1 (CN=Intermediate) -----
-WARNING: The extended key usage does not include server auth but instead includes Netscape Server Gated Crypto
ERROR: The extended key usage does not include server auth
diff --git a/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha256-eku-serverAuth.test b/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha256-eku-serverAuth.test
index 1d3623a..f64c12b 100644
--- a/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha256-eku-serverAuth.test
+++ b/pki/testdata/verify_certificate_chain_unittest/intermediate-eku-server-gated-crypto/sha256-eku-serverAuth.test
@@ -4,6 +4,5 @@
key_purpose: SERVER_AUTH
expected_errors:
----- Certificate i=1 (CN=Intermediate) -----
-WARNING: The extended key usage does not include server auth but instead includes Netscape Server Gated Crypto
ERROR: The extended key usage does not include server auth
diff --git a/pki/testdata/verify_certificate_chain_unittest/rebase-errors.py b/pki/testdata/verify_certificate_chain_unittest/rebase-errors.py
index 3096c8c..7a5e868 100755
--- a/pki/testdata/verify_certificate_chain_unittest/rebase-errors.py
+++ b/pki/testdata/verify_certificate_chain_unittest/rebase-errors.py
@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
# Copyright 2016 The Chromium Authors
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
@@ -11,8 +11,8 @@
To use this run the affected tests, and then pass the input to this script
(either via stdin, or as the first argument). For instance:
- $ ./out/Release/net_unittests --gtest_filter="*VerifyCertificateChain*" | \
- net/data/verify_certificate_chain_unittest/rebase-errors.py
+ $ ./build/pki_test --gtest_filter="*VerifyCertificateChain*" | \
+ pki/testdata/verify_certificate_chain_unittest/rebase-errors.py
The script works by scanning the stdout looking for gtest failures having a
particular format. The C++ test side should have been instrumented to dump out
@@ -39,7 +39,7 @@
ACTUAL:
((?:.|\n)*?)
-===> Use net/data/verify_certificate_chain_unittest/rebase-errors.py to rebaseline.
+===> Use pki/testdata/verify_certificate_chain_unittest/rebase-errors.py to rebaseline.
""", re.MULTILINE)
@@ -57,15 +57,17 @@
def get_src_root():
- """Returns the path to the enclosing //src directory. This assumes the
+ """Returns the path to BoringSSL source tree. This assumes the
current script is inside the source tree."""
cur_dir = os.path.dirname(os.path.realpath(__file__))
while True:
- parent_dir, dirname = os.path.split(cur_dir)
- # Check if it looks like the src/ root.
- if dirname == "src" and os.path.isdir(os.path.join(cur_dir, "net")):
+ # Check if it looks like the BoringSSL root.
+ if os.path.isdir(os.path.join(cur_dir, "crypto")) and \
+ os.path.isdir(os.path.join(cur_dir, "pki")) and \
+ os.path.isdir(os.path.join(cur_dir, "ssl")):
return cur_dir
+ parent_dir, _ = os.path.split(cur_dir)
if not parent_dir or parent_dir == cur_dir:
break
cur_dir = parent_dir
@@ -109,7 +111,7 @@
test_stdout = sys.stdin.read()
for m in failed_test_regex.finditer(test_stdout):
- src_relative_errors_path = m.group(1)
+ src_relative_errors_path = "pki/" + m.group(1)
errors_path = get_abs_path(src_relative_errors_path)
actual_errors = m.group(2)
diff --git a/pki/verify_certificate_chain.cc b/pki/verify_certificate_chain.cc
index f504ef0..da37d62 100644
--- a/pki/verify_certificate_chain.cc
+++ b/pki/verify_certificate_chain.cc
@@ -202,7 +202,6 @@
bool has_code_signing_eku = false;
bool has_time_stamping_eku = false;
bool has_ocsp_signing_eku = false;
- bool has_nsgc = false;
if (cert.has_extended_key_usage()) {
for (const auto &key_purpose_oid : cert.extended_key_usage()) {
if (key_purpose_oid == der::Input(kAnyEKU)) {
@@ -223,9 +222,6 @@
if (key_purpose_oid == der::Input(kOCSPSigning)) {
has_ocsp_signing_eku = true;
}
- if (key_purpose_oid == der::Input(kNetscapeServerGatedCrypto)) {
- has_nsgc = true;
- }
}
}
@@ -304,7 +300,6 @@
return;
case KeyPurpose::SERVER_AUTH:
case KeyPurpose::SERVER_AUTH_STRICT: {
- bool nsgc_hack = false;
if (has_any_eku && !has_server_auth_eku) {
if (is_target_cert || is_target_cert_issuer) {
errors->AddWarning(cert_errors::kEkuLacksServerAuthButHasAnyEKU);
@@ -318,29 +313,10 @@
// TODO(bbe): remove this once BR requirements catch up with CA's.
has_server_auth_eku = true;
}
- if (has_nsgc && !has_server_auth_eku) {
- errors->AddWarning(cert_errors::kEkuLacksServerAuthButHasGatedCrypto);
-
- // Allow NSGC for legacy RSA SHA1 intermediates, for compatibility
- // with platform verifiers.
- //
- // In practice the chain will be rejected with or without this
- // compatibility hack. The difference is whether the final error will
- // be ERR_CERT_WEAK_SIGNATURE_ALGORITHM (with compatibility hack) vs
- // ERR_CERT_INVALID (without hack).
- //
- // TODO(https://crbug.com/843735): Remove this once error-for-error
- // equivalence between builtin verifier and platform verifier is less
- // important.
- if ((cert.has_basic_constraints() && cert.basic_constraints().is_ca) &&
- cert.signature_algorithm() == SignatureAlgorithm::kRsaPkcs1Sha1) {
- nsgc_hack = true;
- }
- }
if (required_key_purpose == KeyPurpose::SERVER_AUTH) {
// Legacy compatible.
if (cert.has_extended_key_usage() && !has_server_auth_eku &&
- !has_any_eku && !nsgc_hack) {
+ !has_any_eku) {
errors->AddError(cert_errors::kEkuLacksServerAuth);
}
} else {
