Skip to content

Commit

Permalink
Fix seg fault crash of quic_client with --disable-certificate-verific…
Browse files Browse the repository at this point in the history
…ation

When quic_client is executed with --disable-certificate-verification,
it crashes with segmentation fault due to SCT check in
ProofVerifierChromium. Instead, FakeProofVerifier is added in order to
pass VerifyProof when a certification verification is disabled.

BUG=

Review-Url: https://codereview.chromium.org/2441053002
Cr-Commit-Position: refs/heads/master@{#427003}
  • Loading branch information
shigeki authored and Commit bot committed Oct 23, 2016
1 parent f09660d commit bf71e63
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 34 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,7 @@ Shaobo Yan <shaobo.yan@intel.com>
Shashi Kumar <sk.kumar@samsung.com>
Sherry Mou <wenjinm@amazon.com>
Shez Baig <sbaig1@bloomberg.net>
Shigeki Ohtsu <shigeki.ohtsu@gmail.com>
Shiliu Wang <aofdwsl@gmail.com>
Shiliu Wang <shiliu.wang@intel.com>
Shilpa Shri <shilpa.shri@samsung.com>
Expand Down
50 changes: 33 additions & 17 deletions net/tools/quic/quic_client_bin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ using net::CertVerifier;
using net::CTPolicyEnforcer;
using net::CTVerifier;
using net::MultiLogCTVerifier;
using net::ProofVerifier;
using net::ProofVerifierChromium;
using net::SpdyHeaderBlock;
using net::TransportSecurityState;
Expand Down Expand Up @@ -105,19 +106,33 @@ bool FLAGS_redirect_is_success = true;
// Initial MTU of the connection.
int32_t FLAGS_initial_mtu = 0;

class FakeCertVerifier : public net::CertVerifier {
class FakeProofVerifier : public ProofVerifier {
public:
int Verify(const RequestParams& params,
net::CRLSet* crl_set,
net::CertVerifyResult* verify_result,
const net::CompletionCallback& callback,
std::unique_ptr<Request>* out_req,
const net::NetLogWithSource& net_log) override {
return net::OK;
net::QuicAsyncStatus VerifyProof(
const string& hostname,
const uint16_t port,
const string& server_config,
net::QuicVersion quic_version,
StringPiece chlo_hash,
const vector<string>& certs,
const string& cert_sct,
const string& signature,
const net::ProofVerifyContext* context,
string* error_details,
std::unique_ptr<net::ProofVerifyDetails>* details,
std::unique_ptr<net::ProofVerifierCallback> callback) override {
return net::QUIC_SUCCESS;
}

// Returns true if this CertVerifier supports stapled OCSP responses.
bool SupportsOCSPStapling() override { return false; }
net::QuicAsyncStatus VerifyCertChain(
const std::string& hostname,
const std::vector<std::string>& certs,
const net::ProofVerifyContext* verify_context,
std::string* error_details,
std::unique_ptr<net::ProofVerifyDetails>* verify_details,
std::unique_ptr<net::ProofVerifierCallback> callback) override {
return net::QUIC_SUCCESS;
}
};

int main(int argc, char* argv[]) {
Expand Down Expand Up @@ -245,18 +260,19 @@ int main(int argc, char* argv[]) {
}
// For secure QUIC we need to verify the cert chain.
std::unique_ptr<CertVerifier> cert_verifier(CertVerifier::CreateDefault());
if (line->HasSwitch("disable-certificate-verification")) {
cert_verifier.reset(new FakeCertVerifier());
}
std::unique_ptr<TransportSecurityState> transport_security_state(
new TransportSecurityState);
transport_security_state.reset(new TransportSecurityState);
std::unique_ptr<CTVerifier> ct_verifier(new MultiLogCTVerifier());
std::unique_ptr<CTPolicyEnforcer> ct_policy_enforcer(new CTPolicyEnforcer());
std::unique_ptr<net::ProofVerifierChromium> proof_verifier(
new ProofVerifierChromium(cert_verifier.get(), ct_policy_enforcer.get(),
transport_security_state.get(),
ct_verifier.get()));
std::unique_ptr<ProofVerifier> proof_verifier;
if (line->HasSwitch("disable-certificate-verification")) {
proof_verifier.reset(new FakeProofVerifier());
} else {
proof_verifier.reset(new ProofVerifierChromium(
cert_verifier.get(), ct_policy_enforcer.get(),
transport_security_state.get(), ct_verifier.get()));
}
net::QuicClient client(net::IPEndPoint(ip_addr, port), server_id, versions,
&epoll_server, std::move(proof_verifier));
client.set_initial_max_packet_length(
Expand Down
50 changes: 33 additions & 17 deletions net/tools/quic/quic_simple_client_bin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ using net::CertVerifier;
using net::CTPolicyEnforcer;
using net::CTVerifier;
using net::MultiLogCTVerifier;
using net::ProofVerifier;
using net::ProofVerifierChromium;
using net::TransportSecurityState;
using std::cout;
Expand Down Expand Up @@ -105,19 +106,33 @@ bool FLAGS_redirect_is_success = true;
// Initial MTU of the connection.
int32_t FLAGS_initial_mtu = 0;

class FakeCertVerifier : public net::CertVerifier {
class FakeProofVerifier : public ProofVerifier {
public:
int Verify(const RequestParams& params,
net::CRLSet* crl_set,
net::CertVerifyResult* verify_result,
const net::CompletionCallback& callback,
std::unique_ptr<Request>* out_req,
const net::NetLogWithSource& net_log) override {
return net::OK;
net::QuicAsyncStatus VerifyProof(
const string& hostname,
const uint16_t port,
const string& server_config,
net::QuicVersion quic_version,
StringPiece chlo_hash,
const vector<string>& certs,
const string& cert_sct,
const string& signature,
const net::ProofVerifyContext* context,
string* error_details,
std::unique_ptr<net::ProofVerifyDetails>* details,
std::unique_ptr<net::ProofVerifierCallback> callback) override {
return net::QUIC_SUCCESS;
}

// Returns true if this CertVerifier supports stapled OCSP responses.
bool SupportsOCSPStapling() override { return false; }
net::QuicAsyncStatus VerifyCertChain(
const std::string& hostname,
const std::vector<std::string>& certs,
const net::ProofVerifyContext* verify_context,
std::string* error_details,
std::unique_ptr<net::ProofVerifyDetails>* verify_details,
std::unique_ptr<net::ProofVerifierCallback> callback) override {
return net::QUIC_SUCCESS;
}
};

int main(int argc, char* argv[]) {
Expand Down Expand Up @@ -246,17 +261,18 @@ int main(int argc, char* argv[]) {
}
// For secure QUIC we need to verify the cert chain.
std::unique_ptr<CertVerifier> cert_verifier(CertVerifier::CreateDefault());
if (line->HasSwitch("disable-certificate-verification")) {
cert_verifier.reset(new FakeCertVerifier());
}
std::unique_ptr<TransportSecurityState> transport_security_state(
new TransportSecurityState);
std::unique_ptr<CTVerifier> ct_verifier(new MultiLogCTVerifier());
std::unique_ptr<CTPolicyEnforcer> ct_policy_enforcer(new CTPolicyEnforcer());
std::unique_ptr<ProofVerifierChromium> proof_verifier(
new ProofVerifierChromium(cert_verifier.get(), ct_policy_enforcer.get(),
transport_security_state.get(),
ct_verifier.get()));
std::unique_ptr<ProofVerifier> proof_verifier;
if (line->HasSwitch("disable-certificate-verification")) {
proof_verifier.reset(new FakeProofVerifier());
} else {
proof_verifier.reset(new ProofVerifierChromium(
cert_verifier.get(), ct_policy_enforcer.get(),
transport_security_state.get(), ct_verifier.get()));
}
net::QuicSimpleClient client(net::IPEndPoint(ip_addr, port), server_id,
versions, std::move(proof_verifier));
client.set_initial_max_packet_length(
Expand Down

0 comments on commit bf71e63

Please sign in to comment.