CertVerifyProcInternalTest.PublicKeyHashes/CertVerifyProcNSS causes failures in other tests |
|||||
Issue descriptionCertVerifyProcInternalTest.PublicKeyHashes/CertVerifyProcNSS seems to leak some state which causes subsequent tests to fail. In particular, ExpectStaple/ExpectStapleErrorResponseTest.CheckResponseStatusSerialization/0 I verified that if I comment out the call to verify_proc_->Verify in cert_verify_proc_unittest.cc, then subsequent tests pass, so I would *Guess* that something is cached as a result of that call? % net_unittests --single-process-tests --gtest_filter=CertVerifyProcInternalTest.PublicKeyHashes/CertVerifyProcNSS:ExpectStaple/ExpectStapleErrorResponseTest.CheckResponseStatusSerialization/0 Note: Google Test filter = CertVerifyProcInternalTest.PublicKeyHashes/CertVerifyProcNSS:ExpectStaple/ExpectStapleErrorResponseTest.CheckResponseStatusSerialization/0 [==========] Running 2 tests from 2 test cases. [----------] Global test environment set-up. [----------] 1 test from CertVerifyProcInternalTest [ RUN ] CertVerifyProcInternalTest.PublicKeyHashes/CertVerifyProcNSS [ OK ] CertVerifyProcInternalTest.PublicKeyHashes/CertVerifyProcNSS (1116 ms) [----------] 1 test from CertVerifyProcInternalTest (1116 ms total) [----------] 1 test from ExpectStaple/ExpectStapleErrorResponseTest [ RUN ] ExpectStaple/ExpectStapleErrorResponseTest.CheckResponseStatusSerialization/0 ../../net/http/transport_security_state_unittest.cc:228: Failure Value of: cert_chain Actual: false Expected: true ../../net/http/transport_security_state_unittest.cc:367: Failure Expected: CompareCertificateChainWithList( ssl_info.unverified_cert, report_served_certificate_chain) doesn't generate new fatal failures in the current thread. Actual: it does. ../../net/http/transport_security_state_unittest.cc:397: Failure Expected: CheckSerializedExpectStapleReport( serialized_report, host_port, ssl_info, ocsp_response, response_status, cert_status) doesn't generate new fatal failures in the current thread. Actual: it does. ../../net/http/transport_security_state_unittest.cc:2444: Failure Expected: CheckExpectStapleReport(&state, &reporter, ssl_info, ocsp_response, test.response_status_string, std::string()) doesn't generate new fatal failures in the current thread. Actual: it does. [ FAILED ] ExpectStaple/ExpectStapleErrorResponseTest.CheckResponseStatusSerialization/0, where GetParam() = 16-byte object <01-00 00-00 AB-AB AB-AB 18-08 30-62 72-14 00-00> (10 ms) [----------] 1 test from ExpectStaple/ExpectStapleErrorResponseTest (10 ms total) [----------] Global test environment tear-down [==========] 2 tests from 2 test cases ran. (1133 ms total) [ PASSED ] 1 test. [ FAILED ] 1 test, listed below: [ FAILED ] ExpectStaple/ExpectStapleErrorResponseTest.CheckResponseStatusSerialization/0, where GetParam() = 16-byte object <01-00 00-00 AB-AB AB-AB 18-08 30-62 72-14 00-00>
,
Jun 5 2017
Assigning to estark@ for visibility, but I'll look into it.
,
Jun 5 2017
So it looks like another "NSS failing to parse certificates because of duplicate serials". Still digging through the relationships.
,
Jun 5 2017
Yeah, it's a manifestation of duplicate serials. The test itself fails because of a lack of ASSERT on the parsing (that would have it fail faster), but the root issue is that several certs from the test CA have reused serials. I'm regenerating them all now to sort out the root, and have added asserts for robustness.
,
Jun 5 2017
,
Jun 5 2017
*mind blown* I never would have guessed! Thanks for getting to the bottom of this.
,
Jun 7 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6ee44a7247c639c0703f291d320bdf05c1531b57 commit 6ee44a7247c639c0703f291d320bdf05c1531b57 Author: rsleevi <rsleevi@chromium.org> Date: Wed Jun 07 17:21:33 2017 Fix TransportSecurityState unittests to run in --single-process mode. Several of the TransportSecurityState unittests do not ASSERT that they are able to parse a certificate. If parsing fails, they end up causing failures with Expect-Staple reporting. This CL adds robustness checks by consistently ASSERTing that certificates could be loaded from disk successfully. This is the fix for diagnostics, but not the fix for root cause. The root cause is that several test certificates duplicated the issuer and serial number tuples, due to their organic growth and addition. On systems using NSS to represent the OS certificate (e.g. Linux and ChromeOS), due to NSS unfortunately being implemented assuming X.500's (never implemented) uniqueness of issuer+serial tuples, if two certificates have the same tuple, NSS will fail to parse the second certificate if the first certificate is still in memory. When running tests with --single-process, and having run a test that successfully verifies a certificate using a test intermediate, NSS unfortunately keeps a copy of the intermediate in memory until NSS itself is unloaded. Since we never unload NSS, this intermediate is always kept around, thus causing observable side-effects. On the bots and under normal testing, this doesn't manifest, because the test harness runs each test separately as needed, to ensure hermeticism. However, some developers like to use --single-process for speed (less process spawning overhead) or debugging, despite no bots running this config. The duplicate serial number itself emerged from side-effects related to bash functions and environment variables, hence why it was not initially spotted. To properly resolve this issue, this change does the following: 1) Fix the test generation script to not leak environment state. a) Fixes the duplicate serial number issue. b) Fixes incorrect subject names and issuer names due to env bleeding. 2) Regenerate the test certificates to ensure serial uniqueness. 3) Update the tests that have hardcoded aspects of the chain (PKP testing and name constraint testing). 4) Add additional assertions to TransportSecurityState to fail quicker. 5) Remove an unnecessary HPKP test chain that had been refactored away. BUG= 729341 Review-Url: https://codereview.chromium.org/2926463002 Cr-Commit-Position: refs/heads/master@{#477691} [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/chrome/common/net/x509_certificate_model_unittest.cc [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/components/certificate_reporting/error_report_unittest.cc [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/content/browser/loader/resource_loader_unittest.cc [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/BUILD.gn [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/cert/cert_verify_proc.cc [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/cert/cert_verify_proc_unittest.cc [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/10_year_validity.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/11_year_validity.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/39_months_after_2015_04.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/40_months_after_2015_04.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/60_months_after_2012_07.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/61_months_after_2012_07.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/README [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/bad_validity.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/crlset_by_intermediate_serial.raw [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/crlset_by_leaf_spki.raw [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/crlset_by_root_serial.raw [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/expired_cert.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/intermediate_ca_cert.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/large_key.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/localhost_cert.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/name_constraint_bad.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/name_constraint_good.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/ok_cert.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/ok_cert_by_intermediate.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/post_june_2016.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/pre_br_validity_bad_121.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/pre_br_validity_bad_2020.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/pre_br_validity_ok.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/pre_june_2016.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/punycodetest.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/reject_intranet_hosts.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/root_ca_cert.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/sha1_2016.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/sha1_dec_2015.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/sha1_jan_2016.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/spdy_pooling.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/start_after_expiry.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/subjectAltName_sanity_check.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/subjectAltName_www_example_com.pem [delete] https://crrev.com/c53f4ad87510ee97b5c3425a14c0e79780cdf262/net/data/ssl/certificates/test_mail_google_com.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/tls_feature_extension.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/wildcard.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/certificates/x509_verify_results.chain.pem [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/ssl/scripts/generate-test-certs.sh [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/url_request_unittest/hpkp-headers-report-only.html.mock-http-headers [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/url_request_unittest/hpkp-headers.html.mock-http-headers [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/url_request_unittest/hsts-and-hpkp-headers.html.mock-http-headers [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/data/url_request_unittest/hsts-and-hpkp-headers2.html.mock-http-headers [modify] https://crrev.com/6ee44a7247c639c0703f291d320bdf05c1531b57/net/http/transport_security_state_unittest.cc
,
Jun 7 2017
|
|||||
►
Sign in to add a comment |
|||||
Comment 1 by rch@chromium.org
, Jun 3 2017