Skip to content

Commit

Permalink
Fix the error message when generating an AES key using a length other…
Browse files Browse the repository at this point in the history
… than 128, 192, 256.

Also changes the error type to OperationError to match the spec update:
https://dvcs.w3.org/hg/webcrypto-api/rev/5e7ba79bdf36

BUG=436581,425670

Review URL: https://codereview.chromium.org/762593003

Cr-Commit-Position: refs/heads/master@{#305769}
  • Loading branch information
eroman authored and Commit bot committed Nov 26, 2014
1 parent 0731f29 commit 9e9ed05
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 11 deletions.
20 changes: 19 additions & 1 deletion content/child/webcrypto/status.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,17 @@ Status Status::ErrorUnsupportedExportKeyFormat() {
}

Status Status::ErrorImportAesKeyLength() {
// TODO(eroman): Mention only 128 and 256 bits since 192 is unsupported.
// http://crbug.com/425670
return Status(blink::WebCryptoErrorTypeData,
"AES key data must be 128, 192 or 256 bits");
}

Status Status::ErrorGenerateAesKeyLength() {
return Status(blink::WebCryptoErrorTypeOperation,
"AES key length must be 128 or 256 bits");
}

Status Status::ErrorAes192BitUnsupported() {
return Status(blink::WebCryptoErrorTypeNotSupported,
"192-bit AES keys are not supported");
Expand Down Expand Up @@ -241,12 +248,23 @@ Status Status::ErrorKeyNotExtractable() {
"They key is not extractable");
}

Status Status::ErrorGenerateKeyLength() {
Status Status::ErrorGenerateHmacKeyLengthPartialByte() {
// TODO(eroman): This message needs to be fixed (however appears in a
// LayoutTest).
// * The error type is no longer spec compliant
// * The message text is poor
// * In fact the spec no longer requires key lengths to be multiples of 8
// bits so this message is bogus (http://crbug.com/431085)
return Status(blink::WebCryptoErrorTypeData,
"Invalid key length: it is either zero or not a multiple of 8 "
"bits");
}

Status Status::ErrorGenerateHmacKeyLengthZero() {
return Status(blink::WebCryptoErrorTypeOperation,
"HMAC key length must be not be zero");
}

Status Status::ErrorCreateKeyBadUsages() {
return Status(blink::WebCryptoErrorTypeSyntax,
"Cannot create a key using the specified key usages.");
Expand Down
12 changes: 9 additions & 3 deletions content/child/webcrypto/status.h
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,9 @@ class CONTENT_EXPORT Status {
// AES.
static Status ErrorImportAesKeyLength();

// Attempted to generate an AES key with an invalid length.
static Status ErrorGenerateAesKeyLength();

// 192-bit AES keys are valid, however unsupported.
static Status ErrorAes192BitUnsupported();

Expand Down Expand Up @@ -205,9 +208,12 @@ class CONTENT_EXPORT Status {
// An unextractable key was used by an operation which exports the key data.
static Status ErrorKeyNotExtractable();

// The key length specified when generating a key was invalid. Either it was
// zero, or it was not a multiple of 8 bits.
static Status ErrorGenerateKeyLength();
// Attempted to generate an HMAC key with a key length in bits that was not a
// multiple of 8.
static Status ErrorGenerateHmacKeyLengthPartialByte();

// Attempted to generate an HMAC key using a key length of 0.
static Status ErrorGenerateHmacKeyLengthZero();

// Attempted to create a key (either by importKey(), generateKey(), or
// unwrapKey()) however the key usages were not applicable for the key type
Expand Down
2 changes: 1 addition & 1 deletion content/child/webcrypto/test/aes_cbc_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ TEST(WebCryptoAesCbcTest, GenerateKeyBadLength) {
blink::WebCryptoKey key;
for (size_t i = 0; i < arraysize(kKeyLen); ++i) {
SCOPED_TRACE(i);
EXPECT_EQ(Status::ErrorGenerateKeyLength(),
EXPECT_EQ(Status::ErrorGenerateAesKeyLength(),
GenerateSecretKey(CreateAesCbcKeyGenAlgorithm(kKeyLen[i]), true,
0, &key));
}
Expand Down
2 changes: 1 addition & 1 deletion content/child/webcrypto/test/aes_gcm_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ TEST(WebCryptoAesGcmTest, GenerateKeyBadLength) {
blink::WebCryptoKey key;
for (size_t i = 0; i < arraysize(kKeyLen); ++i) {
SCOPED_TRACE(i);
EXPECT_EQ(Status::ErrorGenerateKeyLength(),
EXPECT_EQ(Status::ErrorGenerateAesKeyLength(),
GenerateSecretKey(CreateAesGcmKeyGenAlgorithm(kKeyLen[i]), true,
0, &key));
}
Expand Down
2 changes: 1 addition & 1 deletion content/child/webcrypto/test/aes_kw_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ TEST(WebCryptoAesKwTest, GenerateKeyBadLength) {
blink::WebCryptoKey key;
for (size_t i = 0; i < arraysize(kKeyLen); ++i) {
SCOPED_TRACE(i);
EXPECT_EQ(Status::ErrorGenerateKeyLength(),
EXPECT_EQ(Status::ErrorGenerateAesKeyLength(),
GenerateSecretKey(CreateAesKwKeyGenAlgorithm(kKeyLen[i]), true, 0,
&key));
}
Expand Down
10 changes: 6 additions & 4 deletions content/child/webcrypto/webcrypto_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ Status GetAesKeyGenLengthInBits(const blink::WebCryptoAesKeyGenParams* params,
if (*keylen_bits == 192)
return Status::ErrorAes192BitUnsupported();

return Status::ErrorGenerateKeyLength();
return Status::ErrorGenerateAesKeyLength();
}

Status GetHmacKeyGenLengthInBits(const blink::WebCryptoHmacKeyGenParams* params,
Expand All @@ -203,14 +203,16 @@ Status GetHmacKeyGenLengthInBits(const blink::WebCryptoHmacKeyGenParams* params,
}
}

// TODO(eroman): Non multiple of 8 bit keylengths should be allowed:
// http://crbug.com/431085.
if (params->optionalLengthBits() % 8)
return Status::ErrorGenerateKeyLength();
return Status::ErrorGenerateHmacKeyLengthPartialByte();

*keylen_bits = params->optionalLengthBits();

// TODO(eroman): NSS fails when generating a zero-length secret key.
// Zero-length HMAC keys are disallowed by the spec.
if (*keylen_bits == 0)
return Status::ErrorGenerateKeyLength();
return Status::ErrorGenerateHmacKeyLengthZero();

return Status::Success();
}
Expand Down

0 comments on commit 9e9ed05

Please sign in to comment.