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

Issue 1131573005: Revert of Use cert config options in SSLServerSocketOpenSSL. (Closed)

Created:
5 years, 7 months ago by benwells
Modified:
5 years, 7 months ago
Reviewers:
Sergey Ulanov, davidben
CC:
chromium-reviews, cbentzel+watch_chromium.org, chromoting-reviews_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Use cert config options in SSLServerSocketOpenSSL. (patchset #4 id:80001 of https://codereview.chromium.org/1138813003/) Reason for revert: It seems like this change has caused new leaks on Linux and ChromeOS. First build it appeared: http://build.chromium.org/p/chromium.memory.fyi/builders/Linux%20Tests%20%28valgrind%29%281%29/builds/41241 Log output: Memcheck:Leak fun:calloc fun:PORT_ZAlloc_Util fun:ConvertToSID fun:ServerSessionIDLookup fun:ssl3_HandleClientHello fun:ssl3_HandleHandshakeMessage fun:ssl3_HandleHandshake fun:ssl3_HandleRecord fun:ssl3_GatherCompleteHandshake fun:SSL_ForceHandshake fun:_ZN3net18SSLServerSocketNSS11DoHandshakeEv fun:_ZN3net18SSLServerSocketNSS15DoHandshakeLoopEi fun:_ZN3net18SSLServerSocketNSS21OnHandshakeIOCompleteEi fun:_ZN3net18SSLServerSocketNSS14OnRecvCompleteEi fun:_ZN3net18SSLServerSocketNSS18BufferRecvCompleteEi This might be tickling some bug in underlying libraries, or it might be a problem with the change itself. You can reproduce the leak by running valgrind and running all the SSLServerSocket tests. I didn't narrow down which test. See https://www.chromium.org/developers/how-tos/using-valgrind for more details on using valgrind. Original issue's description: > Use cipher suite config options in SSLServerSocketOpenSSL. > > Previously SSLServerSocketOpenSSL was ignoring disabled_cipher_suites > list and require_forward_secrecy flag from SSLConfig. Fixed > SSLServerSocketOpenSSL to trim the list of cipher suites used in BoringSSL. > > BUG=481163 > > Committed: https://crrev.com/d0eae58087e6f45088d6ef349d9ebaa2da450ea1 > Cr-Commit-Position: refs/heads/master@{#329528} TBR=davidben@chromium.org,sergeyu@chromium.org NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=481163 Committed: https://crrev.com/d0f1ada67a97c7cfa29cea88ee4b5b07e6aeb86d Cr-Commit-Position: refs/heads/master@{#329594}

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -97 lines) Patch
M net/socket/ssl_server_socket_nss.cc View 1 chunk +1 line, -1 line 0 comments Download
M net/socket/ssl_server_socket_openssl.cc View 2 chunks +0 lines, -43 lines 0 comments Download
M net/socket/ssl_server_socket_unittest.cc View 2 chunks +10 lines, -46 lines 0 comments Download
M net/ssl/ssl_config.h View 1 chunk +4 lines, -4 lines 0 comments Download
M net/ssl/ssl_config.cc View 1 chunk +1 line, -1 line 0 comments Download
M net/ssl/ssl_config_service.cc View 1 chunk +2 lines, -1 line 0 comments Download
M remoting/protocol/ssl_hmac_channel_authenticator.cc View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 4 (0 generated)
benwells
Created Revert of Use cert config options in SSLServerSocketOpenSSL.
5 years, 7 months ago (2015-05-13 06:02:10 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1131573005/1
5 years, 7 months ago (2015-05-13 06:02:19 UTC) #2
commit-bot: I haz the power
Committed patchset #1 (id:1)
5 years, 7 months ago (2015-05-13 06:03:13 UTC) #3
commit-bot: I haz the power
5 years, 7 months ago (2015-05-13 06:03:51 UTC) #4
Message was sent while issue was closed.
Patchset 1 (id:??) landed as
https://crrev.com/d0f1ada67a97c7cfa29cea88ee4b5b07e6aeb86d
Cr-Commit-Position: refs/heads/master@{#329594}

Powered by Google App Engine
This is Rietveld 408576698