Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
275943004:
Add support for RSA-OAEP when using NSS 3.16.2 or later (Closed)
|
Created:
6 years, 7 months ago by Ryan Sleevi Modified:
6 years, 7 months ago Reviewers:
eroman CC:
chromium-reviews, darin-cc_chromium.org, jam Base URL:
svn://svn.chromium.org/chrome/trunk/src Visibility:
Public. |
DescriptionAdd support for RSA-OAEP when using NSS 3.16.2 or later.
This also rolls the NSS deps to include the RSA-OAEP support backported for Windows/Mac
BUG=372917
R=eroman@chromium.org
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=272221
Patch Set 1 #
Total comments: 13
Patch Set 2 : Unit tests #
Total comments: 39
Patch Set 3 : More tests #Patch Set 4 : Update to match NSS changes #
Total comments: 1
Patch Set 5 : Rebase ALL the things #Patch Set 6 : Review feedback #Patch Set 7 : Roll DEPS #Patch Set 8 : Rebased #Patch Set 9 : Fix ifdef #
Messages
Total messages: 24 (0 generated)
Eric: Please take a look. I'll work on adding tests if you're ok with the general approach. https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... File content/child/webcrypto/platform_crypto_nss.cc (right): https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... content/child/webcrypto/platform_crypto_nss.cc:104: } Note: For both the AES-GCM case and the RSA-OAEP, the fact that NSS's pk11wrap layer supports the functions is not an intrinsic guarantee that it will be linked against a softoken that supports the CK_MECHANISM_TYPEs. The distinction is that it will likely be treated as an OperationError, rather than a ErrorUnsupported(). This only really matters for distros that do horrible things (read: FIPS), so I'm not too worried - just FYI. https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/shar... File content/child/webcrypto/shared_crypto.cc (right): https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/shar... content/child/webcrypto/shared_crypto.cc:811: // supporting C_Wrap/C_Unwrap with AES-KW This does feel kinda hacky. It could be wrapped into some helper function "DoesSupportRawWrapAndUnwrap" or something, but that seems overkill.
approach lgtm. looking forward to the tests https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... File content/child/webcrypto/platform_crypto_nss.cc (right): https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... content/child/webcrypto/platform_crypto_nss.cc:350: oaep_params->mgf = WebCryptoHashToMGFMechanism(hash); What is the consequence of failure for WebCryptoHashToMGFMechanism()? Does passing CK_INVALID_MECHANISM do anything weird, should it be checked for and return failure? https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... content/child/webcrypto/platform_crypto_nss.cc:351: oaep_params->hashAlg = WebCryptoHashToDigestMechanism(hash); Same question above, for the sake of future proofing. https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... content/child/webcrypto/platform_crypto_nss.cc:1272: Status EncryptRsaOaep(PublicKey* key, nit: for consistency add a new line after ------------ https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... content/child/webcrypto/platform_crypto_nss.cc:1313: if (!g_nss_runtime_support.Get().IsRsaOaepSupported()) We might consider failing with ErrorUnsupported during key creation as well. https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/shar... File content/child/webcrypto/shared_crypto.cc (right): https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/shar... content/child/webcrypto/shared_crypto.cc:856: return UnwrapKeyDecryptAndImport(format, I thought style was to not use an "else" after a return?
https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... File content/child/webcrypto/platform_crypto_nss.cc (right): https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... content/child/webcrypto/platform_crypto_nss.cc:1282: Do we need to check the length of the input |data|? RSA-OAEP has a limit on the length data is can encrypt, involving the modulus length and hash block size. Or are you depending on NSS to do that check?
https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/shar... File content/child/webcrypto/shared_crypto.cc (right): https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/shar... content/child/webcrypto/shared_crypto.cc:845: if (wrapping_algorithm.id() == blink::WebCryptoAlgorithmIdAesKw) { I don't understand the special-casing here. UnwrapKeyRaw() currently also handles RSA-ES via PK11_PubUnwrapSymKeyWithFlagsPerm(). Are you saying that PK11_PubUnwrapSymKeyWithFlagsPerm() cannot do RSA-OAEP? If not, we should rename UnwrapKeyRaw() to something specifically mentioning AES-KW since it will only be used for that alg.
https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... File content/child/webcrypto/platform_crypto_nss.cc (right): https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... content/child/webcrypto/platform_crypto_nss.cc:1313: if (!g_nss_runtime_support.Get().IsRsaOaepSupported()) On 2014/05/14 01:50:47, eroman wrote: > We might consider failing with ErrorUnsupported during key creation as well. Good point. https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/shar... File content/child/webcrypto/shared_crypto.cc (right): https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/shar... content/child/webcrypto/shared_crypto.cc:845: if (wrapping_algorithm.id() == blink::WebCryptoAlgorithmIdAesKw) { On 2014/05/14 02:14:41, padolph wrote: > I don't understand the special-casing here. UnwrapKeyRaw() currently also > handles RSA-ES via PK11_PubUnwrapSymKeyWithFlagsPerm(). Are you saying that > PK11_PubUnwrapSymKeyWithFlagsPerm() cannot do RSA-OAEP? If not, we should rename > UnwrapKeyRaw() to something specifically mentioning AES-KW since it will only be > used for that alg. Correct. Much of the pk11wrap layer is not situated to handle mechanisms with arguments well - as demonstrated by the patches we needed for AES-GCM and now here. In particular, PubWrap/PubUnwrap assume all they need to know is the mechanism type of the key to determine the wrapping algorithm, which is incorrect, since they also need to be able to supply key parameters (label, mgf, hash function) Fixing pk11wrap is a much larger effort - as it also does its own layer of SSA encoding when handling RSA keys. I'm fine renaming this. Note: the reason AES-KW is a special snowflake is the PKCS mechanism is documented to only work with C_Wrap/C_Unwrap, whereas OAEP works with C_Encrypt/C_Decrypt as well. https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/shar... content/child/webcrypto/shared_crypto.cc:856: return UnwrapKeyDecryptAndImport(format, On 2014/05/14 01:50:47, eroman wrote: > I thought style was to not use an "else" after a return? D'oh! Well spotted.
https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... File content/child/webcrypto/platform_crypto_nss.cc (right): https://codereview.chromium.org/275943004/diff/1/content/child/webcrypto/plat... content/child/webcrypto/platform_crypto_nss.cc:350: oaep_params->mgf = WebCryptoHashToMGFMechanism(hash); On 2014/05/14 01:50:47, eroman wrote: > What is the consequence of failure for WebCryptoHashToMGFMechanism()? Does > passing CK_INVALID_MECHANISM do anything weird, should it be checked for and > return failure? Officially: "It depends". Practically: No PKCS#11 is underspecified in this regard, but given that it's meant to be extensible (in terms of CKG_MGF1 functions and CKM_* digest functions). A "well-behaved" token is meant to handle these gracefully. Softoken is a "well-behaved" token.
BTW here is the missing RSA-OAEP piece from Blink side: https://codereview.chromium.org/289073005/ If you patch locally should allow you to test from javascript.
Eric: More unit tests - which of course found bugs. Just a few edge cases remain.
https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... File content/child/webcrypto/jwk.cc (left): https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/jwk.cc:570: jwk_dict->SetString("alg", "RSA-OAEP"); LULWUT https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... File content/child/webcrypto/shared_crypto.cc (right): https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto.cc:813: // supporting C_Wrap/C_Unwrap with AES-KW Once the RsaEs code removal lands, I'll be happy here. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... File content/child/webcrypto/shared_crypto_unittest.cc (right): https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:476: ASSERT_EQ(Status::Success(), Ran into CHECK failures here when testing and I didn't have the OAEP stuff wired up in NSS. Specifically, line 484, but I figure it's better to do the check here. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:1569: // export format is the same as kPublicKeySpkiDerHex These should probably be moved into the JSON proper at a later time. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:2048: // as OAEP/PSS Just documenting various SPKI failures. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:2303: ASSERT_NO_FATAL_FAILURE(ImportRsaKeyPair( *slightly* unrelated, but the OAEP correction (to ASSERT_) made me decide to wrap all of these as well, since everything else crashes if this import fails. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:3566: TEST_F(SharedCryptoRsaOaepTest, ImportPkcs8WithRsaEncryption) { More tests needed: Variations of SPKI param handling. I'm having to look into NSS first though.
https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... File content/child/webcrypto/jwk.cc (right): https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/jwk.cc:114: // | "RSA-OAEP" | RSAES using Optimal Asymmetric Encryption Padding | Update this comment. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... File content/child/webcrypto/platform_crypto_nss.cc (right): https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/platform_crypto_nss.cc:160: !!PK11_DoesMechanism(slot.get(), CKM_RSA_PKCS_OAEP); Is the double negative intentional? Does PRBool not convert to bool implicitly? https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/platform_crypto_nss.cc:1091: // TODO(rsleevi): Implement OAEP support according to the spec. This comment is ambiguous, please be more specific. Are you referring to http://crbug.com/373545 ? If so, link to it. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/platform_crypto_nss.cc:1490: return Status::ErrorUnsupported(); Do the same thing for import/unwrap key? https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... File content/child/webcrypto/shared_crypto.cc (right): https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto.cc:813: // supporting C_Wrap/C_Unwrap with AES-KW On 2014/05/16 05:17:22, Ryan Sleevi wrote: > Once the RsaEs code removal lands, I'll be happy here. Just waiting on blink side reviews for that (need to be landed in particular sequence). https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... File content/child/webcrypto/shared_crypto_unittest.cc (right): https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:120: if (!NSS_VersionCheck("3.16.2")) I prefer to not have this sort of knowledge in unit-tests: - Imagine how a comparable LayoutTest would look -- it cannot do these sort of internal checks. - It is good to exercise the actual code users will run when it is unavailable. I think that is a worthwhile test. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:123: return !!PK11_DoesMechanism(slot.get(), CKM_RSA_PKCS_OAEP); Same question as earlier, why the !! rather than implicit cast, or a static_cast<bool>(). Feels like I am reading Javascript. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:157: // Creates an RSA-OAEP algorithm Might want to mention "for encryption/decryption/wrap/unwrap". https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:435: "A56E4A0E701017589A5187DC7EA841D156F2EC0E36AD52A44DFEB1E61F7AD991D8C51056" [optional] OCD nit: The hex string above uses lower case whereas this uses upper case. Do you hate me? https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:439: const char* const kPublicKeyExponentHex = "010001"; Consider adding a TODO to replace the existing usages with this constant. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:1478: LOG(WARNING) << "RSA-OAEP not supported on this platform. Skipping some" sometests --> some tests https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:1534: continue; Consider adding the skip warning here. The outter log has higher chance of becoming obsolete during some refactor. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:1569: // export format is the same as kPublicKeySpkiDerHex On 2014/05/16 05:17:22, Ryan Sleevi wrote: > These should probably be moved into the JSON proper at a later time. What is the difference in format, just the OID? Feels like there is a TODO missing here. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:2048: // as OAEP/PSS On 2014/05/16 05:17:22, Ryan Sleevi wrote: > Just documenting various SPKI failures. why TODO(eroman) and not TODO(rsleevi)? FYI TODO(name) roughly means "todo written by name" not "name will do": http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=TODO_C... https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:3551: SharedCryptoRsaOaepTest() { Init(); } Would inheriting from SharedCryptoTest instead work? https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:3553: scoped_ptr<base::DictionaryValue> CreatePublicKeyJwkDict() { why introduce a new test fixture, can't this be a function? https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:3566: TEST_F(SharedCryptoRsaOaepTest, ImportPkcs8WithRsaEncryption) { These tests are going to fail when run on Android no? Wrap the function name in MAYBE(). https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:3600: &public_key)); Assert that the resulting key.algorithm() matches? https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:3664: TEST_F(SharedCryptoRsaOaepTest, ExportJwk) { Include "Public" somewhere in the name. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:3776: FAIL(); Wut? Either fill these out, make them into TODOs, or TODO+bug. https://codereview.chromium.org/275943004/diff/20001/content/test/data/webcry... File content/test/data/webcrypto/rsa_oaep.json (right): https://codereview.chromium.org/275943004/diff/20001/content/test/data/webcry... content/test/data/webcrypto/rsa_oaep.json:2: // Tests for RSA-OAEP Encrypt/Decrypt without a label (the empty string). Can you say something about the data in the comment? * public_key hex of spki * private_key hex of pkcs8
https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... File content/child/webcrypto/shared_crypto_unittest.cc (right): https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:120: if (!NSS_VersionCheck("3.16.2")) On 2014/05/16 20:29:51, eroman wrote: > I prefer to not have this sort of knowledge in unit-tests: > > - Imagine how a comparable LayoutTest would look -- it cannot do these sort of > internal checks. > > - It is good to exercise the actual code users will run when it is > unavailable. I think that is a worthwhile test. I strongly disagree with this. 1) Performance - these tests should be highly performant. 2) Coupling - Your choice of ImportKey unduly couples the thing under test to specifics of your implementation 3) Correctness - This added check is to ensure that - independent of what the IUT thinks, *this test* believes OAEP should be supported. That is, it serves as a check in and of itself whether or not the IUT is properly checking if RSA-OAEP is supported. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:123: return !!PK11_DoesMechanism(slot.get(), CKM_RSA_PKCS_OAEP); On 2014/05/16 20:29:51, eroman wrote: > Same question as earlier, why the !! rather than implicit cast, or a > static_cast<bool>(). Feels like I am reading Javascript. Because PR_Boolean is an int, and bool is a narrower type. This is a standard pattern for Windows API code, which does the same thing (with FALSE and TRUE), as an artifact of the lack of a primitive bool type. Rather than coercing an int to a bool (which itself is confusing to reason about - since now you're wondering about what bits are set, what to do with -1, etc), the !! ensures that any non-zero value is properly and sanely converted. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:435: "A56E4A0E701017589A5187DC7EA841D156F2EC0E36AD52A44DFEB1E61F7AD991D8C51056" On 2014/05/16 20:29:51, eroman wrote: > [optional] OCD nit: The hex string above uses lower case whereas this uses upper > case. Do you hate me? This was your string - from 1445. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:439: const char* const kPublicKeyExponentHex = "010001"; On 2014/05/16 20:29:51, eroman wrote: > Consider adding a TODO to replace the existing usages with this constant. I already fixed the existing usages. I'm not sure what you're asking here. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:1534: continue; On 2014/05/16 20:29:51, eroman wrote: > Consider adding the skip warning here. The outter log has higher chance of > becoming obsolete during some refactor. I generally oppose logging skipped tests on principle, because they create significant log spew on any bot that doesn't implement them, and they provide no visibility in terms of tree closing/test failures that something is wrong. The better way to handle such tests is to use GTest generators, so that the number of parameterized tests can change based on platform, providing a clearer signal about tests that changed. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:1569: // export format is the same as kPublicKeySpkiDerHex On 2014/05/16 20:29:51, eroman wrote: > On 2014/05/16 05:17:22, Ryan Sleevi wrote: > > These should probably be moved into the JSON proper at a later time. > > What is the difference in format, just the OID? Feels like there is a TODO > missing here. You already filed the bug - https://code.google.com/p/chromium/issues/detail?id=373545 https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:2048: // as OAEP/PSS On 2014/05/16 20:29:51, eroman wrote: > On 2014/05/16 05:17:22, Ryan Sleevi wrote: > > Just documenting various SPKI failures. > > why TODO(eroman) and not TODO(rsleevi)? > > FYI TODO(name) roughly means "todo written by name" not "name will do": > > http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml?showone=TODO_C... Because you filed the bug on Import/Export SPKI and PKCS8. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:3553: scoped_ptr<base::DictionaryValue> CreatePublicKeyJwkDict() { On 2014/05/16 20:29:51, eroman wrote: > why introduce a new test fixture, can't this be a function? It can. A new fixture was created, because in my ideal testing world, we would have files per algorithm being tested - not a 10,000 line test file. Creating a free function also creates the risk (as evidenced by my own 'misuse') that it will be used by other tests. I intended this function to only be used by these tests, because their tests explicitly depend on these values being what they are. Creating a free function creates more opportunity for unnecessary coupling. https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:3566: TEST_F(SharedCryptoRsaOaepTest, ImportPkcs8WithRsaEncryption) { On 2014/05/16 20:29:51, eroman wrote: > These tests are going to fail when run on Android no? Wrap the function name in > MAYBE(). No. Because SupportsRsaOaep().
lgtm https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... File content/child/webcrypto/shared_crypto_unittest.cc (right): https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:439: const char* const kPublicKeyExponentHex = "010001"; On 2014/05/16 22:43:05, Ryan Sleevi wrote: > On 2014/05/16 20:29:51, eroman wrote: > > Consider adding a TODO to replace the existing usages with this constant. > > I already fixed the existing usages. I'm not sure what you're asking here. There are other occurrences of "010001" in the file, which are from tests using kPrivateKeyPkcs8DerHex / kPublicKeySpkiDerHex. I believe they could be switched over to the constant as well (although not as part of this CL). https://codereview.chromium.org/275943004/diff/20001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:3553: scoped_ptr<base::DictionaryValue> CreatePublicKeyJwkDict() { On 2014/05/16 22:43:05, Ryan Sleevi wrote: > On 2014/05/16 20:29:51, eroman wrote: > > why introduce a new test fixture, can't this be a function? > > It can. A new fixture was created, because in my ideal testing world, we would > have files per algorithm being tested - not a 10,000 line test file. > > Creating a free function also creates the risk (as evidenced by my own 'misuse') > that it will be used by other tests. I intended this function to only be used by > these tests, because their tests explicitly depend on these values being what > they are. > > Creating a free function creates more opportunity for unnecessary coupling. Ok https://codereview.chromium.org/275943004/diff/60001/content/child/webcrypto/... File content/child/webcrypto/shared_crypto_unittest.cc (right): https://codereview.chromium.org/275943004/diff/60001/content/child/webcrypto/... content/child/webcrypto/shared_crypto_unittest.cc:3664: TEST_F(SharedCryptoRsaOaepTest, ExportJwk) { Same comment as before; can you indicate in the name that this is for Public key export, not Private key export?
The CQ bit was checked by rsleevi@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rsleevi@chromium.org/275943004/120002
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are real, and report flakes to chrome-troopers@google.com. The failing builders are: mac_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_compile_...) mac_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/mac_chromium_rel/buil...) win_chromium_compile_dbg on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_compile_...) win_chromium_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_rel/buil...) win_chromium_x64_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/win_chromium_x64_rel/...)
The CQ bit was unchecked by rsleevi@chromium.org
The CQ bit was checked by rsleevi@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rsleevi@chromium.org/275943004/150001
FYI, CQ is re-trying this CL (attempt #1). Please consider checking whether the failures are real, and report flakes to chrome-troopers@google.com. The failing builders are: linux_chromium_chromeos_rel on tryserver.chromium (http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...)
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_gpu_triggered_tests on tryserver.chromium.gpu (http://build.chromium.org/p/tryserver.chromium.gpu/builders/win_gpu_triggered...)
The CQ bit was checked by rsleevi@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/rsleevi@chromium.org/275943004/150001
Message was sent while issue was closed.
Change committed as 272221 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
