Skip to content

GitLab

T
tangled
Commit a6173cd8 authored by davidben's avatar davidben Committed by Commit bot
Browse files
Options

Account for the OS returning an empty certificate chain. 

OS X sometimes returns an empty certificate chain on unrecoverable certificate
chain. This was leaving the verified certificate NULL and causing crashes.

BUG=472291

Review URL: https://codereview.chromium.org/1094983002

Cr-Commit-Position: refs/heads/master@{#326683}
parent 0a7a6691
Hide whitespace changes
Inline Side-by-side
Showing with 192 additions and 23 deletions
net/cert/cert_verify_proc.h
View file @ a6173cd8
...@@ -76,7 +76,9 @@ class NET_EXPORT CertVerifyProc ...@@ -76,7 +76,9 @@ class NET_EXPORT CertVerifyProc
FRIEND_TEST_ALL_PREFIXES(CertVerifyProcTest, TestHasTooLongValidity); FRIEND_TEST_ALL_PREFIXES(CertVerifyProcTest, TestHasTooLongValidity);
// Performs the actual verification using the desired underlying // Performs the actual verification using the desired underlying
// cryptographic library. // cryptographic library. On entry, |verify_result->verified_cert|
// is set to |cert|, the unverified chain. If no chain is built, the
// value must be left untouched.
virtual int VerifyInternal(X509Certificate* cert, virtual int VerifyInternal(X509Certificate* cert,
const std::string& hostname, const std::string& hostname,
int flags, int flags,
......
net/cert/cert_verify_proc_mac.cc
View file @ a6173cd8
...@@ -178,13 +178,14 @@ OSStatus CreateTrustPolicies(const std::string& hostname, ...@@ -178,13 +178,14 @@ OSStatus CreateTrustPolicies(const std::string& hostname,
// Stores the constructed certificate chain |cert_chain| and information about // Stores the constructed certificate chain |cert_chain| and information about
// the signature algorithms used into |*verify_result|. If the leaf cert in // the signature algorithms used into |*verify_result|. If the leaf cert in
// |cert_chain| contains a weak (MD2, MD4, MD5, SHA-1) signature, stores that // |cert_chain| contains a weak (MD2, MD4, MD5, SHA-1) signature, stores that
// in |*leaf_is_weak|. // in |*leaf_is_weak|. |cert_chain| must not be empty.
void GetCertChainInfo(CFArrayRef cert_chain, void GetCertChainInfo(CFArrayRef cert_chain,
CSSM_TP_APPLE_EVIDENCE_INFO* chain_info, CSSM_TP_APPLE_EVIDENCE_INFO* chain_info,
CertVerifyResult* verify_result, CertVerifyResult* verify_result,
bool* leaf_is_weak) { bool* leaf_is_weak) {
DCHECK_LT(0, CFArrayGetCount(cert_chain));
*leaf_is_weak = false; *leaf_is_weak = false;
verify_result->verified_cert = nullptr;
verify_result->has_md2 = false; verify_result->has_md2 = false;
verify_result->has_md4 = false; verify_result->has_md4 = false;
verify_result->has_md5 = false; verify_result->has_md5 = false;
...@@ -253,8 +254,10 @@ void GetCertChainInfo(CFArrayRef cert_chain, ...@@ -253,8 +254,10 @@ void GetCertChainInfo(CFArrayRef cert_chain,
*leaf_is_weak = true; *leaf_is_weak = true;
} }
} }
if (!verified_cert) if (!verified_cert) {
NOTREACHED();
return; return;
}
verify_result->verified_cert = verify_result->verified_cert =
X509Certificate::CreateFromHandle(verified_cert, verified_chain); X509Certificate::CreateFromHandle(verified_cert, verified_chain);
...@@ -561,17 +564,23 @@ int CertVerifyProcMac::VerifyInternal( ...@@ -561,17 +564,23 @@ int CertVerifyProcMac::VerifyInternal(
if (rv != OK) if (rv != OK)
return rv; return rv;
CertVerifyResult temp_verify_result;
bool leaf_is_weak = false;
GetCertChainInfo(temp_chain, temp_chain_info, &temp_verify_result,
&leaf_is_weak);
bool untrusted = (temp_trust_result != kSecTrustResultUnspecified && bool untrusted = (temp_trust_result != kSecTrustResultUnspecified &&
temp_trust_result != kSecTrustResultProceed); temp_trust_result != kSecTrustResultProceed);
bool weak_chain = bool weak_chain = false;
!leaf_is_weak && if (CFArrayGetCount(temp_chain) == 0) {
(temp_verify_result.has_md2 || temp_verify_result.has_md4 || // If the chain is empty, it cannot be trusted or have recoverable
temp_verify_result.has_md5 || temp_verify_result.has_sha1); // errors.
DCHECK(untrusted);
DCHECK_NE(kSecTrustResultRecoverableTrustFailure, temp_trust_result);
} else {
CertVerifyResult temp_verify_result;
bool leaf_is_weak = false;
GetCertChainInfo(temp_chain, temp_chain_info, &temp_verify_result,
&leaf_is_weak);
weak_chain = !leaf_is_weak &&
(temp_verify_result.has_md2 || temp_verify_result.has_md4 ||
temp_verify_result.has_md5 || temp_verify_result.has_sha1);
}
// Set the result to the current chain if: // Set the result to the current chain if:
// - This is the first verification attempt. This ensures that if // - This is the first verification attempt. This ensures that if
// everything is awful (e.g. it may just be an untrusted cert), that // everything is awful (e.g. it may just be an untrusted cert), that
...@@ -609,9 +618,11 @@ int CertVerifyProcMac::VerifyInternal( ...@@ -609,9 +618,11 @@ int CertVerifyProcMac::VerifyInternal(
if (crl_set && !CheckRevocationWithCRLSet(completed_chain, crl_set)) if (crl_set && !CheckRevocationWithCRLSet(completed_chain, crl_set))
verify_result->cert_status |= CERT_STATUS_REVOKED; verify_result->cert_status |= CERT_STATUS_REVOKED;
bool leaf_is_weak_unused = false; if (CFArrayGetCount(completed_chain) > 0) {
GetCertChainInfo(completed_chain, chain_info, verify_result, bool leaf_is_weak_unused = false;
&leaf_is_weak_unused); GetCertChainInfo(completed_chain, chain_info, verify_result,
&leaf_is_weak_unused);
}
// As of Security Update 2012-002/OS X 10.7.4, when an RSA key < 1024 bits // As of Security Update 2012-002/OS X 10.7.4, when an RSA key < 1024 bits
// is encountered, CSSM returns CSSMERR_TP_VERIFY_ACTION_FAILED and adds // is encountered, CSSM returns CSSMERR_TP_VERIFY_ACTION_FAILED and adds
......
net/cert/cert_verify_proc_unittest.cc
View file @ a6173cd8
...@@ -1587,4 +1587,28 @@ WRAPPED_INSTANTIATE_TEST_CASE_P( ...@@ -1587,4 +1587,28 @@ WRAPPED_INSTANTIATE_TEST_CASE_P(
CertVerifyProcNameTest, CertVerifyProcNameTest,
testing::ValuesIn(kVerifyNameData)); testing::ValuesIn(kVerifyNameData));
#if defined(OS_MACOSX) && !defined(OS_IOS)
// Test that CertVerifyProcMac reacts appropriately when Apple's certificate
// verifier rejects a certificate with a fatal error. This is a regression
// test for https://crbug.com/472291.
TEST_F(CertVerifyProcTest, LargeKey) {
// Load root_ca_cert.pem into the test root store.
ScopedTestRoot test_root(
ImportCertFromFile(GetTestCertsDirectory(), "root_ca_cert.pem").get());
scoped_refptr<X509Certificate> cert(
ImportCertFromFile(GetTestCertsDirectory(), "large_key.pem"));
// Apple's verifier rejects this certificate as invalid because the
// RSA key is too large. If a future version of OS X changes this,
// large_key.pem may need to be regenerated with a larger key.
int flags = 0;
CertVerifyResult verify_result;
int error = Verify(cert.get(), "127.0.0.1", flags, NULL, empty_cert_list_,
&verify_result);
EXPECT_EQ(ERR_CERT_INVALID, error);
EXPECT_EQ(CERT_STATUS_INVALID, verify_result.cert_status);
}
#endif // defined(OS_MACOSX) && !defined(OS_IOS)
} // namespace net } // namespace net
net/cert/x509_certificate_mac.cc
View file @ a6173cd8
...@@ -497,7 +497,9 @@ void X509Certificate::GetPublicKeyInfo(OSCertHandle cert_handle, ...@@ -497,7 +497,9 @@ void X509Certificate::GetPublicKeyInfo(OSCertHandle cert_handle,
SecKeyRef key; SecKeyRef key;
OSStatus status = SecCertificateCopyPublicKey(cert_handle, &key); OSStatus status = SecCertificateCopyPublicKey(cert_handle, &key);
if (status) { if (status) {
NOTREACHED() << "SecCertificateCopyPublicKey failed: " << status; // SecCertificateCopyPublicKey may fail if the certificate has an invalid
// key. See https://crbug.com/472291.
LOG(WARNING) << "SecCertificateCopyPublicKey failed: " << status;
return; return;
} }
ScopedCFTypeRef<SecKeyRef> scoped_key(key); ScopedCFTypeRef<SecKeyRef> scoped_key(key);
......
net/cert/x509_certificate_unittest.cc
View file @ a6173cd8
...@@ -1158,12 +1158,23 @@ const struct PublicKeyInfoTestData { ...@@ -1158,12 +1158,23 @@ const struct PublicKeyInfoTestData {
size_t expected_bits; size_t expected_bits;
X509Certificate::PublicKeyType expected_type; X509Certificate::PublicKeyType expected_type;
} kPublicKeyInfoTestData[] = { } kPublicKeyInfoTestData[] = {
{ "768-rsa-ee-by-768-rsa-intermediate.pem", 768, {"768-rsa-ee-by-768-rsa-intermediate.pem",
X509Certificate::kPublicKeyTypeRSA }, 768,
{ "1024-rsa-ee-by-768-rsa-intermediate.pem", 1024, X509Certificate::kPublicKeyTypeRSA},
X509Certificate::kPublicKeyTypeRSA }, {"1024-rsa-ee-by-768-rsa-intermediate.pem",
{ "prime256v1-ecdsa-ee-by-1024-rsa-intermediate.pem", 256, 1024,
X509Certificate::kPublicKeyTypeECDSA }, X509Certificate::kPublicKeyTypeRSA},
{"prime256v1-ecdsa-ee-by-1024-rsa-intermediate.pem",
256,
X509Certificate::kPublicKeyTypeECDSA},
#if defined(OS_MACOSX) && !defined(OS_IOS)
// OS X has an key length limit of 4096 bits. This should manifest as an
// unknown key. If a future version of OS X changes this, large_key.pem may
// need to be renegerated with a larger key. See https://crbug.com/472291.
{"large_key.pem", 0, X509Certificate::kPublicKeyTypeUnknown},
#else
{"large_key.pem", 4104, X509Certificate::kPublicKeyTypeRSA},
#endif
}; };
class X509CertificatePublicKeyInfoTest class X509CertificatePublicKeyInfoTest
......
net/data/ssl/certificates/large_key.pem 0 → 100644
View file @ a6173cd8
Certificate:
Data:
Version: 1 (0x0)
Serial Number:
fa:ba:a5:10:ef:85:bb:70
Signature Algorithm: sha256WithRSAEncryption
Issuer: C=US, ST=California, L=Mountain View, O=Test CA, CN=127.0.0.1
Validity
Not Before: Apr 23 21:32:21 2015 GMT
Not After : Apr 20 21:32:21 2025 GMT
Subject: C=US, ST=California, L=Mountain View, O=Test CA, CN=127.0.0.1
Subject Public Key Info:
Public Key Algorithm: rsaEncryption
RSA Public Key: (4104 bit)
Modulus (4104 bit):
00:c9:98:35:7d:a5:d4:41:1b:83:60:84:76:14:c2:
0d:a6:7f:e7:52:15:a5:7c:96:96:56:b8:98:f9:bc:
94:56:23:14:f2:74:05:e6:ba:d9:3a:58:dd:b5:03:
9c:d1:f5:47:76:c2:45:78:f9:b8:e1:44:40:45:08:
2d:d1:ff:c3:bf:d7:b2:4a:94:6a:61:37:dd:cb:66:
22:28:8d:cc:5f:35:dd:9a:1a:d5:0e:cc:36:76:ae:
65:a2:96:d3:cd:de:8d:b4:c9:28:a8:a8:2d:45:89:
98:aa:7d:9e:12:7b:e1:a5:ba:d9:56:8a:4a:1f:1c:
7d:e2:cf:f5:f0:54:38:6d:47:4c:1d:1c:22:3c:74:
72:4b:fd:c2:97:66:b1:9c:8e:c3:d6:88:5e:6a:ed:
c9:fe:b7:d9:ef:03:25:5d:da:4a:ab:5e:fa:f4:a8:
87:30:90:00:85:a6:5b:40:77:b6:34:64:63:58:05:
cd:1e:58:d6:fd:b2:13:51:ba:a4:9f:7f:e3:db:88:
88:58:1f:df:5f:67:cc:49:0a:bb:e0:ce:d9:3f:ce:
26:20:1e:35:f3:f0:8a:bd:8b:8b:4f:16:80:53:4e:
44:3b:1a:9e:60:6b:ff:cf:f7:9f:40:bf:d2:86:e4:
79:39:3a:d7:5a:bd:d0:63:cf:f6:54:e1:9e:09:e7:
63:8a:17:33:bf:f2:5f:ec:ee:45:18:52:ee:e7:23:
a2:a8:af:d2:85:4b:bb:f9:12:ba:35:22:d2:5e:9c:
42:2d:66:e1:01:02:d4:0f:ff:86:1c:91:8f:55:9c:
9f:24:4e:17:c8:23:c1:a7:4e:39:ed:4b:dc:b8:c4:
3f:7d:c9:0f:ff:01:40:4d:dd:9a:55:29:84:f8:52:
72:da:38:32:9a:ef:2b:2e:67:5b:d2:2d:70:46:91:
42:79:22:a2:08:c0:fd:24:95:9e:04:a6:5c:9e:c0:
ed:25:2a:57:62:17:b7:c0:12:d4:8b:59:f3:db:8b:
cd:95:4f:d9:32:f2:f0:e1:14:9b:db:73:e7:b4:97:
a6:90:f7:0d:cb:59:93:94:12:2a:c1:28:7d:3c:bd:
b5:7e:bb:13:c0:4c:ef:ce:20:3d:3b:4f:84:25:77:
dc:1f:0f:08:dd:76:9c:ad:7d:00:2f:23:42:6a:37:
e9:d0:6b:8e:9a:6f:3b:46:09:74:23:43:a9:58:46:
62:33:69:2a:0c:df:cd:6c:75:88:f1:01:3d:67:e0:
a8:e3:bd:23:03:66:f3:11:f2:07:a8:bf:4b:c9:81:
ce:a9:ed:8d:48:a6:10:15:fb:79:27:ae:2e:8f:35:
c5:39:12:eb:f2:b7:28:d3:b3:e7:4c:71:d8:e1:d1:
f7:39:94:87
Exponent: 65537 (0x10001)
Signature Algorithm: sha256WithRSAEncryption
2d:3b:5f:58:30:58:97:65:d0:36:a6:03:0c:6b:bc:a6:5b:e7:
8f:a3:a7:05:5d:db:9b:3f:0e:22:89:3e:e1:00:91:3c:17:ab:
10:10:80:f5:d1:f9:cb:d7:38:9e:bc:36:a0:31:d2:e3:2f:98:
62:d8:a2:cd:73:7c:56:29:b0:c0:41:01:71:50:70:7a:b2:9d:
b6:ad:13:71:d8:19:7d:4f:7a:be:4b:36:2e:d7:f7:81:53:a7:
e0:7f:04:77:09:84:a3:56:22:48:7e:6d:ca:20:12:ed:a7:da:
29:cf:46:46:81:8f:97:4b:17:63:25:f2:18:48:c1:82:21:c7:
be:4f:54:29:88:29:78:d0:be:98:9c:d9:4f:76:e7:6b:a2:10:
4a:ce:45:02:c7:5e:5a:ac:5f:0e:4b:a3:51:a9:ec:7a:3f:a0:
51:49:75:bc:af:f4:89:88:35:69:65:15:a5:33:c5:2f:1e:c5:
30:98:6b:04:14:b9:29:b5:31:76:23:88:32:72:29:e4:16:de:
bb:95:d6:bb:48:e5:38:37:57:6e:8a:2f:c0:64:2c:01:4d:e6:
69:9e:ae:4a:39:87:96:5d:50:e6:25:08:46:77:ea:da:4d:b2:
bd:5e:44:ca:e2:46:b1:7a:4a:3d:83:16:a9:a6:e5:c4:ef:fc:
af:77:58:6e:3b:cd:b3:b3:0e:d2:59:d2:be:16:ac:64:fa:f7:
b1:06:02:7d:c9:8a:06:65:73:da:d8:a8:02:41:d5:68:41:7a:
0f:a7:5c:90:b3:6f:01:66:65:96:4c:50:bd:c7:38:67:61:21:
52:dc:a9:f0:f7:0b:16:95:2d:5d:b8:0b:da:9a:fe:f8:4c:e9:
fa:6a:32:a7:d4:67:7b:21:45:7a:4e:7c:a6:fa:cb:65:3d:8f:
38:90:5a:29:7d:7f:21:32:6d:c6:61:70:f9:a8:4e:3f:9b:3c:
f0:4b:40:d6:7d:e5:d6:31:08:ff:29:21:f9:e7:81:36:1a:86:
d5:96:06:1d:1e:b1:fc:4c:e3:6d:ce:3a:2c:16:b8:03:76:f3:
70:77:15:ea:b5:32:c3:f5:9f:a6:61:e1:29:bb:88:2c:a0:cf:
05:f5:3d:97:ae:b5:b9:01:e8:2f:5e:54:17:2e:09:5b:ea:d0:
4a:95:65:d9:cf:83:3a:bc:1d:7d:28:4b:7b:8d:5c:ef:8a:06:
f9:64:b5:ec:3f:73:fc:6c:58:cc:57:da:d3:5d:5e:b5:58:43:
53:27:f0:49:01:e4:45:8d:eb:68:f3:40:fc:ec:63:62:ce:c3:
d9:97:d3:a3:51:8c:c5:09:a9:81:6e:23:e0:6a:1f:02:b0:f8:
b0:40:0c:2f:ea:ee:c2:1f:1b
-----BEGIN CERTIFICATE-----
MIIFPjCCAyUCCQD6uqUQ74W7cDANBgkqhkiG9w0BAQsFADBgMQswCQYDVQQGEwJV
UzETMBEGA1UECAwKQ2FsaWZvcm5pYTEWMBQGA1UEBwwNTW91bnRhaW4gVmlldzEQ
MA4GA1UECgwHVGVzdCBDQTESMBAGA1UEAwwJMTI3LjAuMC4xMB4XDTE1MDQyMzIx
MzIyMVoXDTI1MDQyMDIxMzIyMVowYDELMAkGA1UEBhMCVVMxEzARBgNVBAgMCkNh
bGlmb3JuaWExFjAUBgNVBAcMDU1vdW50YWluIFZpZXcxEDAOBgNVBAoMB1Rlc3Qg
Q0ExEjAQBgNVBAMMCTEyNy4wLjAuMTCCAiMwDQYJKoZIhvcNAQEBBQADggIQADCC
AgsCggICAMmYNX2l1EEbg2CEdhTCDaZ/51IVpXyWlla4mPm8lFYjFPJ0Bea62TpY
3bUDnNH1R3bCRXj5uOFEQEUILdH/w7/XskqUamE33ctmIiiNzF813Zoa1Q7MNnau
ZaKW083ejbTJKKioLUWJmKp9nhJ74aW62VaKSh8cfeLP9fBUOG1HTB0cIjx0ckv9
wpdmsZyOw9aIXmrtyf632e8DJV3aSqte+vSohzCQAIWmW0B3tjRkY1gFzR5Y1v2y
E1G6pJ9/49uIiFgf319nzEkKu+DO2T/OJiAeNfPwir2Li08WgFNORDsanmBr/8/3
n0C/0obkeTk611q90GPP9lThngnnY4oXM7/yX+zuRRhS7ucjoqiv0oVLu/kSujUi
0l6cQi1m4QEC1A//hhyRj1WcnyROF8gjwadOOe1L3LjEP33JD/8BQE3dmlUphPhS
cto4MprvKy5nW9ItcEaRQnkiogjA/SSVngSmXJ7A7SUqV2IXt8AS1ItZ89uLzZVP
2TLy8OEUm9tz57SXppD3DctZk5QSKsEofTy9tX67E8BM784gPTtPhCV33B8PCN12
nK19AC8jQmo36dBrjppvO0YJdCNDqVhGYjNpKgzfzWx1iPEBPWfgqOO9IwNm8xHy
B6i/S8mBzqntjUimEBX7eSeuLo81xTkS6/K3KNOz50xx2OHR9zmUhwIDAQABMA0G
CSqGSIb3DQEBCwUAA4ICAgAtO19YMFiXZdA2pgMMa7ymW+ePo6cFXdubPw4iiT7h
AJE8F6sQEID10fnL1zievDagMdLjL5hi2KLNc3xWKbDAQQFxUHB6sp22rRNx2Bl9
T3q+SzYu1/eBU6fgfwR3CYSjViJIfm3KIBLtp9opz0ZGgY+XSxdjJfIYSMGCIce+
T1QpiCl40L6YnNlPdudrohBKzkUCx15arF8OS6NRqex6P6BRSXW8r/SJiDVpZRWl
M8UvHsUwmGsEFLkptTF2I4gycinkFt67lda7SOU4N1duii/AZCwBTeZpnq5KOYeW
XVDmJQhGd+raTbK9XkTK4kaxeko9gxappuXE7/yvd1huO82zsw7SWdK+Fqxk+vex
BgJ9yYoGZXPa2KgCQdVoQXoPp1yQs28BZmWWTFC9xzhnYSFS3Knw9wsWlS1duAva
mv74TOn6ajKn1Gd7IUV6Tnym+stlPY84kFopfX8hMm3GYXD5qE4/mzzwS0DWfeXW
MQj/KSH554E2GobVlgYdHrH8TONtzjosFrgDdvNwdxXqtTLD9Z+mYeEpu4gsoM8F
9T2XrrW5AegvXlQXLglb6tBKlWXZz4M6vB19KEt7jVzvigb5ZLXsP3P8bFjMV9rT
XV61WENTJ/BJAeRFjeto80D87GNizsPZl9OjUYzFCamBbiPgah8CsPiwQAwv6u7C
Hxs=
-----END CERTIFICATE-----
net/data/ssl/scripts/generate-test-certs.sh
View file @ a6173cd8
...@@ -150,6 +150,13 @@ SUBJECT_NAME="req_dn" \ ...@@ -150,6 +150,13 @@ SUBJECT_NAME="req_dn" \
-config ../scripts/ee.cnf -newkey rsa:2048 -text \ -config ../scripts/ee.cnf -newkey rsa:2048 -text \
-out ../certificates/reject_intranet_hosts.pem -out ../certificates/reject_intranet_hosts.pem
## Leaf certificate with a large key; Apple's certificate verifier rejects with
## a fatal error if the key is bigger than 4096 bits.
try openssl req -x509 -days 3650 \
-config ../scripts/ee.cnf -newkey rsa:4104 -text \
-sha256 \
-out ../certificates/large_key.pem
## Validity too long unit test support. ## Validity too long unit test support.
try openssl req -config ../scripts/ee.cnf \ try openssl req -config ../scripts/ee.cnf \
-newkey rsa:2048 -text -out ../certificates/10_year_validity.req -newkey rsa:2048 -text -out ../certificates/10_year_validity.req
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment