Change RC5_32_set_key to return an int type
authorMatt Caswell <matt@openssl.org>
Fri, 28 Jun 2019 15:29:42 +0000 (16:29 +0100)
committerMatt Caswell <matt@openssl.org>
Mon, 1 Jul 2019 09:18:37 +0000 (10:18 +0100)
If the key is too long we now return an error.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from https://github.com/openssl/openssl/pull/8834)

CHANGES
apps/speed.c
crypto/evp/e_rc5.c
crypto/rc5/rc5_skey.c
include/openssl/rc5.h
test/rc5test.c

diff --git a/CHANGES b/CHANGES
index 4c70b93..8b70fa3 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,12 @@
 
  Changes between 1.1.1 and 3.0.0 [xx XXX xxxx]
 
 
  Changes between 1.1.1 and 3.0.0 [xx XXX xxxx]
 
+  *) RC5_32_set_key has been changed to return an int type, with 0 indicating
+     an error and 1 indicating success. In previous versions of OpenSSL this
+     was a void type. If a key was set longer than the maximum possible this
+     would crash.
+     [Matt Caswell]
+
   *) Support SM2 signing and verification schemes with X509 certificate.
      [Paul Yang]
 
   *) Support SM2 signing and verification schemes with X509 certificate.
      [Paul Yang]
 
index 5f16b13..0f3ca9c 100644 (file)
@@ -1985,7 +1985,10 @@ int speed_main(int argc, char **argv)
     RC2_set_key(&rc2_ks, 16, key16, 128);
 #endif
 #ifndef OPENSSL_NO_RC5
     RC2_set_key(&rc2_ks, 16, key16, 128);
 #endif
 #ifndef OPENSSL_NO_RC5
-    RC5_32_set_key(&rc5_ks, 16, key16, 12);
+    if (!RC5_32_set_key(&rc5_ks, 16, key16, 12)) {
+        BIO_printf(bio_err, "Failed setting RC5 key\n");
+        goto end;
+    }
 #endif
 #ifndef OPENSSL_NO_BF
     BF_set_key(&bf_ks, 16, key16);
 #endif
 #ifndef OPENSSL_NO_BF
     BF_set_key(&bf_ks, 16, key16);
index fdd4e9d..95a626b 100644 (file)
@@ -70,9 +70,8 @@ static int r_32_12_16_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key,
         EVPerr(EVP_F_R_32_12_16_INIT_KEY, EVP_R_BAD_KEY_LENGTH);
         return 0;
     }
         EVPerr(EVP_F_R_32_12_16_INIT_KEY, EVP_R_BAD_KEY_LENGTH);
         return 0;
     }
-    RC5_32_set_key(&data(ctx)->ks, EVP_CIPHER_CTX_key_length(ctx),
-                   key, data(ctx)->rounds);
-    return 1;
+    return RC5_32_set_key(&data(ctx)->ks, EVP_CIPHER_CTX_key_length(ctx),
+                          key, data(ctx)->rounds);
 }
 
 #endif
 }
 
 #endif
index 1746406..43dc932 100644 (file)
 #include <openssl/rc5.h>
 #include "rc5_locl.h"
 
 #include <openssl/rc5.h>
 #include "rc5_locl.h"
 
-void RC5_32_set_key(RC5_32_KEY *key, int len, const unsigned char *data,
-                    int rounds)
+int RC5_32_set_key(RC5_32_KEY *key, int len, const unsigned char *data,
+                   int rounds)
 {
     RC5_32_INT L[64], l, ll, A, B, *S, k;
     int i, j, m, c, t, ii, jj;
 
 {
     RC5_32_INT L[64], l, ll, A, B, *S, k;
     int i, j, m, c, t, ii, jj;
 
+    if (len > 255)
+        return 0;
+
     if ((rounds != RC5_16_ROUNDS) &&
         (rounds != RC5_12_ROUNDS) && (rounds != RC5_8_ROUNDS))
         rounds = RC5_16_ROUNDS;
     if ((rounds != RC5_16_ROUNDS) &&
         (rounds != RC5_12_ROUNDS) && (rounds != RC5_8_ROUNDS))
         rounds = RC5_16_ROUNDS;
@@ -58,4 +61,6 @@ void RC5_32_set_key(RC5_32_KEY *key, int len, const unsigned char *data,
         if (++jj >= c)
             jj = 0;
     }
         if (++jj >= c)
             jj = 0;
     }
+
+    return 1;
 }
 }
index 80a7d68..97e22f7 100644 (file)
@@ -39,8 +39,8 @@ typedef struct rc5_key_st {
     RC5_32_INT data[2 * (RC5_16_ROUNDS + 1)];
 } RC5_32_KEY;
 
     RC5_32_INT data[2 * (RC5_16_ROUNDS + 1)];
 } RC5_32_KEY;
 
-void RC5_32_set_key(RC5_32_KEY *key, int len, const unsigned char *data,
-                    int rounds);
+int RC5_32_set_key(RC5_32_KEY *key, int len, const unsigned char *data,
+                   int rounds);
 void RC5_32_ecb_encrypt(const unsigned char *in, unsigned char *out,
                         RC5_32_KEY *key, int enc);
 void RC5_32_encrypt(unsigned long *data, RC5_32_KEY *key);
 void RC5_32_ecb_encrypt(const unsigned char *in, unsigned char *out,
                         RC5_32_KEY *key, int enc);
 void RC5_32_encrypt(unsigned long *data, RC5_32_KEY *key);
index 16f4071..39a113e 100644 (file)
@@ -181,7 +181,8 @@ static int test_rc5_ecb(int n)
     RC5_32_KEY key;
     unsigned char buf[8], buf2[8];
 
     RC5_32_KEY key;
     unsigned char buf[8], buf2[8];
 
-    RC5_32_set_key(&key, 16, &RC5key[n][0], 12);
+    if (!TEST_true(RC5_32_set_key(&key, 16, &RC5key[n][0], 12)))
+        return 0;
 
     RC5_32_ecb_encrypt(&RC5plain[n][0], buf, &key, RC5_ENCRYPT);
     if (!TEST_mem_eq(&RC5cipher[n][0], sizeof(RC5cipher[0]), buf, sizeof(buf)))
 
     RC5_32_ecb_encrypt(&RC5plain[n][0], buf, &key, RC5_ENCRYPT);
     if (!TEST_mem_eq(&RC5cipher[n][0], sizeof(RC5cipher[0]), buf, sizeof(buf)))
@@ -203,7 +204,9 @@ static int test_rc5_cbc(int n)
 
     i = rc5_cbc_rounds[n];
     if (i >= 8) {
 
     i = rc5_cbc_rounds[n];
     if (i >= 8) {
-        RC5_32_set_key(&key, rc5_cbc_key[n][0], &rc5_cbc_key[n][1], i);
+        if (!TEST_true(RC5_32_set_key(&key, rc5_cbc_key[n][0],
+                                      &rc5_cbc_key[n][1], i)))
+            return 0;
 
         memcpy(ivb, &rc5_cbc_iv[n][0], 8);
         RC5_32_cbc_encrypt(&rc5_cbc_plain[n][0], buf, 8,
 
         memcpy(ivb, &rc5_cbc_iv[n][0], 8);
         RC5_32_cbc_encrypt(&rc5_cbc_plain[n][0], buf, 8,