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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1762923002: Revert of Perform CRLSet evaluation during Path Building on NSS (Closed)

Created:
4 years, 9 months ago by benwells
Modified:
4 years, 9 months ago
Reviewers:
eroman, Ryan Sleevi
CC:
chromium-reviews, cbentzel+watch_chromium.org, davidben
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Perform CRLSet evaluation during Path Building on NSS (patchset #5 id:80001 of https://codereview.chromium.org/1724413002/ ) Reason for revert: 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%28valgrind%29%281%29/builds/46538 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/memory-sheriff#TOC-Suppressing-memory-reports { <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 } Original issue's description: > 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} TBR=eroman@chromium.org,rsleevi@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=589336 Committed: https://crrev.com/cbfcd90189bcbc5fe11bb3bd4ddfc827b8de5305 Cr-Commit-Position: refs/heads/master@{#379219}

Patch Set 1 #

Messages

Total messages: 8 (2 generated)
benwells
Created Revert of Perform CRLSet evaluation during Path Building on NSS
4 years, 9 months ago (2016-03-04 05:28:50 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1762923002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1762923002/1
4 years, 9 months ago (2016-03-04 05:29:07 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
4 years, 9 months ago (2016-03-04 05:29:39 UTC) #4
commit-bot: I haz the power
Patchset 1 (id:??) landed as https://crrev.com/cbfcd90189bcbc5fe11bb3bd4ddfc827b8de5305 Cr-Commit-Position: refs/heads/master@{#379219}
4 years, 9 months ago (2016-03-04 05:31:12 UTC) #6
Ryan Sleevi
On 2016/03/04 05:31:12, commit-bot: I haz the power wrote: > Patchset 1 (id:??) landed as ...
4 years, 9 months ago (2016-03-06 16:16:33 UTC) #7
benwells
4 years, 9 months ago (2016-03-06 22:54:10 UTC) #8
Message was sent while issue was closed. 
On 2016/03/06 16:16:33, Ryan Sleevi - OOO until 3-10 wrote:
> On 2016/03/04 05:31:12, commit-bot: I haz the power wrote:
> > Patchset 1 (id:??) landed as
> > https://crrev.com/cbfcd90189bcbc5fe11bb3bd4ddfc827b8de5305
> > Cr-Commit-Position: refs/heads/master@{#379219}
> 
> Did this speculatively fix it? Will you be unreverting?

It did fix it, first build with green results on that bot is here:
https://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28...

So I won't be unreverting. If I get time while still sherrifing, I'll see if I
can figure out what's going on.

Powered by Google App Engine
This is Rietveld 408576698