CRYPTO_128_unwrap(): Fix refactoring damage

crypto/modes/wrap128.c was heavily refactored to support AES Key Wrap
with Padding, and four bugs were introduced into CRYPTO_128_unwrap() at
that time:

- crypto_128_unwrap_raw()'s return value ('ret') is checked incorrectly,
  and the function immediately returns 'ret' in (almost) all cases.
  This makes the IV checking code later in the function unreachable, but
  callers think the IV check succeeded since CRYPTO_128_unwrap()'s
  return value is non-zero.

  FIX: Return 0 (error) if crypto_128_unwrap_raw() returned 0 (error).

- crypto_128_unwrap_raw() writes the IV to the 'got_iv' buffer, not to
  the first 8 bytes of the output buffer ('out') as the IV checking code
  expects.  This makes the IV check fail.

  FIX: Compare 'iv' to 'got_iv', not 'out'.

- The data written to the output buffer ('out') is "cleansed" if the IV
  check fails, but the code passes OPENSSL_cleanse() the input buffer
  length ('inlen') instead of the number of bytes that
  crypto_128_unwrap_raw() wrote to the output buffer ('ret').  This
  means that OPENSSL_cleanse() could potentially write past the end of
  'out'.

  FIX: Change 'inlen' to 'ret' in the OPENSSL_cleanse() call.

- CRYPTO_128_unwrap() is returning the length of the input buffer
  ('inlen') instead of the number of bytes written to the output buffer
  ('ret').  This could cause the caller to read past the end of 'out'.

  FIX: Return 'ret' instead of 'inlen' at the end of the function.

PR#3749

Reviewed-by: Stephen Henson <steve@openssl.org>
Reviewed-by: Emilia Käsper <emilia@openssl.org>

diff --git a/crypto/modes/wrap128.c b/crypto/modes/wrap128.c
index e1191e496e9eab6fb31ea525d3c67eb407e47474..fe33a986bed572cc1b45fc9956b6bd2781516faa 100644 (file)
--- a/crypto/modes/wrap128.c
+++ b/crypto/modes/wrap128.c
@@ -201,16 +201,16 @@ size_t CRYPTO_128_unwrap(void *key, const unsigned char *iv,
     unsigned char got_iv[8];
 
     ret = crypto_128_unwrap_raw(key, got_iv, out, in, inlen, block);
-    if (ret != inlen)
-        return ret;
+    if (ret == 0)
+        return 0;
 
     if (!iv)
         iv = default_iv;
-    if (CRYPTO_memcmp(out, iv, 8)) {
-        OPENSSL_cleanse(out, inlen);
+    if (CRYPTO_memcmp(got_iv, iv, 8)) {
+        OPENSSL_cleanse(out, ret);
         return 0;
     }
-    return inlen;
+    return ret;
 }
 
 /** Wrapping according to RFC 5649 section 4.1.