[openssl-commits] [openssl] master update

Dr. Stephen Henson steve at openssl.org
Sat Mar 21 20:05:15 UTC 2015


The branch master has been updated
       via  77e127ea6e4801a0bb584717f966fa17adabc45f (commit)
       via  5724bd49a2f11e1e9663ac82f4b5e63e18da65e6 (commit)
       via  e6abba3ad6107d35a6e8b01a1a145902edf0062d (commit)
       via  1062ecfc53622ff42edef5af63ace39c23dd3b49 (commit)
      from  3c381e54233be3d0dcbce7cc853c4767d979fe90 (commit)


- Log -----------------------------------------------------------------
commit 77e127ea6e4801a0bb584717f966fa17adabc45f
Author: Dr. Stephen Henson <steve at openssl.org>
Date:   Fri Mar 20 22:53:16 2015 +0000

    Add AES unwrap test with invalid key.
    
    This tests the unwrap algorithm with an invalid key. The result should
    be rejected without returning any plaintext.
    
    Reviewed-by: Emilia Käsper <emilia at openssl.org>

commit 5724bd49a2f11e1e9663ac82f4b5e63e18da65e6
Author: Dr. Stephen Henson <steve at openssl.org>
Date:   Fri Mar 20 23:08:30 2015 +0000

    Fix memory leak.
    
    Reviewed-by: Emilia Käsper <emilia at openssl.org>

commit e6abba3ad6107d35a6e8b01a1a145902edf0062d
Author: Richard Godbee <richard at godbee.net>
Date:   Fri Mar 13 21:23:21 2015 -0700

    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 at openssl.org>
    Reviewed-by: Emilia Käsper <emilia at openssl.org>

commit 1062ecfc53622ff42edef5af63ace39c23dd3b49
Author: Richard Godbee <richard at godbee.net>
Date:   Fri Mar 13 20:54:39 2015 -0700

    wrap128.c: Fix Doxygen comments
    
    Reviewed-by: Stephen Henson <steve at openssl.org>
    Reviewed-by: Emilia Käsper <emilia at openssl.org>

-----------------------------------------------------------------------

Summary of changes:
 crypto/evp/evp_test.c   | 20 ++++++++++++++++----
 crypto/evp/evptests.txt |  9 +++++++++
 crypto/modes/wrap128.c  | 49 +++++++++++++++++++++++++------------------------
 3 files changed, 50 insertions(+), 28 deletions(-)

diff --git a/crypto/evp/evp_test.c b/crypto/evp/evp_test.c
index 1b17f64..1524658 100644
--- a/crypto/evp/evp_test.c
+++ b/crypto/evp/evp_test.c
@@ -247,16 +247,27 @@ static void hex_print(const char *name, const unsigned char *buf, size_t len)
     fputs("\n", stderr);
 }
 
+static void free_expected(struct evp_test *t)
+{
+    if (t->expected_err) {
+        OPENSSL_free(t->expected_err);
+        t->expected_err = NULL;
+    }
+    if (t->out_expected) {
+        OPENSSL_free(t->out_expected);
+        OPENSSL_free(t->out_got);
+        t->out_expected = NULL;
+        t->out_got = NULL;
+    }
+}
+
 static void print_expected(struct evp_test *t)
 {
     if (t->out_expected == NULL)
         return;
     hex_print("Expected:", t->out_expected, t->out_len);
     hex_print("Got:     ", t->out_got, t->out_len);
-    OPENSSL_free(t->out_expected);
-    OPENSSL_free(t->out_got);
-    t->out_expected = NULL;
-    t->out_got = NULL;
+    free_expected(t);
 }
 
 static int check_test_error(struct evp_test *t)
@@ -313,6 +324,7 @@ static int setup_test(struct evp_test *t, const struct evp_test_method *tmeth)
             OPENSSL_free(t->expected_err);
             t->expected_err = NULL;
         }
+        free_expected(t);
     }
     t->meth = tmeth;
     return 1;
diff --git a/crypto/evp/evptests.txt b/crypto/evp/evptests.txt
index 8bf90d0..26d371c 100644
--- a/crypto/evp/evptests.txt
+++ b/crypto/evp/evptests.txt
@@ -2002,6 +2002,15 @@ Key = 000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E1F
 Plaintext = 00112233445566778899AABBCCDDEEFF000102030405060708090A0B0C0D0E0F
 Ciphertext = 28C9F404C4B810F4CBCCB35CFB87F8263F5786E2D80ED326CBC7F0E71A99F43BFB988B9B7A02DD21
 
+# Same as previous example but with invalid unwrap key: should be rejected
+# without returning any plaintext
+Cipher = id-aes256-wrap
+Operation = DECRYPT
+Key = 000102030405060708090A0B0C0D0E0F101112131415161718191A1B1C1D1E00
+Plaintext = 00112233445566778899AABBCCDDEEFF000102030405060708090A0B0C0D0E0F
+Ciphertext = 28C9F404C4B810F4CBCCB35CFB87F8263F5786E2D80ED326CBC7F0E71A99F43BFB988B9B7A02DD21
+Result = CIPHERUPDATE_ERROR
+
 # AES wrap tests from RFC5649
 Cipher = id-aes192-wrap-pad
 Key = 5840df6e29b02af1ab493b705bf16ea1ae8338f4dcc176a8
