Perform CRLSet evaluation during Path Building on NSS

Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*
TBR=mattm@chromium.org

Committed: https://crrev.com/56139459f834b6b4ac3aad37b466d9ae997ff15c
Cr-Commit-Position: refs/heads/master@{#380590}

[Ryan Sleevi, 2016-02-24 03:18:40]

rsleevi@chromium.org changed reviewers:
+ eroman@chromium.org

[Ryan Sleevi, 2016-02-24 03:18:41]

CC davidben since he had a lot of strong opinions on the Windows CL (and
rightfully so)

Eric, would you mind taking a look at this? I'm trying to spread the review
knowledge around - and it's good to get a fresh reviewer to ask "WTF" to make
sure the code is self-documented enough.

[Ryan Sleevi, 2016-02-24 03:19:25]

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

[commit-bot: I haz the power, 2016-02-24 03:20:53]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1724413002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1724413002/1

[commit-bot: I haz the power, 2016-02-24 03:38:39]

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

[commit-bot: I haz the power, 2016-02-24 03:38:40]

Dry run: Try jobs failed on following builders:
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[eroman, 2016-02-25 03:12:02]

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
File net/cert/cert_verify_proc_nss.cc (right):

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:366:
intermediates.erase(intermediates.begin());
Could have avoided the erase().

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:385: if (!is_chain_valid_arg) {
Is this actually reachable?

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:394: if (args->crl_set)
curlies

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:406: if (!args->next_callback ||
!args->next_callback->isChainValid)
Is it expected that isChainValid can be NULL, or is the second part just
defensive?

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:889: check_chain_revocation_args.revoked_chain
= nullptr;
could have omitted this unless you are trying to be explicit in naming all the
members.

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:970: crl_set_result =
CheckRevocationWithCRLSet(
Wasn't this already called by PKIXVerifyCert() on the constructed path, why call
it again?

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:978: check_chain_revocation_args.revoked_chain)
{
It doesn't look like the revoked_chain is every consumed, except for its
existence.
Why not just use a boolean then?

[Ryan Sleevi, 2016-03-01 22:44:58]

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
File net/cert/cert_verify_proc_nss.cc (right):

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:366:
intermediates.erase(intermediates.begin());
On 2016/02/25 03:12:02, eroman wrote:
> Could have avoided the erase().

I'm not sure your remark here, or what you're suggesting.

As best I can tell, you're suggesting to track leaf (CERT_LIST_HEAD) vs the rest
of the intermediates, to avoid the erase? If so, that seems significantly more
complicated logic, but perhaps I'm missing something obvious.

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:385: if (!is_chain_valid_arg) {
On 2016/02/25 03:12:02, eroman wrote:
> Is this actually reachable?

It's NSS; the guarantees the API provides are undocumented.

That said, moved it to a CHECK

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:406: if (!args->next_callback ||
!args->next_callback->isChainValid)
On 2016/02/25 03:12:02, eroman wrote:
> Is it expected that isChainValid can be NULL, or is the second part just
> defensive?

Yes, it is; this matches the internal checks of NSS with respect to invoking its
callbacks. This is due to how the structure behaves.

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:889: check_chain_revocation_args.revoked_chain
= nullptr;
On 2016/02/25 03:12:01, eroman wrote:
> could have omitted this unless you are trying to be explicit in naming all the
> members.

crl_set and next_callback are pointer types; they aren't guaranteed to be
zero-initialized without an explicit constructor. But hey, C++11 means we can
use default initializers - which I did

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:970: crl_set_result =
CheckRevocationWithCRLSet(
On 2016/02/25 03:12:02, eroman wrote:
> Wasn't this already called by PKIXVerifyCert() on the constructed path, why
call
> it again?

As with the Windows code, we're taking a defense in depth approach.

But to directly answer your question: It's supposed to be, yes. But I don't
trust NSS.

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:978: check_chain_revocation_args.revoked_chain)
{
On 2016/02/25 03:12:02, eroman wrote:
> It doesn't look like the revoked_chain is every consumed, except for its
> existence.
> Why not just use a boolean then?

You're right; I was originally going to sub in the revoked chain since it would
have passed all the verification checks, but given that we can't presently
guarantee the behaviour that a invalid chain will be available across all
platforms, that was an unnecessary optimization. Switched to a bool.

[eroman, 2016-03-02 00:05:07]

lgtm

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
File net/cert/cert_verify_proc_nss.cc (right):

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:366:
intermediates.erase(intermediates.begin());
On 2016/03/01 22:44:57, Ryan Sleevi wrote:
> On 2016/02/25 03:12:02, eroman wrote:
> > Could have avoided the erase().
> 
> I'm not sure your remark here, or what you're suggesting.
> 
> As best I can tell, you're suggesting to track leaf (CERT_LIST_HEAD) vs the
rest
> of the intermediates, to avoid the erase? If so, that seems significantly more
> complicated logic, but perhaps I'm missing something obvious.

In the construction of intermediates vector above, you could have avoiding
pushing the leaf node in since it is just popped here.

Not worth it if these are small vectors and you think it will complicate your
code.

https://codereview.chromium.org/1724413002/diff/1/net/cert/cert_verify_proc_n...
net/cert/cert_verify_proc_nss.cc:889: check_chain_revocation_args.revoked_chain
= nullptr;
On 2016/03/01 22:44:58, Ryan Sleevi wrote:
> On 2016/02/25 03:12:01, eroman wrote:
> > could have omitted this unless you are trying to be explicit in naming all
the
> > members.
> 
> crl_set and next_callback are pointer types; they aren't guaranteed to be
> zero-initialized without an explicit constructor. But hey, C++11 means we can
> use default initializers - which I did

I was referring to revoked_chain, which is a scoped_refptr<>.

That is default initialized 4 lines earlier.
So it is guaranteed to already be nullptr.

Shrug, no longer relevant in new patchset.

https://codereview.chromium.org/1724413002/diff/40001/net/cert/cert_verify_pr...
File net/cert/cert_verify_proc_nss.cc (right):

https://codereview.chromium.org/1724413002/diff/40001/net/cert/cert_verify_pr...
net/cert/cert_verify_proc_nss.cc:929: trust_anchors.get(), &crlset_callback,
cvout);
I think it would be better to explicitly reset |check_chain_revocation_args|
before each call to PKIXVerifyCert(), rather than relying on any mutations from
the previous call to not affect the final value of |check_chain_revocation_args|
which is tested for later.

Maybe just check_chain_revocation_args.Reset() before each PKIXVerifyCert() call
that links to it.

I can't tell if the current code not re-setting was_revoked between calls leads
to problems. At the very least it is subtle.

[Ryan Sleevi, 2016-03-02 23:45:14]

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

[commit-bot: I haz the power, 2016-03-02 23:45:50]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1724413002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1724413002/60001

[commit-bot: I haz the power, 2016-03-03 00:22:32]

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

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

Dry run: Try jobs failed on following builders:
  cast_shell_linux on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)

[Ryan Sleevi, 2016-03-03 00:51:37]

I am an idiot

[Ryan Sleevi, 2016-03-03 22:14:44]

The CQ bit was checked by rsleevi@chromium.org

[Ryan Sleevi, 2016-03-03 22:14:44]

The patchset sent to the CQ was uploaded after l-g-t-m from eroman@chromium.org
Link to the patchset: https://codereview.chromium.org/1724413002/#ps80001
(title: "I am an idiot")

[commit-bot: I haz the power, 2016-03-03 22:15:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1724413002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1724413002/80001

[commit-bot: I haz the power, 2016-03-03 22:25:06]

Committed patchset #5 (id:80001)

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

Description was changed from

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*
==========

to

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*

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

[commit-bot: I haz the power, 2016-03-03 22:26:06]

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

[benwells, 2016-03-04 05:28:50]

A revert of this CL (patchset #5 id:80001) has been created in
https://codereview.chromium.org/1762923002/ by benwells@chromium.org.

The reason for reverting is: Since this patch landed there are consistent
failures on Linux valgrind.

It isn't clear to me why this change would cause the failures, but am reverting
speculatively to see if the problems go away, as this is the only CL which seems
close to the area.

First build with failures:
https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28...

Sample output:
[ RUN      ] SSLServerSocketTest.HandshakeWithClientCert
../../net/socket/ssl_server_socket_unittest.cc:389: Failure
Value of: client_ssl_config_.client_cert
  Actual: false
Expected: true
[10870:10870:0303/174222:1424458412:ERROR:ssl_server_socket_openssl.cc(649)]
handshake failed; returned -1, SSL error code 1, net_error -107
[10870:10870:0303/174222:1424462684:ERROR:ssl_client_socket_openssl.cc(1140)]
handshake failed; returned 0, SSL error code 1, net_error -107
../../net/socket/ssl_server_socket_unittest.cc:514: Failure
Value of: client_ret
  Actual: -107
Expected: OK
Which is: 0
[  FAILED  ] SSLServerSocketTest.HandshakeWithClientCert (242 ms)
[ RUN      ] SSLServerSocketTest.HandshakeWithClientCertRequiredNotSupplied
Received signal 11 SEGV_MAPERR 000000000148
17:42:24 common.py [INFO] process ended, did not time out
17:42:24 common.py [INFO] flushing stdout
17:42:24 common.py [INFO] collecting result code
17:42:24 common.py [ERROR]
/mnt/data/b/build/slave/chromium-rel-linux-valgrind-tests-1/build/src/third_party/valgrind/linux_x64/bin/valgrind
exited with non-zero result code -9
-----------------------------------------------------
Suppressions used:
  count name
      1 bug_269278b
      2 glibc-2.5.x-on-SUSE-10.2-(PPC)-2a
      6 bug_176891a
      9 bug_87500_a (Intentional)
     26 bug_32624_c
   1639 bug_64887_a
-----------------------------------------------------
17:42:25 memcheck_analyze.py [ERROR] FAIL! There were 1 errors: 
17:42:25 memcheck_analyze.py [ERROR] 
### BEGIN MEMORY TOOL REPORT (error hash=#A693C36946E9BE71#)
Command:
/mnt/data/b/build/slave/chromium-rel-linux-valgrind-tests-1/build/src/out/Release/net_unittests
--gtest_print_time --single-process-tests
--gtest_filter=-Spdy_SpdyNetworkTransactionTest.StartTransactionOnReadCallback_0:DiskCacheEntryTest.SimpleCacheSizeChanges:DiskCacheBackendTest.FAILS_AppCacheInvalidEntry:DiskCacheBackendTest.FAILS_NewEvictionInvalidEntryWithLoad:KeygenHandlerTest.*ConcurrencyTest:DiskCacheBackendTest.NewEvictionTrimInvalidEntry:KeygenHandlerTest.FLAKY_*SmokeTest:DiskCacheBackendTest.InvalidEntryRead:DiskCacheBackendTest.FLAKY_NewEvictionInvalidEntryEnumeration:DiskCacheBackendTest.FLAKY_InvalidEntryWithLoad:DiskCacheBackendTest.FLAKY_NewEvictionInvalidEntryWithLoad:DiskCacheBackendTest.FAILS_ShutdownWithPendingCreate_Fast:DiskCacheEntryTest.FLAKY_SimpleCacheSizeChanges:KeygenHandlerTest.FAILS_*ConcurrencyTest:KeygenHandlerTest.FAILS_*SmokeTest:DiskCacheEntryTest.FAILS_SimpleCacheSizeChanges:DiskCacheBackendTest.NewEvictionInvalidEntryEnumeration:DiskCacheBackendTest.FLAKY_AppCacheInvalidEntry:DiskCacheBackendTest.FLAKY_NewEvictionTrimInvalidEntry2:DiskCacheBackendTest.AppCacheInvalidEntryWithLoad:DiskCacheBackendTest.FAILS_AppCacheInvalidEntryRead:DiskCacheBackendTest.FLAKY_InvalidEntryEnumeration:DiskCacheBackendTest.FLAKY_NewEvictionInvalidEntryRead:DiskCacheBackendTest.ShutdownWithPendingIO_Fast:DiskCacheBackendTest.FLAKY_AppCacheInvalidEntryWithLoad:DiskCacheBackendTest.FAILS_ShutdownWithPendingIO_Fast:DiskCacheBackendTest.ShutdownWithPendingCreate_Fast:DiskCacheBackendTest.FAILS_InvalidEntryEnumeration:DiskCacheBackendTest.FAILS_ShutdownWithPendingFileIO_Fast:DiskCacheBackendTest.FAILS_AppCacheInvalidEntryWithLoad:DiskCacheBackendTest.FLAKY_InvalidEntryRead:DiskCacheBackendTest.FLAKY_ShutdownWithPendingFileIO_Fast:DiskCacheBackendTest.NewEvictionInvalidEntryRead:KeygenHandlerTest.FLAKY_*ConcurrencyTest:DiskCacheBackendTest.FAILS_InvalidEntryRead:Spdy_SpdyNetworkTransactionTest.FLAKY_StartTransactionOnReadCallback_0:Spdy_SpdyNetworkTransactionTest.FAILS_StartTransactionOnReadCallback_0:KeygenHandlerTest.*SmokeTest:DiskCacheBackendTest.FAILS_InvalidEntry:EndToEndTests/EndToEndTest.*:DirectoryListerTest.FAILS_BigDirRecursiveTest:DiskCacheBackendTest.AppCacheInvalidEntryRead:DiskCacheBackendTest.FAILS_InvalidEntryWithLoad:DiskCacheBackendTest.ShutdownWithPendingFileIO_Fast:DiskCacheBackendTest.FAILS_NewEvictionInvalidEntryRead:DiskCacheEntryTest.SimpleCacheGrowData:DiskCacheBackendTest.FAILS_NewEvictionTrimInvalidEntry:DiskCacheBackendTest.FLAKY_ShutdownWithPendingCreate_Fast:DiskCacheBackendTest.FAILS_TrimInvalidEntry:DiskCacheEntryTest.FLAKY_SimpleCacheGrowData:DiskCacheBackendTest.FAILS_NewEvictionInvalidEntryEnumeration:DiskCacheBackendTest.NewEvictionInvalidEntryWithLoad:DiskCacheBackendTest.FAILS_NewEvictionInvalidEntry:DiskCacheBackendTest.FLAKY_ShutdownWithPendingIO_Fast:DirectoryListerTest.FLAKY_BigDirRecursiveTest:DiskCacheBackendTest.TrimInvalidEntry2:DiskCacheBackendTest.TrimInvalidEntry:DiskCacheBackendTest.NewEvictionInvalidEntry:DiskCacheBackendTest.FLAKY_TrimInvalidEntry2:DiskCacheEntryTest.SimpleCacheStreamAccess:DiskCacheEntryTest.FLAKY_SimpleCacheStreamAccess:DiskCacheBackendTest.InvalidEntryWithLoad:DiskCacheEntryTest.FAILS_SimpleCacheGrowData:DiskCacheEntryTest.FAILS_SimpleCacheStreamAccess:DiskCacheBackendTest.FLAKY_NewEvictionTrimInvalidEntry:DiskCacheBackendTest.FAILS_NewEvictionTrimInvalidEntry2:DiskCacheBackendTest.InvalidEntry:DiskCacheBackendTest.NewEvictionTrimInvalidEntry2:DiskCacheBackendTest.FAILS_TrimInvalidEntry2:DiskCacheBackendTest.FLAKY_InvalidEntry:DiskCacheBackendTest.FLAKY_AppCacheInvalidEntryRead:DiskCacheBackendTest.InvalidEntryEnumeration:DirectoryListerTest.BigDirRecursiveTest:DiskCacheBackendTest.FLAKY_NewEvictionInvalidEntry:DiskCacheBackendTest.AppCacheInvalidEntry:DiskCacheBackendTest.FLAKY_TrimInvalidEntry
--test-tiny-timeout=1000
InvalidRead
Invalid read of size 8
  __gnu_cxx::new_allocator<CERTCertificateStr*>::construct(CERTCertificateStr**,
CERTCertificateStr* const&)
(/mnt/data/b/build/slave/chromium-rel-linux-valgrind-tests-1/build/src/out/Release/net_unittests)
 
_ZNSt6vectorIP18CERTCertificateStrSaIS1_EE13_M_insert_auxIJRKS1_EEEvN9__gnu_cxx17__normal_iteratorIPS1_S3_EEDpOT_
(build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/vector.tcc:335)
  std::vector<CERTCertificateStr*, std::allocator<CERTCertificateStr*>
>::push_back(CERTCertificateStr* const&)
(build/linux/debian_wheezy_amd64-sysroot/usr/lib/gcc/x86_64-linux-gnu/4.6/../../../../include/c++/4.6/bits/stl_vector.h:834)
  net::X509Certificate::IsIssuedByEncoded(std::vector<std::basic_string<char,
std::char_traits<char>, std::allocator<char> >,
std::allocator<std::basic_string<char, std::char_traits<char>,
std::allocator<char> > > > const&) (net/cert/x509_certificate_nss.cc:128)
 
net::SSLServerSocketTest_HandshakeWithClientCertRequiredNotSupplied_Test::TestBody()
(net/socket/ssl_server_socket_unittest.cc:550)
Address 0x148 is not stack'd, malloc'd or (recently) free'd
Suppression (error hash=#A693C36946E9BE71#):
  For more info on using suppressions see
http://dev.chromium.org/developers/tree-sheriffs/sheriff-details-chromium/mem...
{
   <insert_a_suppression_name_here>
   Memcheck:Unaddressable
   fun:_ZN9__gnu_cxx13new_allocatorIP18CERTCertificateStrE9constructEPS2_RKS2_
  
fun:_ZNSt6vectorIP18CERTCertificateStrSaIS1_EE13_M_insert_auxIJRKS1_EEEvN9__gnu_cxx17__normal_iteratorIPS1_S3_EEDpOT_
   fun:_ZNSt6vectorIP18CERTCertificateStrSaIS1_EE9push_backERKS1_
   fun:_ZN3net15X509Certificate17IsIssuedByEncodedERKSt6vectorISsSaISsEE
  
fun:_ZN3net67SSLServerSocketTest_HandshakeWithClientCertRequiredNotSupplied_Test8TestBodyEv
}.

[benwells, 2016-03-04 05:34:10]

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

[benwells, 2016-03-04 05:34:11]

This also appears to have caused errors on CHromeOS valgrind:

https://build.chromium.org/p/chromium.memory.fyi/builders/Chromium%20OS%20%28...

Sample output:
NSSCertDatabaseChromeOSTest.ListCertsReadsSystemSlot:
Did not complete.

[Ryan Sleevi, 2016-03-11 02:47:19]

Description was changed from

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*

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

to

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*

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

[Ryan Sleevi, 2016-03-11 02:48:04]

Description was changed from

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*

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

to

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*
==========

[Ryan Sleevi, 2016-03-11 02:49:17]

Eric: Would you re-review? The only non-automated change is
https://codereview.chromium.org/1724413002/diff2/100001:120001/net/data/ssl/s...

TL;DR: I hate NSS. It has a bug.

The valgrind failures were just a misdirect because the SSLServerSocket
unittests weren't written very defensively, and thus de-ref'ed invalid pointers
when the test failed. That's why I sent you
https://codereview.chromium.org/1782273002/

[Ryan Sleevi, 2016-03-11 02:49:26]

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

[commit-bot: I haz the power, 2016-03-11 02:49:56]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1724413002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1724413002/120001

[commit-bot: I haz the power, 2016-03-11 02:59:21]

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

[commit-bot: I haz the power, 2016-03-11 02:59:22]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[eroman, 2016-03-11 03:11:21]

patchset7 lgtm

[Ryan Sleevi, 2016-03-11 05:23:24]

The CQ bit was checked by rsleevi@chromium.org

[commit-bot: I haz the power, 2016-03-11 05:24:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1724413002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1724413002/120001

[commit-bot: I haz the power, 2016-03-11 06:30:26]

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

[commit-bot: I haz the power, 2016-03-11 06:30:27]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[Ryan Sleevi, 2016-03-11 06:31:05]

The CQ bit was checked by rsleevi@chromium.org

[commit-bot: I haz the power, 2016-03-11 06:31:45]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1724413002/120001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1724413002/120001

[commit-bot: I haz the power, 2016-03-11 07:01:47]

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

[commit-bot: I haz the power, 2016-03-11 07:01:48]

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

[Ryan Sleevi, 2016-03-11 07:19:20]

Description was changed from

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*
==========

to

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*
TBR=mattm
==========

[Ryan Sleevi, 2016-03-11 07:19:20]

rsleevi@chromium.org changed reviewers:
+ mattm@chromium.org

[Ryan Sleevi, 2016-03-11 07:19:36]

Description was changed from

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*
TBR=mattm
==========

to

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*
TBR=mattm@chromium.org
==========

[Ryan Sleevi, 2016-03-11 07:19:58]

matt: TBRing you on the chrome/browser/chromeos/net change because it's just
updating some strings

[Ryan Sleevi, 2016-03-11 07:20:09]

The CQ bit was checked by rsleevi@chromium.org

[Ryan Sleevi, 2016-03-11 07:20:09]

The patchset sent to the CQ was uploaded after l-g-t-m from eroman@chromium.org
Link to the patchset: https://codereview.chromium.org/1724413002/#ps140001
(title: "Fix ChromeOS Test")

[commit-bot: I haz the power, 2016-03-11 07:20:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1724413002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1724413002/140001

[commit-bot: I haz the power, 2016-03-11 08:08:21]

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

[commit-bot: I haz the power, 2016-03-11 08:08:22]

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

[Ryan Sleevi, 2016-03-11 08:39:32]

The CQ bit was checked by rsleevi@chromium.org

[commit-bot: I haz the power, 2016-03-11 08:39:49]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1724413002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1724413002/140001

[commit-bot: I haz the power, 2016-03-11 10:06:56]

Description was changed from

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*
TBR=mattm@chromium.org
==========

to

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*
TBR=mattm@chromium.org
==========

[commit-bot: I haz the power, 2016-03-11 10:06:58]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-03-11 10:08:56]

Description was changed from

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*
TBR=mattm@chromium.org
==========

to

==========
Perform CRLSet evaluation during Path Building on NSS

When using NSS for certificate verification, add CRLSet checking by
injecting a revocation callback function which will examine the
CRLSet and reject the certificate. If the CRLSet does not
affirmatively reject it, continue invoking the originally supplied
application callback (such as the ChromeOS callback) and allow it
an opportunity to reject.

Because of how NSS caches virtually everything, horribly so, this
restructures the unittests to no longer depend on how the underlying
library will select the path (since with NSS, it's fundamentally
non-determistic), and instead tests that as long as a singular
certificate path is still valid and un-revoked, it can be discovered.

BUG=589336
TEST=CertVerifyProcTest.CRLSet*
TBR=mattm@chromium.org

Committed: https://crrev.com/56139459f834b6b4ac3aad37b466d9ae997ff15c
Cr-Commit-Position: refs/heads/master@{#380590}
==========

[commit-bot: I haz the power, 2016-03-11 10:08:58]

Patchset 8 (id:??) landed as
https://crrev.com/56139459f834b6b4ac3aad37b466d9ae997ff15c
Cr-Commit-Position: refs/heads/master@{#380590}

[mattm, 2016-03-12 02:20:49]

lgtm