Index: content/child/webcrypto/shared_crypto.cc |
diff --git a/content/child/webcrypto/shared_crypto.cc b/content/child/webcrypto/shared_crypto.cc |
index 7d4704fc656ec7aca87fdefa3ffe22201bd70608..539617d323b563f02084adf6816cb49c4e213a35 100644 |
--- a/content/child/webcrypto/shared_crypto.cc |
+++ b/content/child/webcrypto/shared_crypto.cc |
@@ -474,12 +474,13 @@ Status UnwrapKeyDecryptAndImport( |
wrapping_algorithm, wrapping_key, wrapped_key_data, &buffer); |
if (status.IsError()) |
return status; |
- status = ImportKey( |
+ // NOTE that returning the details of ImportKey() failures may leak |
+ // information about the plaintext of the encrypted key (for instance the JWK |
+ // key_ops). As long as the ImportKey error messages don't describe actual |
+ // key bytes however this should be OK. For more discussion see |
+ // http://crubg.com/372040 |
Ryan Sleevi
2014/05/15 06:29:40
crbug.com
|
+ return ImportKey( |
format, CryptoData(buffer), algorithm, extractable, usage_mask, key); |
- // NOTE! Returning the details of any ImportKey() failure here would leak |
- // information about the plaintext internals of the encrypted key. Instead, |
- // collapse any error into the generic Status::OperationError(). |
- return status.IsError() ? Status::OperationError() : Status::Success(); |
} |
Status WrapKeyExportAndEncrypt( |
@@ -511,6 +512,150 @@ unsigned int ShaBlockSizeBytes(blink::WebCryptoAlgorithmId hash_id) { |
} |
} |
+Status ImportKeyDontCheckKeyUsages(blink::WebCryptoKeyFormat format, |
+ const CryptoData& key_data, |
+ const blink::WebCryptoAlgorithm& algorithm, |
+ bool extractable, |
+ blink::WebCryptoKeyUsageMask usage_mask, |
+ blink::WebCryptoKey* key) { |
+ switch (format) { |
+ case blink::WebCryptoKeyFormatRaw: |
+ return ImportKeyRaw(key_data, algorithm, extractable, usage_mask, key); |
+ case blink::WebCryptoKeyFormatSpki: |
+ return platform::ImportKeySpki( |
+ algorithm, key_data, extractable, usage_mask, key); |
+ case blink::WebCryptoKeyFormatPkcs8: |
+ return platform::ImportKeyPkcs8( |
+ algorithm, key_data, extractable, usage_mask, key); |
+ case blink::WebCryptoKeyFormatJwk: |
+ return ImportKeyJwk(key_data, algorithm, extractable, usage_mask, key); |
+ default: |
+ return Status::ErrorUnsupported(); |
+ } |
+} |
+ |
+Status UnwrapKeyDontCheckKeyUsages( |
+ blink::WebCryptoKeyFormat format, |
+ const CryptoData& wrapped_key_data, |
+ const blink::WebCryptoKey& wrapping_key, |
+ const blink::WebCryptoAlgorithm& wrapping_algorithm, |
+ const blink::WebCryptoAlgorithm& algorithm, |
+ bool extractable, |
+ blink::WebCryptoKeyUsageMask usage_mask, |
+ blink::WebCryptoKey* key) { |
+ if (!KeyUsageAllows(wrapping_key, blink::WebCryptoKeyUsageUnwrapKey)) |
+ return Status::ErrorUnexpected(); |
+ if (wrapping_algorithm.id() != wrapping_key.algorithm().id()) |
+ return Status::ErrorUnexpected(); |
+ |
+ switch (format) { |
+ case blink::WebCryptoKeyFormatRaw: |
+ return UnwrapKeyRaw(wrapped_key_data, |
+ wrapping_key, |
+ wrapping_algorithm, |
+ algorithm, |
+ extractable, |
+ usage_mask, |
+ key); |
+ case blink::WebCryptoKeyFormatJwk: |
+ return UnwrapKeyDecryptAndImport(format, |
+ wrapped_key_data, |
+ wrapping_key, |
+ wrapping_algorithm, |
+ algorithm, |
+ extractable, |
+ usage_mask, |
+ key); |
+ case blink::WebCryptoKeyFormatSpki: |
+ case blink::WebCryptoKeyFormatPkcs8: |
+ return Status::ErrorUnsupported(); // TODO(padolph) |
+ default: |
+ NOTREACHED(); |
+ return Status::ErrorUnsupported(); |
+ } |
+} |
+ |
+// Returns the mask of all key usages that are possible for |algorithm| and |
+// |key_type|. |
+blink::WebCryptoKeyUsageMask GetValidKeyUsagesForKeyType( |
+ blink::WebCryptoAlgorithmId algorithm, |
+ blink::WebCryptoKeyType key_type) { |
+ switch (algorithm) { |
+ case blink::WebCryptoAlgorithmIdAesCbc: |
+ case blink::WebCryptoAlgorithmIdAesGcm: |
+ case blink::WebCryptoAlgorithmIdAesCtr: |
+ return blink::WebCryptoKeyUsageEncrypt | blink::WebCryptoKeyUsageDecrypt | |
+ blink::WebCryptoKeyUsageWrapKey | |
+ blink::WebCryptoKeyUsageUnwrapKey; |
+ case blink::WebCryptoAlgorithmIdAesKw: |
+ return blink::WebCryptoKeyUsageWrapKey | |
+ blink::WebCryptoKeyUsageUnwrapKey; |
+ case blink::WebCryptoAlgorithmIdHmac: |
+ return blink::WebCryptoKeyUsageSign | blink::WebCryptoKeyUsageVerify; |
+ case blink::WebCryptoAlgorithmIdRsaSsaPkcs1v1_5: |
+ switch (key_type) { |
+ case blink::WebCryptoKeyTypePublic: |
+ return blink::WebCryptoKeyUsageVerify; |
+ case blink::WebCryptoKeyTypePrivate: |
+ return blink::WebCryptoKeyUsageSign; |
+ default: |
+ return 0; |
+ } |
+ case blink::WebCryptoAlgorithmIdRsaEsPkcs1v1_5: |
+ case blink::WebCryptoAlgorithmIdRsaOaep: |
+ switch (key_type) { |
+ case blink::WebCryptoKeyTypePublic: |
Ryan Sleevi
2014/05/15 06:29:40
Structurally, I'm wondering whether it makes more
eroman
2014/05/16 00:04:37
I can paint the bikeshed either colour, don't part
Ryan Sleevi
2014/05/19 23:06:29
Part of the comment was motivated by things like l
|
+ return blink::WebCryptoKeyUsageEncrypt | |
+ blink::WebCryptoKeyUsageWrapKey; |
+ case blink::WebCryptoKeyTypePrivate: |
+ return blink::WebCryptoKeyUsageDecrypt | |
+ blink::WebCryptoKeyUsageUnwrapKey; |
+ default: |
+ return 0; |
+ } |
+ default: |
+ return 0; |
+ } |
+} |
+ |
+// Returns Status::Success() if |usages| is a valid set of key usages for |
+// |algorithm| and |key_type|. Otherwise returns an error. |
+Status CheckKeyUsages(blink::WebCryptoAlgorithmId algorithm, |
+ blink::WebCryptoKeyType key_type, |
+ blink::WebCryptoKeyUsageMask usages) { |
+ blink::WebCryptoKeyUsageMask all_usages = |
+ GetValidKeyUsagesForKeyType(algorithm, key_type); |
+ |
+ if (!ContainsKeyUsages(all_usages, usages)) |
+ return Status::ErrorCreateKeyBadUsages(); |
+ |
+ return Status::Success(); |
+} |
+ |
+// Returns an error if |combined_usage_mask| is invalid for generating a key |
+// pair for |algorithm|. Otherwise returns Status::Success(), and fills |
+// |public_key_usages| with the usages for the public key, and |
+// |private_key_usages| with those for the private key. |
+Status CheckKeyUsagesForGenerateKeyPair( |
+ blink::WebCryptoAlgorithmId algorithm, |
+ blink::WebCryptoKeyUsageMask combined_usage_mask, |
+ blink::WebCryptoKeyUsageMask* public_key_usages, |
+ blink::WebCryptoKeyUsageMask* private_key_usages) { |
+ blink::WebCryptoKeyUsageMask all_public_key_usages = |
+ GetValidKeyUsagesForKeyType(algorithm, blink::WebCryptoKeyTypePublic); |
+ blink::WebCryptoKeyUsageMask all_private_key_usages = |
+ GetValidKeyUsagesForKeyType(algorithm, blink::WebCryptoKeyTypePrivate); |
+ |
+ if (!ContainsKeyUsages(all_public_key_usages | all_private_key_usages, |
+ combined_usage_mask)) |
+ return Status::ErrorCreateKeyBadUsages(); |
+ |
+ *public_key_usages = combined_usage_mask & all_public_key_usages; |
+ *private_key_usages = combined_usage_mask & all_private_key_usages; |
Ryan Sleevi
2014/05/15 06:29:40
Seems like it should be invalid to generate a keyp
eroman
2014/05/16 00:04:37
My interpretation of the spec is that keys with no
Ryan Sleevi
2014/05/19 23:06:29
Fair point, though I wonder if we shouldn't correc
eroman
2014/05/20 00:32:19
Not sure I follow.
GenerateKey() does a pre-check
|
+ |
+ return Status::Success(); |
+} |
+ |
} // namespace |
void Init() { platform::Init(); } |
@@ -556,6 +701,11 @@ Status GenerateSecretKey(const blink::WebCryptoAlgorithm& algorithm, |
bool extractable, |
blink::WebCryptoKeyUsageMask usage_mask, |
blink::WebCryptoKey* key) { |
+ Status status = |
+ CheckKeyUsages(algorithm.id(), blink::WebCryptoKeyTypeSecret, usage_mask); |
+ if (status.IsError()) |
+ return status; |
+ |
unsigned int keylen_bytes = 0; |
// Get the secret key length in bytes from generation parameters. |
@@ -600,9 +750,19 @@ Status GenerateSecretKey(const blink::WebCryptoAlgorithm& algorithm, |
Status GenerateKeyPair(const blink::WebCryptoAlgorithm& algorithm, |
bool extractable, |
- blink::WebCryptoKeyUsageMask usage_mask, |
+ blink::WebCryptoKeyUsageMask combined_usage_mask, |
blink::WebCryptoKey* public_key, |
blink::WebCryptoKey* private_key) { |
+ blink::WebCryptoKeyUsageMask public_key_usage_mask = 0; |
+ blink::WebCryptoKeyUsageMask private_key_usage_mask = 0; |
+ |
+ Status status = CheckKeyUsagesForGenerateKeyPair(algorithm.id(), |
+ combined_usage_mask, |
+ &public_key_usage_mask, |
+ &private_key_usage_mask); |
+ if (status.IsError()) |
+ return status; |
+ |
// TODO(padolph): Handle other asymmetric algorithm key generation. |
switch (algorithm.paramsType()) { |
case blink::WebCryptoAlgorithmParamsTypeRsaHashedKeyGenParams: |
@@ -626,7 +786,8 @@ Status GenerateKeyPair(const blink::WebCryptoAlgorithm& algorithm, |
return platform::GenerateRsaKeyPair(algorithm, |
extractable, |
- usage_mask, |
+ public_key_usage_mask, |
+ private_key_usage_mask, |
params->modulusLengthBits(), |
publicExponent, |
hash_or_null, |
@@ -645,20 +806,13 @@ Status ImportKey(blink::WebCryptoKeyFormat format, |
bool extractable, |
blink::WebCryptoKeyUsageMask usage_mask, |
blink::WebCryptoKey* key) { |
- switch (format) { |
- case blink::WebCryptoKeyFormatRaw: |
- return ImportKeyRaw(key_data, algorithm, extractable, usage_mask, key); |
- case blink::WebCryptoKeyFormatSpki: |
- return platform::ImportKeySpki( |
- algorithm, key_data, extractable, usage_mask, key); |
- case blink::WebCryptoKeyFormatPkcs8: |
- return platform::ImportKeyPkcs8( |
- algorithm, key_data, extractable, usage_mask, key); |
- case blink::WebCryptoKeyFormatJwk: |
- return ImportKeyJwk(key_data, algorithm, extractable, usage_mask, key); |
- default: |
- return Status::ErrorUnsupported(); |
- } |
+ Status status = ImportKeyDontCheckKeyUsages( |
+ format, key_data, algorithm, extractable, usage_mask, key); |
Ryan Sleevi
2014/05/15 06:29:40
Shouldn't we be able to fast-fail if |algorithm| a
eroman
2014/05/16 00:04:37
I opted to do the key usages test *after* key crea
Ryan Sleevi
2014/05/19 23:06:29
For the similar reasons as the generate case above
eroman
2014/05/20 01:27:57
FYI: I am updating the CL to do fail-fast wherever
|
+ |
+ if (status.IsError()) |
+ return status; |
+ |
+ return CheckKeyUsages(algorithm.id(), key->type(), usage_mask); |
} |
// TODO(eroman): Move this to anonymous namespace. |
@@ -783,36 +937,18 @@ Status UnwrapKey(blink::WebCryptoKeyFormat format, |
bool extractable, |
blink::WebCryptoKeyUsageMask usage_mask, |
blink::WebCryptoKey* key) { |
- if (!KeyUsageAllows(wrapping_key, blink::WebCryptoKeyUsageUnwrapKey)) |
- return Status::ErrorUnexpected(); |
- if (wrapping_algorithm.id() != wrapping_key.algorithm().id()) |
- return Status::ErrorUnexpected(); |
+ Status status = UnwrapKeyDontCheckKeyUsages(format, |
+ wrapped_key_data, |
+ wrapping_key, |
+ wrapping_algorithm, |
+ algorithm, |
+ extractable, |
+ usage_mask, |
+ key); |
+ if (status.IsError()) |
+ return status; |
- switch (format) { |
- case blink::WebCryptoKeyFormatRaw: |
- return UnwrapKeyRaw(wrapped_key_data, |
- wrapping_key, |
- wrapping_algorithm, |
- algorithm, |
- extractable, |
- usage_mask, |
- key); |
- case blink::WebCryptoKeyFormatJwk: |
- return UnwrapKeyDecryptAndImport(format, |
- wrapped_key_data, |
- wrapping_key, |
- wrapping_algorithm, |
- algorithm, |
- extractable, |
- usage_mask, |
- key); |
- case blink::WebCryptoKeyFormatSpki: |
- case blink::WebCryptoKeyFormatPkcs8: |
- return Status::ErrorUnsupported(); // TODO(padolph) |
- default: |
- NOTREACHED(); |
- return Status::ErrorUnsupported(); |
- } |
+ return CheckKeyUsages(algorithm.id(), key->type(), usage_mask); |
} |
// Note that this function is called from the target Blink thread. |