Commit 3f5ff3e9 authored by Ryan Sleevi's avatar Ryan Sleevi Committed by Commit Bot

Don't export MapNetErrorToCertStatus from //net

CertStatus contains additional information than contained solely
within a net::Error, and so converting from a
Status -> Error -> Status loses that fidelity, even though it's
"safe" to go Error -> Status -> Error.

This also corrects a few unittest CertVerifiers flagged
during review which have no functional impact.

Bug: 618799
Change-Id: I6a098c70c2a870f4f9c22801f24d906a4faf04ba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1879852Reviewed-by: default avatarBoris Sazonov <bsazonov@chromium.org>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarMatt Mueller <mattm@chromium.org>
Commit-Queue: Ryan Sleevi <rsleevi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#709467}
parent 9ab9bdd9
...@@ -80,11 +80,10 @@ class TestCertVerifier : public net::CertVerifier { ...@@ -80,11 +80,10 @@ class TestCertVerifier : public net::CertVerifier {
net::CompletionOnceCallback callback, net::CompletionOnceCallback callback,
std::unique_ptr<Request>* out_req, std::unique_ptr<Request>* out_req,
const net::NetLogWithSource& net_log) override { const net::NetLogWithSource& net_log) override {
net::Error result = net::OK; verify_result->Reset();
verify_result->verified_cert = params.certificate(); verify_result->verified_cert = params.certificate();
verify_result->cert_status = net::MapNetErrorToCertStatus(result);
verify_result->is_issued_by_known_root = true; verify_result->is_issued_by_known_root = true;
return result; return net::OK;
} }
void SetConfig(const Config& config) override {} void SetConfig(const Config& config) override {}
}; };
......
...@@ -36,9 +36,9 @@ class TestCertVerifier : public net::MockCertVerifier { ...@@ -36,9 +36,9 @@ class TestCertVerifier : public net::MockCertVerifier {
net::CompletionOnceCallback callback, net::CompletionOnceCallback callback,
std::unique_ptr<Request>* out_req, std::unique_ptr<Request>* out_req,
const net::NetLogWithSource& net_log) override { const net::NetLogWithSource& net_log) override {
verify_result->Reset();
if (params.hostname() == "test.example.com") { if (params.hostname() == "test.example.com") {
verify_result->verified_cert = params.certificate(); verify_result->verified_cert = params.certificate();
verify_result->cert_status = MapNetErrorToCertStatus(net::OK);
verify_result->is_issued_by_known_root = true; verify_result->is_issued_by_known_root = true;
return net::OK; return net::OK;
} }
......
...@@ -136,6 +136,7 @@ class GMockCertVerifier : public net::CertVerifier { ...@@ -136,6 +136,7 @@ class GMockCertVerifier : public net::CertVerifier {
net::CompletionOnceCallback callback, net::CompletionOnceCallback callback,
std::unique_ptr<net::CertVerifier::Request>* out_req, std::unique_ptr<net::CertVerifier::Request>* out_req,
const net::NetLogWithSource& net_log) override { const net::NetLogWithSource& net_log) override {
verify_result->Reset();
return VerifyImpl(params, verify_result, out_req, net_log); return VerifyImpl(params, verify_result, out_req, net_log);
} }
......
...@@ -153,6 +153,8 @@ class MyTestCertVerifier : public net::CertVerifier { ...@@ -153,6 +153,8 @@ class MyTestCertVerifier : public net::CertVerifier {
net::CompletionOnceCallback callback, net::CompletionOnceCallback callback,
std::unique_ptr<Request>* out_req, std::unique_ptr<Request>* out_req,
const net::NetLogWithSource& net_log) override { const net::NetLogWithSource& net_log) override {
verify_result->Reset();
verify_result->verified_cert = params.certificate();
return net::OK; return net::OK;
} }
void SetConfig(const Config& config) override {} void SetConfig(const Config& config) override {}
......
...@@ -30,9 +30,14 @@ static inline bool IsCertStatusError(CertStatus status) { ...@@ -30,9 +30,14 @@ static inline bool IsCertStatusError(CertStatus status) {
return (CERT_STATUS_ALL_ERRORS & status) != 0; return (CERT_STATUS_ALL_ERRORS & status) != 0;
} }
// Maps a network error code to the equivalent certificate status flag. If // Maps a network error code to the equivalent certificate status flag. If
// the error code is not a certificate error, it is mapped to 0. // the error code is not a certificate error, it is mapped to 0.
NET_EXPORT CertStatus MapNetErrorToCertStatus(int error); // Note: It is not safe to go net::CertStatus -> net::Error -> net::CertStatus,
// as the CertStatus contains more information. Conversely, going from
// net::Error -> net::CertStatus -> net::Error is not a lossy function, for the
// same reason.
// To avoid incorrect use, this is only exported for unittest helpers.
NET_EXPORT_PRIVATE CertStatus MapNetErrorToCertStatus(int error);
// Maps the most serious certificate error in the certificate status flags // Maps the most serious certificate error in the certificate status flags
// to the equivalent network error code. // to the equivalent network error code.
......
...@@ -447,7 +447,7 @@ HashValueVector MakeHashValueVector(uint8_t tag) { ...@@ -447,7 +447,7 @@ HashValueVector MakeHashValueVector(uint8_t tag) {
} }
TEST_F(ProofVerifierChromiumTest, IsFatalErrorNotSetForNonFatalError) { TEST_F(ProofVerifierChromiumTest, IsFatalErrorNotSetForNonFatalError) {
dummy_result_.cert_status = MapNetErrorToCertStatus(ERR_CERT_DATE_INVALID); dummy_result_.cert_status = CERT_STATUS_DATE_INVALID;
MockCertVerifier dummy_verifier; MockCertVerifier dummy_verifier;
dummy_verifier.AddResultForCert(test_cert_.get(), dummy_result_, dummy_verifier.AddResultForCert(test_cert_.get(), dummy_result_,
...@@ -471,7 +471,7 @@ TEST_F(ProofVerifierChromiumTest, IsFatalErrorNotSetForNonFatalError) { ...@@ -471,7 +471,7 @@ TEST_F(ProofVerifierChromiumTest, IsFatalErrorNotSetForNonFatalError) {
} }
TEST_F(ProofVerifierChromiumTest, IsFatalErrorSetForFatalError) { TEST_F(ProofVerifierChromiumTest, IsFatalErrorSetForFatalError) {
dummy_result_.cert_status = MapNetErrorToCertStatus(ERR_CERT_DATE_INVALID); dummy_result_.cert_status = CERT_STATUS_DATE_INVALID;
MockCertVerifier dummy_verifier; MockCertVerifier dummy_verifier;
dummy_verifier.AddResultForCert(test_cert_.get(), dummy_result_, dummy_verifier.AddResultForCert(test_cert_.get(), dummy_result_,
......
...@@ -305,8 +305,7 @@ TEST_P(QuicChromiumClientSessionTest, IsFatalErrorNotSetForNonFatalError) { ...@@ -305,8 +305,7 @@ TEST_P(QuicChromiumClientSessionTest, IsFatalErrorNotSetForNonFatalError) {
ProofVerifyDetailsChromium details; ProofVerifyDetailsChromium details;
details.cert_verify_result.verified_cert = details.cert_verify_result.verified_cert =
ImportCertFromFile(GetTestCertsDirectory(), "spdy_pooling.pem"); ImportCertFromFile(GetTestCertsDirectory(), "spdy_pooling.pem");
details.cert_verify_result.cert_status = details.cert_verify_result.cert_status = CERT_STATUS_DATE_INVALID;
MapNetErrorToCertStatus(ERR_CERT_DATE_INVALID);
details.is_fatal_cert_error = false; details.is_fatal_cert_error = false;
CompleteCryptoHandshake(); CompleteCryptoHandshake();
session_->OnProofVerifyDetailsAvailable(details); session_->OnProofVerifyDetailsAvailable(details);
...@@ -328,8 +327,7 @@ TEST_P(QuicChromiumClientSessionTest, IsFatalErrorSetForFatalError) { ...@@ -328,8 +327,7 @@ TEST_P(QuicChromiumClientSessionTest, IsFatalErrorSetForFatalError) {
ProofVerifyDetailsChromium details; ProofVerifyDetailsChromium details;
details.cert_verify_result.verified_cert = details.cert_verify_result.verified_cert =
ImportCertFromFile(GetTestCertsDirectory(), "spdy_pooling.pem"); ImportCertFromFile(GetTestCertsDirectory(), "spdy_pooling.pem");
details.cert_verify_result.cert_status = details.cert_verify_result.cert_status = CERT_STATUS_DATE_INVALID;
MapNetErrorToCertStatus(ERR_CERT_DATE_INVALID);
details.is_fatal_cert_error = true; details.is_fatal_cert_error = true;
CompleteCryptoHandshake(); CompleteCryptoHandshake();
session_->OnProofVerifyDetailsAvailable(details); session_->OnProofVerifyDetailsAvailable(details);
......
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