Use a new dual-signed binary in safe browsing tests.

Use a new dual-signed binary in safe browsing tests.

BUG=643733
R=wfh@chromium.org

Committed: https://crrev.com/08934823edae715752cd5c501c8b56901e760982
Cr-Commit-Position: refs/heads/master@{#416909}

[grt (UTC plus 2), 2016-09-05 07:03:28]

The CQ bit was checked by grt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-05 07:03:40]

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

[grt (UTC plus 2), 2016-09-05 07:03:41]

Description was changed from

==========
Use a new dual-signed binary in safe browsing tests.

BUG=643733
R=wfh@chromium.org
==========

to

==========
Use a new dual-signed binary in safe browsing tests.

BUG=643733
R=wfh@chromium.org
==========

[grt (UTC plus 2), 2016-09-05 07:03:42]

grt@chromium.org changed reviewers:
+ nparker@chromium.org

[grt (UTC plus 2), 2016-09-05 07:03:53]

PTAL, thanks.

[commit-bot: I haz the power, 2016-09-05 07:09:51]

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

[commit-bot: I haz the power, 2016-09-05 07:09:52]

Dry run: 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...)

[grt (UTC plus 2), 2016-09-05 07:19:56]

The CQ bit was checked by grt@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-05 07:20:08]

Dry run: 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-09-05 07:25:51]

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

[commit-bot: I haz the power, 2016-09-05 07:25:52]

Dry run: 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...)

[Will Harris, 2016-09-06 16:46:04]

lgtm with a :shrug:, since the tests obviously pass still.

[Will Harris, 2016-09-06 16:55:17]

oh sorry my :shrug: is better explained with my comment enclosed.

https://codereview.chromium.org/2308403002/diff/20001/chrome/common/safe_brow...
File chrome/common/safe_browsing/binary_feature_extractor_win_unittest.cc
(right):

https://codereview.chromium.org/2308403002/diff/20001/chrome/common/safe_brow...
chrome/common/safe_browsing/binary_feature_extractor_win_unittest.cc:74:
ASSERT_EQ(1, signature_info.certificate_chain_size());
I'm surprised none of these values changed...?

[Nathan Parker, 2016-09-06 18:31:34]

lgtm

[grt (UTC plus 2), 2016-09-07 10:03:01]

Thanks for the review. I'll land this in three steps:
1. Land the new binary manually (I think the CQ still doesn't like binaries).
2. Rebase this CL and re-upload.
3. CQ this puppy.

https://codereview.chromium.org/2308403002/diff/20001/chrome/common/safe_brow...
File chrome/common/safe_browsing/binary_feature_extractor_win_unittest.cc
(right):

https://codereview.chromium.org/2308403002/diff/20001/chrome/common/safe_brow...
chrome/common/safe_browsing/binary_feature_extractor_win_unittest.cc:74:
ASSERT_EQ(1, signature_info.certificate_chain_size());
On 2016/09/06 16:55:17, Will Harris wrote:
> I'm surprised none of these values changed...?

My understanding is that this is 1 because WinVerifyTrust stops once a single
valid signature and cert chain is found. So, this isn't fetching the full set of
all signing certs in the file, but rather the first one that validates.

https://codereview.chromium.org/2308403002/diff/20001/chrome/common/safe_brow...
File chrome/common/safe_browsing/pe_image_reader_win_unittest.cc (right):

https://codereview.chromium.org/2308403002/diff/20001/chrome/common/safe_brow...
chrome/common/safe_browsing/pe_image_reader_win_unittest.cc:272: 1,
this constant doesn't change because both signatures are in a single PKCS7
SignedData blob.

[grt (UTC plus 2), 2016-09-07 10:25:53]

The CQ bit was checked by grt@chromium.org

[grt (UTC plus 2), 2016-09-07 10:25:53]

The patchset sent to the CQ was uploaded after l-g-t-m from
nparker@chromium.org, wfh@chromium.org
Link to the patchset: https://codereview.chromium.org/2308403002/#ps60001
(title: "rebase to position 416899")

[commit-bot: I haz the power, 2016-09-07 10:26:08]

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-09-07 11:36:38]

Description was changed from

==========
Use a new dual-signed binary in safe browsing tests.

BUG=643733
R=wfh@chromium.org
==========

to

==========
Use a new dual-signed binary in safe browsing tests.

BUG=643733
R=wfh@chromium.org
==========

[commit-bot: I haz the power, 2016-09-07 11:36:40]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-07 11:38:31]

Description was changed from

==========
Use a new dual-signed binary in safe browsing tests.

BUG=643733
R=wfh@chromium.org
==========

to

==========
Use a new dual-signed binary in safe browsing tests.

BUG=643733
R=wfh@chromium.org

Committed: https://crrev.com/08934823edae715752cd5c501c8b56901e760982
Cr-Commit-Position: refs/heads/master@{#416909}
==========

[commit-bot: I haz the power, 2016-09-07 11:38:32]

Patchset 4 (id:??) landed as
https://crrev.com/08934823edae715752cd5c501c8b56901e760982
Cr-Commit-Position: refs/heads/master@{#416909}