crypto: add test case for AES key wrapping - #20587
Conversation
Sorry, something went wrong.
Add test cases for AES key wrapping and only detect output length in cipher case. The reason being is the returned output length is insufficient in AES key unwrapping case. Signed-off-by: Yihong Wang <yh.wang@ibm.com>
|
I tried to use the same mechanism to detect the output length for the AES key unwrapping and I found it may be a glitch in openssl. In the AES key unwrapping code path, it returns ciphertext length - 8 as output lenght. But when it performs the unwrapping, it expect the output length should be equal to ciphertext length. So I skip the output length detection in decipher case. You can see CRYPTO_128_unwrap_pad() in wrap128.c. Inside CRYPTO_128_unwrap_pad(), it says the minimal buffer length of out should be equal to inlen. But in aes_wrap_cipher(), the padding decipher case, it says minimal buffer length of out is inlen - 8. I have a patch for wrap128.c here: --- a/deps/openssl/openssl/crypto/modes/wrap128.c
+++ b/deps/openssl/openssl/crypto/modes/wrap128.c
@@ -237,7 +237,7 @@ size_t CRYPTO_128_wrap_pad(void *key, const unsigned char *icv,
*
* @param[in] key Key value.
* @param[in] icv (Non-standard) IV, 4 bytes. NULL = use default_aiv.
- * @param[out] out Plaintext. Minimal buffer length = inlen bytes.
+ * @param[out] out Plaintext. Minimal buffer length = inlen - 8 bytes.
* Input and output buffers can overlap if block function
* supports that.
* @param[in] in Ciphertext as n 64-bit blocks.
@@ -267,7 +267,6 @@ size_t CRYPTO_128_unwrap_pad(void *key, const unsigned char *icv,
if ((inlen & 0x7) != 0 || inlen < 16 || inlen >= CRYPTO128_WRAP_MAX)
return 0;
- memmove(out, in, inlen);
if (inlen == 16) {
/*
* Section 4.2 - special case in step 1: When n=1, the ciphertext
@@ -275,14 +274,15 @@ size_t CRYPTO_128_unwrap_pad(void *key, const unsigned char *icv,
* single AES block using AES in ECB mode: AIV | P[1] = DEC(K, C[0] |
* C[1])
*/
+ memmove(out, in + 8, inlen - 8);
block(out, out, key);
- memcpy(aiv, out, 8);
+ memcpy(aiv, in, 8);
/* Remove AIV */
memmove(out, out + 8, 8);
padded_len = 8;
} else {
padded_len = inlen - 8;
- ret = crypto_128_unwrap_raw(key, aiv, out, out, inlen, block);
+ ret = crypto_128_unwrap_raw(key, aiv, out, in, inlen, block);
if (padded_len != ret) {
OPENSSL_cleanse(out, inlen);
return 0;But even if we patch wrap128.c, for openssl shared lib user, we still need to skip the output length detection. |
Sorry, something went wrong.
bnoordhuis
left a comment
There was a problem hiding this comment.
LGTM. Were you planning to bring this up with the upstream openssl project as well?
Sorry, something went wrong.
| // For key wrapping algorithms, get output size by calling | ||
| // EVP_CipherUpdate() with null output. | ||
| if (mode == EVP_CIPH_WRAP_MODE && | ||
| if (kind_ == kCipher && mode == EVP_CIPH_WRAP_MODE && |
There was a problem hiding this comment.
Is this change intended? If so, this likely should be a different commit.
Sorry, something went wrong.
There was a problem hiding this comment.
@jasnell this is needed. Sorry the title of this commit is a little misleading but I explained that I need the code change in the description
Sorry, something went wrong.
Sorry, something went wrong.
Add test cases for AES key wrapping and only detect output length in cipher case. The reason being is the returned output length is insufficient in AES key unwrapping case. PR-URL: #20587 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Add test cases for AES key wrapping and only detect output length in cipher case. The reason being is the returned output length is insufficient in AES key unwrapping case. PR-URL: nodejs#20587 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Add test cases for AES key wrapping and only detect output length in cipher case. The reason being is the returned output length is insufficient in AES key unwrapping case. Backport-PR-URL: #20706 PR-URL: #20587 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Tobias Nießen <tniessen@tnie.de>
|
@bnoordhuis the related change is upstreamed to OpenSSL: openssl/openssl#6266 |
Sorry, something went wrong.
Add test cases for AES key wrapping and only detect output length in
cipher case. The reason being is the returned output length is
insufficient in AES key unwrapping case.
Signed-off-by: Yihong Wang yh.wang@ibm.com
Checklist