Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(326)

Issue 509273002: Detect SHA-1 when it appears in certificate chains (Closed)

Created:
6 years, 3 months ago by Ryan Sleevi
Modified:
6 years, 1 month ago
Reviewers:
bannercrown, davidben
CC:
chromium-reviews, cbentzel+watch_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@cert_status_extended
Project:
chromium
Visibility:
Public.

Description

Detect SHA-1 when it appears in certificate chains BUG=401365 Committed: https://crrev.com/b92e6f5c2a5271850a39afa1254d41ccc34ec5e6 Cr-Commit-Position: refs/heads/master@{#297307}

Patch Set 1 #

Total comments: 5

Patch Set 2 : Rebased #

Patch Set 3 : Correct Android comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+98 lines, -31 lines) Patch
M net/cert/cert_verify_proc_android.cc View 1 2 2 chunks +32 lines, -0 lines 0 comments Download
M net/cert/cert_verify_proc_mac.cc View 1 chunk +7 lines, -0 lines 0 comments Download
M net/cert/cert_verify_proc_nss.cc View 1 chunk +6 lines, -0 lines 0 comments Download
M net/cert/cert_verify_proc_openssl.cc View 1 chunk +7 lines, -1 line 0 comments Download
M net/cert/cert_verify_proc_unittest.cc View 1 9 chunks +35 lines, -28 lines 0 comments Download
M net/cert/cert_verify_proc_win.cc View 1 1 chunk +7 lines, -0 lines 0 comments Download
M net/cert/cert_verify_result.h View 1 chunk +2 lines, -1 line 0 comments Download
M net/cert/cert_verify_result.cc View 1 chunk +2 lines, -1 line 0 comments Download

Messages

Total messages: 11 (1 generated)
Ryan Sleevi
rsleevi@chromium.org changed reviewers: + davidben@chromium.org
6 years, 3 months ago (2014-08-28 04:17:50 UTC) #1
Ryan Sleevi
David: Would you take a look at this? Please pay particular attention to the Android ...
6 years, 3 months ago (2014-08-28 04:17:50 UTC) #2
davidben
lgtm with comments: > The slight concern (and perhaps you have more state) is ensuring ...
6 years, 3 months ago (2014-08-28 19:42:08 UTC) #3
Ryan Sleevi
https://codereview.chromium.org/509273002/diff/1/net/cert/cert_verify_proc_nss.cc File net/cert/cert_verify_proc_nss.cc (right): https://codereview.chromium.org/509273002/diff/1/net/cert/cert_verify_proc_nss.cc#newcode208 net/cert/cert_verify_proc_nss.cc:208: case SEC_OID_ANSIX9_DSA_SIGNATURE_WITH_SHA1_DIGEST: On 2014/08/28 19:42:08, David Benjamin wrote: > ...
6 years, 2 months ago (2014-09-26 20:18:43 UTC) #4
bannercrown_gmail.com
> > Enter code here...X509* > On Friday, September 26, 2014 1:18:44 PM UTC-7, rsl...@chromium.org ...
6 years, 2 months ago (2014-09-28 22:05:05 UTC) #5
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/509273002/40001
6 years, 2 months ago (2014-09-29 22:46:29 UTC) #7
commit-bot: I haz the power
Committed patchset #3 (id:40001) as 581cc71be3c77845167f687eed5cbbdeb841cde5
6 years, 2 months ago (2014-09-29 23:48:11 UTC) #8
commit-bot: I haz the power
Patchset 3 (id:??) landed as https://crrev.com/b92e6f5c2a5271850a39afa1254d41ccc34ec5e6 Cr-Commit-Position: refs/heads/master@{#297307}
6 years, 2 months ago (2014-09-29 23:49:04 UTC) #9
Luke Faraone
On 2014/08/28 04:17:50, Ryan Sleevi (ooo until 11-4) wrote: > The slight concern (and perhaps ...
6 years, 1 month ago (2014-11-03 21:28:21 UTC) #10
Ryan Sleevi
6 years, 1 month ago (2014-11-03 22:04:26 UTC) #11
Message was sent while issue was closed.
On 2014/11/03 21:28:21, Luke Faraone wrote:
> On 2014/08/28 04:17:50, Ryan Sleevi (ooo until 11-4) wrote:
> > The slight concern (and perhaps you have more state) is ensuring that we
don't
> > count the trust anchor (which may not be the root), but I don't think
> Android's
> > cert verification supports this.
> 
> I'm pretty sure this code doesn't DTRT; On Ubuntu 14.10 running 39.0.2171.42
> beta (64-bit), https://zulip.com/ is marked as using "outdated security
> settings", yet SSLLabs indicates[1] we're sending only RSA-2048 certificates
> signed with SHA256withRSA.
> 
> [1]: https://www.ssllabs.com/ssltest/analyze.html?d=zulip.com&hideResults=on

Please don't use code reviews for bug reports. Please file a new bug.

Powered by Google App Engine
This is Rietveld 408576698