Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1774583002:
Add IsAcceptableCipher, use instead of GetDefaultCipher. (Closed)
|
Created:
4 years, 9 months ago by torbjorng (webrtc) Modified:
4 years, 9 months ago CC:
webrtc-reviews_webrtc.org, tterriberry_mozilla.com Base URL:
https://chromium.googlesource.com/external/webrtc.git@master Target Ref:
refs/pending/heads/master Project:
webrtc Visibility:
Public. |
DescriptionAdd IsAcceptableCipher, use instead of GetDefaultCipher.
The old code insists on exact cipher suite matches with hardwired expectations. It does this matching parameterized with key type (RSA vs ECDSA) and TLS version (DTLS vs TLS and version 1.0 vs 1.2).
This CL changes things to check against a white-list of cipher suites, with the check parameterized with key type (again RSA vs ECDSA). Then separately checks TLS version since the old implicit check of TLS version by means of resulting cipher suite was too blunt.
Using a white list for cipher suites isn't perfect, but it is safe and requires minimal maintenance. It allows compatibility with not just one exact version of underlying crypto lib, but any version with reasonable defaults.
The CL also re-enables critical tests which had to be disabled recently to allow a boringssl roll.
BUG=webrtc:5634
Committed: https://crrev.com/43166b8adf749c6672eca0cd4a39399cf27d4761
Cr-Commit-Position: refs/heads/master@{#11951}
Patch Set 1 #Patch Set 2 : Loop using modern C++ #Patch Set 3 : Add mask to please Windows' compilers, list another ciphersuite #Patch Set 4 : List another cipher suite #
Total comments: 4
Patch Set 5 : Remove GetDefaultSslCipherForTest and its usages #Patch Set 6 : Rebase #Patch Set 7 : Re-enable tests disabled as part of https://codereview.webrtc.org/1773543002 #Patch Set 8 : Remove kDefaultSsl* constants #
Total comments: 7
Patch Set 9 : Provide and use method for checking TLS protocol version #Patch Set 10 : Format #
Total comments: 8
Patch Set 11 : Address feedback #
Messages
Total messages: 49 (22 generated)
The CQ bit was checked by torbjorng@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1774583002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1774583002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios32_sim_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/5744) ios64_sim_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/5736) ios_arm64_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/8073) ios_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/13267) ios_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/11900) mac_baremetal on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_baremetal/builds/9621) mac_compile_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...) mac_gn_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_dbg/builds/1391) mac_gn_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_rel/builds/7622)
The CQ bit was checked by torbjorng@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1774583002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1774583002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_compile_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_compile_dbg/builds/...)
The CQ bit was checked by torbjorng@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1774583002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1774583002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_msan on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/linux_msan/builds/8371)
Description was changed from ========== Add IsAcceptableCipher, use instead of GetDefaultCipher. BUG=webrtc:5634 ========== to ========== Add IsAcceptableCipher, use instead of GetDefaultCipher. BUG=webrtc:5634 ==========
torbjorng@webrtc.org changed reviewers: + tommi@webrtc.org
Tommi, please take an quick look, and just on the opensslstreamadapter.cc changes for now. I need two variants of IsAcceptableCipher() to limit the amount of changes needed to the tests. I worry that my style there might be considered as archaic C-ish. Is there a better way? I prefer to avoid mentioning the cipher suite names twice (as symbol and as string) and therefore use the CDEF trickery. (One would perhaps expect that there is a cipher suite name => number mapping which could be used instead, but alas, that seem to be missing.)
https://codereview.webrtc.org/1774583002/diff/60001/webrtc/api/peerconnection... File webrtc/api/peerconnection_unittest.cc (right): https://codereview.webrtc.org/1774583002/diff/60001/webrtc/api/peerconnection... webrtc/api/peerconnection_unittest.cc:1451: EXPECT_TRUE_WAIT(rtc::SSLStreamAdapter::IsAcceptableCipher( Just curious - Instead of using the "WAIT" macros, is there a callback that could be used? The WAIT macros can repeatedly evaluate the expression and expand the expression into more than one call site, which is not obvious from just reading the code. https://codereview.webrtc.org/1774583002/diff/60001/webrtc/api/peerconnection... webrtc/api/peerconnection_unittest.cc:1455: #if 0 if this block should be deleted, let's just delete it instead of keeping it around as dead code.
The CQ bit was checked by torbjorng@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1774583002/120001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1774583002/120001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: ios64_sim_dbg on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/5781) ios_arm64_rel on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/8034)
The CQ bit was checked by torbjorng@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1774583002/140001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1774583002/140001
torbjorng@webrtc.org changed reviewers: + davidben@webrtc.org
davidben, please take a look at the code in opensslstreamadapter,cc. tommi, PTAL. https://codereview.webrtc.org/1774583002/diff/60001/webrtc/api/peerconnection... File webrtc/api/peerconnection_unittest.cc (right): https://codereview.webrtc.org/1774583002/diff/60001/webrtc/api/peerconnection... webrtc/api/peerconnection_unittest.cc:1451: EXPECT_TRUE_WAIT(rtc::SSLStreamAdapter::IsAcceptableCipher( On 2016/03/07 23:47:26, tommi-webrtc wrote: > Just curious - Instead of using the "WAIT" macros, is there a callback that > could be used? > The WAIT macros can repeatedly evaluate the expression and expand the expression > into more than one call site, which is not obvious from just reading the code. Perhaps. The present code is a bit odd, with timeout calls nested in each other. Since _WAIT also runs an event loop and since I just want to re-enable tests again, I will for now resist the temptation of improving this code. https://codereview.webrtc.org/1774583002/diff/60001/webrtc/api/peerconnection... webrtc/api/peerconnection_unittest.cc:1455: #if 0 On 2016/03/07 23:47:26, tommi-webrtc wrote: > if this block should be deleted, let's just delete it instead of keeping it > around as dead code. Done.
https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/opensslstrea... File webrtc/base/opensslstreamadapter.cc (right): https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/opensslstrea... webrtc/base/opensslstreamadapter.cc:1135: KeyType key_type) { What is the test actually trying to check? Are you trying to make sure the key type was honored or...? Previously it seems you were somehow interested in the protocol version, but this one isn't. I'm asking because there may be more direct ways to check what you want. You could assert on the actual protocol version, etc. https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/sslstreamada... File webrtc/base/sslstreamadapter_unittest.cc (left): https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/sslstreamada... webrtc/base/sslstreamadapter_unittest.cc:1121: rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam()).type()), Before, I think you were implicitly checking the resulting protocol version by way of the cipher. Is the protocol version check interesting for you? If so, it might be worth exposing that. If not, I'm a little puzzled why this test cared about the cipher you picked.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) linux_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL) win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
Thanks, davidben, for the feedback! https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/opensslstrea... File webrtc/base/opensslstreamadapter.cc (right): https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/opensslstrea... webrtc/base/opensslstreamadapter.cc:1135: KeyType key_type) { On 2016/03/08 20:29:05, davidben_webrtc wrote: > What is the test actually trying to check? Are you trying to make sure the key > type was honored or...? Previously it seems you were somehow interested in the > protocol version, but this one isn't. > > I'm asking because there may be more direct ways to check what you want. You > could assert on the actual protocol version, etc. Mainly I am trying to assert we got a reasonable cipher suite. Separating RSA and ECDSA sanity checks the logic of our code (since we separate them there). I think it makes sense to whitelist cipher suites like this, even at the price of making our test code somewhat sensitive to boringssl default changes. Now the only change we will need is to add a line or two in our whitelist. I'd be interested if there is a better way. You're right that the old code somewhat indirectly checked that the expected protocol version was used. We should check that again, with a definitive check (by means of SSL_version(const SSL *ssl), (which unfortunately is BoringSSL-specific and we still pretend to support plain OpenSSL). https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/sslstreamada... File webrtc/base/sslstreamadapter_unittest.cc (left): https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/sslstreamada... webrtc/base/sslstreamadapter_unittest.cc:1121: rtc::SSL_PROTOCOL_DTLS_10, ::testing::get<1>(GetParam()).type()), On 2016/03/08 20:29:05, davidben_webrtc wrote: > Before, I think you were implicitly checking the resulting protocol version by > way of the cipher. Is the protocol version check interesting for you? If so, it > might be worth exposing that. If not, I'm a little puzzled why this test cared > about the cipher you picked. I don't think the old test had as a main objective to assert the protocol version; it was necessary to pass it to the pesky GetDefaultSslCipherForTest for it to exactly fit boringssl's defaults.
The CQ bit was checked by torbjorng@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1774583002/180001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1774583002/180001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_baremetal on tryserver.webrtc (JOB_FAILED, http://build.chromium.org/p/tryserver.webrtc/builders/win_baremetal/builds/9590)
lgtm https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/opensslstrea... File webrtc/base/opensslstreamadapter.cc (right): https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/opensslstrea... webrtc/base/opensslstreamadapter.cc:1135: KeyType key_type) { On 2016/03/09 13:10:31, torbjorng (webrtc) wrote: > On 2016/03/08 20:29:05, davidben_webrtc wrote: > > What is the test actually trying to check? Are you trying to make sure the key > > type was honored or...? Previously it seems you were somehow interested in the > > protocol version, but this one isn't. > > > > I'm asking because there may be more direct ways to check what you want. You > > could assert on the actual protocol version, etc. > > Mainly I am trying to assert we got a reasonable cipher suite. Separating RSA > and ECDSA sanity checks the logic of our code (since we separate them there). > > I think it makes sense to whitelist cipher suites like this, even at the price > of making our test code somewhat sensitive to boringssl default changes. Now the > only change we will need is to add a line or two in our whitelist. I'd be > interested if there is a better way. I think it depends on what you're interested in checking. If you want to assert something about the cipher suite, you're probably stuck with this. I suppose you can query the certificate out of the SSL* and then check the key type there? I dunno. I can get you an API to categorize ciphers one way or another, but it won't work in OpenSSL. > You're right that the old code somewhat indirectly checked that the expected > protocol version was used. We should check that again, with a definitive check > (by means of SSL_version(const SSL *ssl), (which unfortunately is > BoringSSL-specific and we still pretend to support plain OpenSSL). SSL_version's been in OpenSSL since forever. :-)
Description was changed from ========== Add IsAcceptableCipher, use instead of GetDefaultCipher. BUG=webrtc:5634 ========== to ========== Add IsAcceptableCipher, use instead of GetDefaultCipher. The old code insists on exact cipher suite matches with hardwired expectations. It does this matching parameterized with key type (RSA vs ECDSA) and TLS version (DTLS vs TLS and version 1.0 vs 1.2). This CL changes things to check against a white-list of cipher suites, with the check parameterized with key type (again RSA vs ECDSA). Then separately checks TLS version since the old implicit check of TLS version by means of resulting cipher suite was too blunt. Using a white list for cipher suites isn't perfect, but it is safe and requires minimal maintenance. It allows compatibility with not just one exact version of underlying crypto lib, but any version with reasonable defaults. The CL also re-enables critical tests which had to be disabled recently to allow a boringssl roll. BUG=webrtc:5634 ==========
https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/opensslstrea... File webrtc/base/opensslstreamadapter.cc (right): https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/opensslstrea... webrtc/base/opensslstreamadapter.cc:1135: KeyType key_type) { > I think it depends on what you're interested in checking. If you want to assert > something about the cipher suite, you're probably stuck with this. I suppose you > can query the certificate out of the SSL* and then check the key type there? I > dunno. I can get you an API to categorize ciphers one way or another, but it > won't work in OpenSSL. Thanks, let's consider that for the future, but stick with my solution for now. (This CL was not planned, I need to move on.) It is doubtful whether WebRTC can be compiled against plain OpenSSL. We conditionalize for it, but none of the bots test that it still works, which probably means it doesn't. > SSL_version's been in OpenSSL since forever. :-) Right. I realised that when I implemented the checks yesterday.
lgtm with the below addressed. https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/opensslstrea... File webrtc/base/opensslstreamadapter.cc (right): https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/opensslstrea... webrtc/base/opensslstreamadapter.cc:1155: #undef CDEF ? https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/opensslstrea... File webrtc/base/opensslstreamadapter.h (right): https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/opensslstrea... webrtc/base/opensslstreamadapter.h:115: static bool IsAcceptableCipher(std::string cipher, KeyType key_type); const & https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/sslstreamada... File webrtc/base/sslstreamadapter.h (right): https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/sslstreamada... webrtc/base/sslstreamadapter.h:197: static bool IsAcceptableCipher(std::string cipher, KeyType key_type); const & https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/sslstreamada... File webrtc/base/sslstreamadapter_unittest.cc (right): https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/sslstreamada... webrtc/base/sslstreamadapter_unittest.cc:494: else nit: remove else this could also be written as: return client ? client_ssl_->GetSslVersion() : server_ssl_->GetSslVersion();
https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/opensslstrea... File webrtc/base/opensslstreamadapter.cc (right): https://codereview.webrtc.org/1774583002/diff/140001/webrtc/base/opensslstrea... webrtc/base/opensslstreamadapter.cc:1135: KeyType key_type) { On 2016/03/10 21:29:03, torbjorng (webrtc) wrote: > > I think it depends on what you're interested in checking. If you want to > assert > > something about the cipher suite, you're probably stuck with this. I suppose > you > > can query the certificate out of the SSL* and then check the key type there? I > > dunno. I can get you an API to categorize ciphers one way or another, but it > > won't work in OpenSSL. > > Thanks, let's consider that for the future, but stick with my solution for now. > (This CL was not planned, I need to move on.) > > It is doubtful whether WebRTC can be compiled against plain OpenSSL. We > conditionalize for it, but none of the bots test that it still works, which > probably means it doesn't. Really? I've been under the impression you all need to support it. If not, that would be wonderful! > > SSL_version's been in OpenSSL since forever. :-) > > Right. I realised that when I implemented the checks yesterday.
The CQ bit was checked by torbjorng@webrtc.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1774583002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1774583002/200001
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL) win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/opensslstrea... File webrtc/base/opensslstreamadapter.cc (right): https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/opensslstrea... webrtc/base/opensslstreamadapter.cc:1155: On 2016/03/10 21:37:59, tommi-webrtc wrote: > #undef CDEF ? Done. https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/opensslstrea... File webrtc/base/opensslstreamadapter.h (right): https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/opensslstrea... webrtc/base/opensslstreamadapter.h:115: static bool IsAcceptableCipher(std::string cipher, KeyType key_type); On 2016/03/10 21:37:59, tommi-webrtc wrote: > const & Done. https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/sslstreamada... File webrtc/base/sslstreamadapter.h (right): https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/sslstreamada... webrtc/base/sslstreamadapter.h:197: static bool IsAcceptableCipher(std::string cipher, KeyType key_type); On 2016/03/10 21:37:59, tommi-webrtc wrote: > const & Done. https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/sslstreamada... File webrtc/base/sslstreamadapter_unittest.cc (right): https://codereview.webrtc.org/1774583002/diff/180001/webrtc/base/sslstreamada... webrtc/base/sslstreamadapter_unittest.cc:494: else On 2016/03/10 21:37:59, tommi-webrtc wrote: > nit: remove else > > this could also be written as: > > return client ? > client_ssl_->GetSslVersion() : server_ssl_->GetSslVersion(); Let's clean that up in a separate CL since here I followed the style of many preceding functions.
The CQ bit was checked by torbjorng@webrtc.org
The patchset sent to the CQ was uploaded after l-g-t-m from tommi@webrtc.org, davidben@webrtc.org Link to the patchset: https://codereview.webrtc.org/1774583002/#ps200001 (title: "Address feedback")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1774583002/200001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1774583002/200001
Message was sent while issue was closed.
Description was changed from ========== Add IsAcceptableCipher, use instead of GetDefaultCipher. The old code insists on exact cipher suite matches with hardwired expectations. It does this matching parameterized with key type (RSA vs ECDSA) and TLS version (DTLS vs TLS and version 1.0 vs 1.2). This CL changes things to check against a white-list of cipher suites, with the check parameterized with key type (again RSA vs ECDSA). Then separately checks TLS version since the old implicit check of TLS version by means of resulting cipher suite was too blunt. Using a white list for cipher suites isn't perfect, but it is safe and requires minimal maintenance. It allows compatibility with not just one exact version of underlying crypto lib, but any version with reasonable defaults. The CL also re-enables critical tests which had to be disabled recently to allow a boringssl roll. BUG=webrtc:5634 ========== to ========== Add IsAcceptableCipher, use instead of GetDefaultCipher. The old code insists on exact cipher suite matches with hardwired expectations. It does this matching parameterized with key type (RSA vs ECDSA) and TLS version (DTLS vs TLS and version 1.0 vs 1.2). This CL changes things to check against a white-list of cipher suites, with the check parameterized with key type (again RSA vs ECDSA). Then separately checks TLS version since the old implicit check of TLS version by means of resulting cipher suite was too blunt. Using a white list for cipher suites isn't perfect, but it is safe and requires minimal maintenance. It allows compatibility with not just one exact version of underlying crypto lib, but any version with reasonable defaults. The CL also re-enables critical tests which had to be disabled recently to allow a boringssl roll. BUG=webrtc:5634 ==========
Message was sent while issue was closed.
Committed patchset #11 (id:200001)
Message was sent while issue was closed.
Description was changed from ========== Add IsAcceptableCipher, use instead of GetDefaultCipher. The old code insists on exact cipher suite matches with hardwired expectations. It does this matching parameterized with key type (RSA vs ECDSA) and TLS version (DTLS vs TLS and version 1.0 vs 1.2). This CL changes things to check against a white-list of cipher suites, with the check parameterized with key type (again RSA vs ECDSA). Then separately checks TLS version since the old implicit check of TLS version by means of resulting cipher suite was too blunt. Using a white list for cipher suites isn't perfect, but it is safe and requires minimal maintenance. It allows compatibility with not just one exact version of underlying crypto lib, but any version with reasonable defaults. The CL also re-enables critical tests which had to be disabled recently to allow a boringssl roll. BUG=webrtc:5634 ========== to ========== Add IsAcceptableCipher, use instead of GetDefaultCipher. The old code insists on exact cipher suite matches with hardwired expectations. It does this matching parameterized with key type (RSA vs ECDSA) and TLS version (DTLS vs TLS and version 1.0 vs 1.2). This CL changes things to check against a white-list of cipher suites, with the check parameterized with key type (again RSA vs ECDSA). Then separately checks TLS version since the old implicit check of TLS version by means of resulting cipher suite was too blunt. Using a white list for cipher suites isn't perfect, but it is safe and requires minimal maintenance. It allows compatibility with not just one exact version of underlying crypto lib, but any version with reasonable defaults. The CL also re-enables critical tests which had to be disabled recently to allow a boringssl roll. BUG=webrtc:5634 Committed: https://crrev.com/43166b8adf749c6672eca0cd4a39399cf27d4761 Cr-Commit-Position: refs/heads/master@{#11951} ==========
Message was sent while issue was closed.
Patchset 11 (id:??) landed as https://crrev.com/43166b8adf749c6672eca0cd4a39399cf27d4761 Cr-Commit-Position: refs/heads/master@{#11951} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
