New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 621684 link

Starred by 4 users
Status: Fixed
Owner:
mattm@chromium.org
Closed: Sep 2016
Cc:
rsesek@chromium.org
rsleevi@chromium.org
█ f...@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Mac
Pri: 2
Type: Bug



Sign in to add a comment

OSX certificate hostname check can negate the loop&chop workaround

Project Member Reported by mattm@chromium.org, Jun 20 2016 
In OSX certificate verification we currently do a strange dance where we ask the OS to verify the site's hostname, and then we ignore the result of its check and do a more spec-compliant hostname check ourself.

I discovered while working on  issue 570909  that this can in some cases break the looping & chopping workaround in CertVerifyProcMac::VerifyInternal, since the OSX hostname check can fail in cases where ours would succeed. All the iterations of the loop fail, making it pointless.

 Issue 92678  documents some of the history behind the hostname check. Briefly, giving the OS the hostname is required to handle certificates that were whitelisted within Safari, since the whitelisting is limited to a given hostname. There is a workaround of using Keychain Access to set the preferences for the certificate, rather than through Safari.

Given that this is a fairly niche case with an easy work around, I'd like to just remove the OS hostname check. (The alternative would be some fairly messy refactoring of CertVerifyProcMac to apply the hostname check override in each iteration of the loop instead of just at the end.)

Adrienne: wdyt?

Comment 1 by rsleevi@chromium.org, Jun 20 2016

Cc: rsesek@chromium.org
CC'ing rsesek as well as FYI.

I don't think "match Safari" is necessarily what we want here, and certainly "Trust Safari's exceptions in Chrome" is a weird thing. To me, this struck me a bit like our "Use Keychain so we could use Safari autocomplete as well", which we nuked because the complexity cost/practical reality made it impractical.

I'm strongly in favor of nuking it - same way we wouldn't want exceptions just randomly added to Chrome by other programs, w/o the same UX or metrics measurements done for all the cert memory stuff.

Comment 2 by f...@chromium.org, Jun 21 2016 

> On Mac, respect certificate user trust settings created by Safari.

> When using Safari to explicitly trust a certificate for a site,
> Safari creates the trust settings keyed on the hostname. If the
> same trust settings are created in Keychain Access, these settings
> are made independent of any particular site.

For background: how do you set this up in Safari? I see that clicking through a certificate doesn't trigger this behavior.

Comment 3 by rsleevi@chromium.org, Jun 21 2016 

When the certificate error prompt comes up, you open the trust panel details, and you can change to an "Always Trust" for an X.509 or SSL policy. It then adds an entry to Keychain keyed off the hostname you tried to access.

Comment 4 by rsleevi@chromium.org, Jun 21 2016 

Which is:
Show Certificate
Click Trust Carat
Change one of the X.509 or SSL policies

Comment 5 by f...@chromium.org, Jun 21 2016 

I'm also in favor of removing this behavior, I think it's weird for Chrome to respect these exceptions when we have our own exception policy.

Comment 6 by rsesek@chromium.org, Jun 21 2016 

I also support removing the native OS hostname check.

Comment 7 by eroman@chromium.org, Jun 21 2016

Components: -Internals>Network>SSL Internals>Network>Certificate
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 15 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9cedf75377d817c6b32a01f1d30fbe10663b8bb8

commit 9cedf75377d817c6b32a01f1d30fbe10663b8bb8
Author: mattm <mattm@chromium.org>
Date: Thu Sep 15 00:53:23 2016

CertVerifyProcMac: Add Keychain re-ordering hack, check CRLsets in path pruning loop.

This also removes the native hostname checking workarounds.

BUG= 570909 ,588789, 621684 