diff --git a/crypto/modes/wrap128.c b/crypto/modes/wrap128.c
index 73718ae..fe33a98 100644
--- a/crypto/modes/wrap128.c
+++ b/crypto/modes/wrap128.c
@@ -81,9 +81,9 @@ static const unsigned char default_aiv[] = {
  *
  *  @param[in]  key    Key value.
  *  @param[in]  iv     IV value. Length = 8 bytes. NULL = use default_iv.
- *  @param[in]  in     Plain text as n 64-bit blocks, n >= 2.
+ *  @param[in]  in     Plaintext as n 64-bit blocks, n >= 2.
  *  @param[in]  inlen  Length of in.
- *  @param[out] out    Cipher text. Minimal buffer length = (inlen + 8) bytes.
+ *  @param[out] out    Ciphertext. Minimal buffer length = (inlen + 8) bytes.
  *                     Input and output buffers can overlap if block function
  *                     supports that.
  *  @param[in]  block  Block processing function.
@@ -127,19 +127,19 @@ size_t CRYPTO_128_wrap(void *key, const unsigned char *iv,
 }
 
 /** Unwrapping according to RFC 3394 section 2.2.2 steps 1-2.
- *  IV check (step 3) is responsibility of the caller.
+ *  The IV check (step 3) is responsibility of the caller.
  *
  *  @param[in]  key    Key value.
  *  @param[out] iv     Unchecked IV value. Minimal buffer length = 8 bytes.
- *  @param[out] out    Plain text without IV.
+ *  @param[out] out    Plaintext without IV.
  *                     Minimal buffer length = (inlen - 8) bytes.
  *                     Input and output buffers can overlap if block function
  *                     supports that.
- *  @param[in]  in     Ciphertext text as n 64-bit blocks
+ *  @param[in]  in     Ciphertext as n 64-bit blocks.
  *  @param[in]  inlen  Length of in.
  *  @param[in]  block  Block processing function.
  *  @return            0 if inlen is out of range [24, CRYPTO128_WRAP_MAX]
- *                     or if inlen is not multiply of 8.
+ *                     or if inlen is not a multiple of 8.
  *                     Output length otherwise.
  */
 static size_t crypto_128_unwrap_raw(void *key, unsigned char *iv,
@@ -174,21 +174,22 @@ static size_t crypto_128_unwrap_raw(void *key, unsigned char *iv,
     return inlen;
 }
 
-/** Unwrapping according to RFC 3394 section 2.2.2 including IV check.
- *  First block of plain text have to match supplied IV otherwise an error is
- *  returned.
+/** Unwrapping according to RFC 3394 section 2.2.2, including the IV check.
+ *  The first block of plaintext has to match the supplied IV, otherwise an
+ *  error is returned.
  *
  *  @param[in]  key    Key value.
- *  @param[out] iv     Unchecked IV value. Minimal buffer length = 8 bytes.
- *  @param[out] out    Plain text without IV.
+ *  @param[out] iv     IV value to match against. Length = 8 bytes.
+ *                     NULL = use default_iv.
+ *  @param[out] out    Plaintext without IV.
  *                     Minimal buffer length = (inlen - 8) bytes.
  *                     Input and output buffers can overlap if block function
  *                     supports that.
- *  @param[in]  in     Ciphertext text as n 64-bit blocks
+ *  @param[in]  in     Ciphertext as n 64-bit blocks.
  *  @param[in]  inlen  Length of in.
  *  @param[in]  block  Block processing function.
  *  @return            0 if inlen is out of range [24, CRYPTO128_WRAP_MAX]
- *                     or if inlen is not multiply of 8
+ *                     or if inlen is not a multiple of 8
  *                     or if IV doesn't match expected value.
  *                     Output length otherwise.
  */
@@ -200,26 +201,26 @@ 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.
  *
  *  @param[in]  key    Key value.
  *  @param[in]  icv    (Non-standard) IV, 4 bytes. NULL = use default_aiv.
- *  @param[out] out    Cipher text. Minimal buffer length = (inlen + 15) bytes.
+ *  @param[out] out    Ciphertext. Minimal buffer length = (inlen + 15) bytes.
  *                     Input and output buffers can overlap if block function
  *                     supports that.
- *  @param[in]  in     Plain text as n 64-bit blocks, n >= 2.
+ *  @param[in]  in     Plaintext as n 64-bit blocks, n >= 2.
  *  @param[in]  inlen  Length of in.
  *  @param[in]  block  Block processing function.
  *  @return            0 if inlen is out of range [1, CRYPTO128_WRAP_MAX].
@@ -282,14 +283,14 @@ 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    Plain text. Minimal buffer length = inlen bytes.
+ *  @param[out] out    Plaintext. Minimal buffer length = inlen bytes.
  *                     Input and output buffers can overlap if block function
  *                     supports that.
- *  @param[in]  in     Ciphertext text as n 64-bit blocks
+ *  @param[in]  in     Ciphertext as n 64-bit blocks.
  *  @param[in]  inlen  Length of in.
  *  @param[in]  block  Block processing function.
  *  @return            0 if inlen is out of range [16, CRYPTO128_WRAP_MAX],
- *                     or if inlen is not multiply of 8
+ *                     or if inlen is not a multiple of 8
  *                     or if IV and message length indicator doesn't match.
  *                     Output length if unwrapping succeeded and IV matches.
  */
@@ -308,7 +309,7 @@ size_t CRYPTO_128_unwrap_pad(void *key, const unsigned char *icv,
     static unsigned char zeros[8] = { 0x0 };
     size_t ret;
 
-    /* Section 4.2: Cipher text length has to be (n+1) 64-bit blocks. */
+    /* Section 4.2: Ciphertext length has to be (n+1) 64-bit blocks. */
     if ((inlen & 0x7) != 0 || inlen < 16 || inlen >= CRYPTO128_WRAP_MAX)
         return 0;
 


More information about the openssl-commits mailing list