Fix seg fault crash of quic_client with --disable-certificate-verification

Fix seg fault crash of quic_client with --disable-certificate-verification

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=

Committed: https://crrev.com/bf71e63b6ee9fd0571f7b1db837a697391273a79
Cr-Commit-Position: refs/heads/master@{#427003}

[Ryan Hamilton, 2016-10-21 18:44:17]

rch@chromium.org changed reviewers:
+ rch@chromium.org

[Ryan Hamilton, 2016-10-21 18:44:18]

This change looks good. Just a couple of comments (which apply to both files). 
Also, have you signed the contributor license agreement here:

 https://cla.developers.google.com/clas

https://codereview.chromium.org/2441053002/diff/1/net/tools/quic/quic_simple_...
File net/tools/quic/quic_simple_client_bin.cc (right):

https://codereview.chromium.org/2441053002/diff/1/net/tools/quic/quic_simple_...
net/tools/quic/quic_simple_client_bin.cc:122: };
I think you can remove this class completely, right?

https://codereview.chromium.org/2441053002/diff/1/net/tools/quic/quic_simple_...
net/tools/quic/quic_simple_client_bin.cc:285: cert_verifier.reset(new
FakeCertVerifier());
I think you don't need this line, so I'd just nuke it.

[Shigeki Ohtsu, 2016-10-22 00:37:12]

Thanks for reviewing.

On 2016/10/21 18:44:18, Ryan Hamilton wrote:
> This change looks good. Just a couple of comments (which apply to both files).

> Also, have you signed the contributor license agreement here:
> 
>  https://cla.developers.google.com/clas

Done

> 
>
https://codereview.chromium.org/2441053002/diff/1/net/tools/quic/quic_simple_...
> File net/tools/quic/quic_simple_client_bin.cc (right):
> 
>
https://codereview.chromium.org/2441053002/diff/1/net/tools/quic/quic_simple_...
> net/tools/quic/quic_simple_client_bin.cc:122: };
> I think you can remove this class completely, right?
> 
>
https://codereview.chromium.org/2441053002/diff/1/net/tools/quic/quic_simple_...
> net/tools/quic/quic_simple_client_bin.cc:285: cert_verifier.reset(new
> FakeCertVerifier());
> I think you don't need this line, so I'd just nuke it.

I added a commit to remove FakeCertVerifier. PTAL.

[Ryan Hamilton, 2016-10-22 02:26:58]

The CQ bit was checked by rch@chromium.org

[Ryan Hamilton, 2016-10-22 02:26:58]

lgtm

[commit-bot: I haz the power, 2016-10-22 02:27:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-22 03:05:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-22 03:05:33]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ryan Hamilton, 2016-10-22 03:08:20]

On 2016/10/22 00:37:12, shigeki.ohtsu wrote:
> Thanks for reviewing.
> 
> On 2016/10/21 18:44:18, Ryan Hamilton wrote:
> > This change looks good. Just a couple of comments (which apply to both
files).
> 
> > Also, have you signed the contributor license agreement here:
> > 
> >  https://cla.developers.google.com/clas
> 
> Done

Hm:

** Presubmit Warnings **
shigeki.ohtsu@gmail.com is not in AUTHORS file. If you are a new contributor,
please visit
http://www.chromium.org/developers/contributing-code and read the "Legal"
section
If you are a chromite, verify the contributor signed the CLA.

[Shigeki Ohtsu, 2016-10-22 05:44:08]

On 2016/10/22 03:08:20, Ryan Hamilton wrote:
> On 2016/10/22 00:37:12, shigeki.ohtsu wrote:
> > Thanks for reviewing.
> > 
I should have added my mail address to AUTHOR by myself. It's done.

And the style was changed with `git cl format net`. 

Please submit another job.


> > On 2016/10/21 18:44:18, Ryan Hamilton wrote:
> > > This change looks good. Just a couple of comments (which apply to both
> files).
> > 
> > > Also, have you signed the contributor license agreement here:
> > > 
> > >  https://cla.developers.google.com/clas
> > 
> > Done
> 
> Hm:
> 
> ** Presubmit Warnings **
> mailto:shigeki.ohtsu@gmail.com is not in AUTHORS file. If you are a new
contributor,
> please visit
> http://www.chromium.org/developers/contributing-code and read the "Legal"
> section
> If you are a chromite, verify the contributor signed the CLA.

[Ryan Hamilton, 2016-10-22 19:10:26]

The CQ bit was checked by rch@chromium.org

[Ryan Hamilton, 2016-10-22 19:10:27]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/2441053002/#ps40001
(title: "Fix seg fault crash of quic_client with
--disable-certificate-verification")

[commit-bot: I haz the power, 2016-10-22 19:10:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-22 19:24:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-22 19:24:49]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Ryan Hamilton, 2016-10-22 20:35:17]

https://codereview.chromium.org/2441053002/diff/40001/AUTHORS
File AUTHORS (right):

https://codereview.chromium.org/2441053002/diff/40001/AUTHORS#newcode623
AUTHORS:623: Shigeki Ohtsu <ohtsu@ohtsu.org>
I think this needs to be shigeki.ohtsu@gmail.com, since that's the author of
this CL.

[Shigeki Ohtsu, 2016-10-23 05:35:23]

> I think this needs to be mailto:shigeki.ohtsu@gmail.com, since that's the
author of
> this CL.

Okay, I changed my address to gmail and also amended the commit author address.

[Ryan Hamilton, 2016-10-23 15:28:17]

The CQ bit was checked by rch@chromium.org

[Ryan Hamilton, 2016-10-23 15:28:18]

The patchset sent to the CQ was uploaded after l-g-t-m from rch@chromium.org
Link to the patchset: https://codereview.chromium.org/2441053002/#ps80001
(title: "change mail address")

[commit-bot: I haz the power, 2016-10-23 15:28:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-23 15:44:18]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2016-10-23 15:59:09]

Description was changed from

==========
Fix seg fault crash of quic_client with --disable-certificate-verification

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=
==========

to

==========
Fix seg fault crash of quic_client with --disable-certificate-verification

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=

Committed: https://crrev.com/bf71e63b6ee9fd0571f7b1db837a697391273a79
Cr-Commit-Position: refs/heads/master@{#427003}
==========

[commit-bot: I haz the power, 2016-10-23 15:59:10]

Patchset 5 (id:??) landed as
https://crrev.com/bf71e63b6ee9fd0571f7b1db837a697391273a79
Cr-Commit-Position: refs/heads/master@{#427003}