Review-Url: https://codereview.chromium.org/2101303005
Cr-Commit-Position: refs/heads/master@{#418732}

[modify] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/BUILD.gn
[modify] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/cert/cert_verify_proc_mac.cc
[modify] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/cert/cert_verify_proc_unittest.cc
[add] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/cert/test_keychain_search_list_mac.cc
[add] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/cert/test_keychain_search_list_mac.h
[modify] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/data/ssl/certificates/README
[add] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/data/ssl/certificates/multi-root-BFE.keychain
[add] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/data/ssl/certificates/tripadvisor-verisign-chain.pem
[add] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/data/ssl/certificates/verisign_class3_g5_crosssigned-trusted.keychain
[add] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/data/ssl/certificates/verisign_class3_g5_crosssigned.pem
[add] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/data/ssl/scripts/generate-keychain.sh
[add] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/data/ssl/scripts/generate-multi-root-BFE-keychain.sh
[add] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/data/ssl/scripts/generate-verisign_class3_g5_crosssigned-trusted-keychain.sh
[modify] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/net.gypi
[modify] https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8/net/url_request/url_request_unittest.cc

Comment 9 by mattm@chromium.org, Sep 15 2016

Status: Fixed (was: Assigned)
Project Member

Comment 10 by bugdroid1@chromium.org, Sep 16 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/03a7397bd4ed8c059a7f5977a7672769fb783626

commit 03a7397bd4ed8c059a7f5977a7672769fb783626
Author: davidben <davidben@chromium.org>
Date: Fri Sep 16 19:21:15 2016

Revert of CertVerifyProcMac: Add Keychain re-ordering hack, check CRLsets in path pruning loop. (patchset #11 id:300001 of https://codereview.chromium.org/2101303005/ )

Reason for revert:
This breaks verification on OS X 10.12 and probably needs some further investigation.

Original issue's description:
> CertVerifyProcMac: Add Keychain re-ordering hack, check CRLsets in path pruning loop.
>
> This also removes the native hostname checking workarounds.
>
> BUG= 570909 ,588789, 621684 
>
> Committed: https://crrev.com/9cedf75377d817c6b32a01f1d30fbe10663b8bb8
> Cr-Commit-Position: refs/heads/master@{#418732}

TBR=rsleevi@chromium.org,mattm@chromium.org
# Not skipping CQ checks because original CL landed more than 1 days ago.
BUG= 570909 ,588789, 621684 , 647241 

Review-Url: https://codereview.chromium.org/2347893002
Cr-Commit-Position: refs/heads/master@{#419245}

[modify] https://crrev.com/03a7397bd4ed8c059a7f5977a7672769fb783626/net/BUILD.gn
[modify] https://crrev.com/03a7397bd4ed8c059a7f5977a7672769fb783626/net/cert/cert_verify_proc_mac.cc
[modify] https://crrev.com/03a7397bd4ed8c059a7f5977a7672769fb783626/net/cert/cert_verify_proc_unittest.cc
[delete] https://crrev.com/afc2a8c12033ede8c9cedb501299e47270f75cb8/net/cert/test_keychain_search_list_mac.cc
[delete] https://crrev.com/afc2a8c12033ede8c9cedb501299e47270f75cb8/net/cert/test_keychain_search_list_mac.h
[modify] https://crrev.com/03a7397bd4ed8c059a7f5977a7672769fb783626/net/data/ssl/certificates/README
[delete] https://crrev.com/afc2a8c12033ede8c9cedb501299e47270f75cb8/net/data/ssl/certificates/multi-root-BFE.keychain
[delete] https://crrev.com/afc2a8c12033ede8c9cedb501299e47270f75cb8/net/data/ssl/certificates/tripadvisor-verisign-chain.pem
[delete] https://crrev.com/afc2a8c12033ede8c9cedb501299e47270f75cb8/net/data/ssl/certificates/verisign_class3_g5_crosssigned-trusted.keychain
[delete] https://crrev.com/afc2a8c12033ede8c9cedb501299e47270f75cb8/net/data/ssl/certificates/verisign_class3_g5_crosssigned.pem
[delete] https://crrev.com/afc2a8c12033ede8c9cedb501299e47270f75cb8/net/data/ssl/scripts/generate-keychain.sh
[delete] https://crrev.com/afc2a8c12033ede8c9cedb501299e47270f75cb8/net/data/ssl/scripts/generate-multi-root-BFE-keychain.sh
[delete] https://crrev.com/afc2a8c12033ede8c9cedb501299e47270f75cb8/net/data/ssl/scripts/generate-verisign_class3_g5_crosssigned-trusted-keychain.sh
[modify] https://crrev.com/03a7397bd4ed8c059a7f5977a7672769fb783626/net/net.gypi
[modify] https://crrev.com/03a7397bd4ed8c059a7f5977a7672769fb783626/net/url_request/url_request_unittest.cc
Project Member

Comment 11 by bugdroid1@chromium.org, Sep 23 2016 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/af868e7814f899b9be9d2d3f8231d02b9b4d5f64

commit af868e7814f899b9be9d2d3f8231d02b9b4d5f64
Author: mattm <mattm@chromium.org>
Date: Fri Sep 23 23:25:20 2016

Try #2: CertVerifyProcMac: Add Keychain re-ordering hack, check CRLsets in path pruning loop.

This also removes the native hostname checking workarounds.

BUG= 570909 ,588789, 621684 , 647241 

Review-Url: https://codereview.chromium.org/2362533002
Cr-Commit-Position: refs/heads/master@{#420782}

[modify] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/BUILD.gn
[modify] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/cert/cert_verify_proc_mac.cc
[modify] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/cert/cert_verify_proc_unittest.cc
[add] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/cert/test_keychain_search_list_mac.cc
[add] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/cert/test_keychain_search_list_mac.h
[modify] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/cert/x509_util_mac.cc
[modify] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/data/ssl/certificates/README
[add] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/data/ssl/certificates/multi-root-BFE.keychain
[add] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/data/ssl/certificates/tripadvisor-verisign-chain.pem
[add] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/data/ssl/certificates/verisign_class3_g5_crosssigned-trusted.keychain
[add] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/data/ssl/certificates/verisign_class3_g5_crosssigned.pem
[add] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/data/ssl/scripts/generate-keychain.sh
[add] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/data/ssl/scripts/generate-multi-root-BFE-keychain.sh
[add] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/data/ssl/scripts/generate-verisign_class3_g5_crosssigned-trusted-keychain.sh
[modify] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/net.gypi
[modify] https://crrev.com/af868e7814f899b9be9d2d3f8231d02b9b4d5f64/net/url_request/url_request_unittest.cc
►

Sign in to add a comment