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

Issue 334983006: [webcrypto] Disable RSA key import for NSS versions less than 3.16.2. (Closed)

Created:
6 years, 6 months ago by eroman
Modified:
6 years, 6 months ago
Reviewers:
Ryan Sleevi
CC:
chromium-reviews, darin-cc_chromium.org, jam
Project:
chromium
Visibility:
Public.

Description

[webcrypto] Disable RSA key import for NSS versions less than 3.16.2. Prior to NSS 3.16.2 there wasn't any validation of the RSA key parameters. This has several consequences: * Importing an RSA private key with another key's public modulus can be used to gain access to that key. * importKey() can succeed for invalid RSA keys (invalid n, e, d, p, q etc). This only affects Linux, since other platforms of Chromium bundle NSS/OpenSSL. BUG=380424, 378315 R=rsleevi@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=278803

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use rsleevi's code to test softoken version #

Patch Set 3 : use != SECSuccess rather than == SECFailure #

Patch Set 4 : Exclude checks from being run when using embedded NSS #

Patch Set 5 : Exclude ChromeOS from ifdef #

Patch Set 6 : Don't run rsaoaep or rsa key import tests on ChromeOS (since NSS hasn't rolled yet) #

Patch Set 7 : rebase onto master #

Unified diffs Side-by-side diffs Delta from patch set Stats (+131 lines, -34 lines) Patch
M content/child/webcrypto/platform_crypto_nss.cc View 1 2 3 4 5 6 7 chunks +59 lines, -5 lines 0 comments Download
M content/child/webcrypto/shared_crypto_unittest.cc View 1 2 3 4 5 6 12 chunks +72 lines, -29 lines 0 comments Download

Messages

Total messages: 19 (0 generated)
eroman
CL 1 of 1 marks the affected LayoutTests as flaky under (Linux)
6 years, 6 months ago (2014-06-17 19:18:19 UTC) #1
eroman
Dependent blink change: https://codereview.chromium.org/333273008
6 years, 6 months ago (2014-06-17 19:27:35 UTC) #2
Ryan Sleevi
https://codereview.chromium.org/334983006/diff/1/content/child/webcrypto/platform_crypto_nss.cc File content/child/webcrypto/platform_crypto_nss.cc (right): https://codereview.chromium.org/334983006/diff/1/content/child/webcrypto/platform_crypto_nss.cc#newcode281 content/child/webcrypto/platform_crypto_nss.cc:281: if (IsAlgorithmRsa(algorithm) && !NSS_VersionCheck("3.16.2")) { Sadly, this isn't sufficient ...
6 years, 6 months ago (2014-06-17 19:40:07 UTC) #3
eroman
https://codereview.chromium.org/334983006/diff/1/content/child/webcrypto/platform_crypto_nss.cc File content/child/webcrypto/platform_crypto_nss.cc (right): https://codereview.chromium.org/334983006/diff/1/content/child/webcrypto/platform_crypto_nss.cc#newcode281 content/child/webcrypto/platform_crypto_nss.cc:281: if (IsAlgorithmRsa(algorithm) && !NSS_VersionCheck("3.16.2")) { On 2014/06/17 19:40:07, Ryan ...
6 years, 6 months ago (2014-06-17 20:00:16 UTC) #4
Ryan Sleevi
LGTM if you tested it ;)
6 years, 6 months ago (2014-06-17 20:02:58 UTC) #5
eroman
The CQ bit was checked by eroman@chromium.org
6 years, 6 months ago (2014-06-17 22:43:45 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eroman@chromium.org/334983006/40001
6 years, 6 months ago (2014-06-17 22:45:29 UTC) #7
eroman
I made further changes. Please review differences between patchset 2 and 3. (To fix undefined ...
6 years, 6 months ago (2014-06-18 01:40:38 UTC) #8
Ryan Sleevi
LGTM ;)
6 years, 6 months ago (2014-06-18 01:53:09 UTC) #9
eroman
The CQ bit was checked by eroman@chromium.org
6 years, 6 months ago (2014-06-18 01:57:42 UTC) #10
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eroman@chromium.org/334983006/60001
6 years, 6 months ago (2014-06-18 01:58:47 UTC) #11
eroman
The CQ bit was unchecked by eroman@chromium.org
6 years, 6 months ago (2014-06-18 06:07:48 UTC) #12
eroman
I made some small changes in patchset 5 to exclude ChromeOS from both the RSA ...
6 years, 6 months ago (2014-06-18 19:27:59 UTC) #13
eroman
I have turned off the ChromeOS unittests that required NSS 3.16.2 so that this can ...
6 years, 6 months ago (2014-06-20 18:13:54 UTC) #14
eroman
The CQ bit was checked by eroman@chromium.org
6 years, 6 months ago (2014-06-20 18:14:02 UTC) #15
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eroman@chromium.org/334983006/100001
6 years, 6 months ago (2014-06-20 18:17:39 UTC) #16
eroman
The CQ bit was checked by eroman@chromium.org
6 years, 6 months ago (2014-06-20 18:27:50 UTC) #17
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/eroman@chromium.org/334983006/120001
6 years, 6 months ago (2014-06-20 18:30:51 UTC) #18
eroman
6 years, 6 months ago (2014-06-20 19:51:14 UTC) #19
Message was sent while issue was closed.
Committed patchset #7 manually as r278803 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698