Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that rc5 doesn't try to use a key longer than 2040 bits #8834

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@

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]

Expand Down
5 changes: 4 additions & 1 deletion apps/speed.c
Original file line number Diff line number Diff line change
Expand Up @@ -1985,7 +1985,10 @@ int speed_main(int argc, char **argv)
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);
Expand Down
2 changes: 2 additions & 0 deletions crypto/err/openssl.txt
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,7 @@ EVP_F_PKEY_SET_TYPE:158:pkey_set_type
EVP_F_POLY1305_CTRL:216:poly1305_ctrl
EVP_F_RC2_MAGIC_TO_METH:109:rc2_magic_to_meth
EVP_F_RC5_CTRL:125:rc5_ctrl
EVP_F_R_32_12_16_INIT_KEY:242:r_32_12_16_init_key
EVP_F_S390X_AES_GCM_CTRL:201:s390x_aes_gcm_ctrl
EVP_F_S390X_AES_GCM_TLS_CIPHER:208:s390x_aes_gcm_tls_cipher
EVP_F_SCRYPT_ALG:228:scrypt_alg
Expand Down Expand Up @@ -2385,6 +2386,7 @@ ESS_R_ESS_SIGNING_CERT_V2_ADD_ERROR:101:ess signing cert v2 add error
EVP_R_AES_KEY_SETUP_FAILED:143:aes key setup failed
EVP_R_ARIA_KEY_SETUP_FAILED:176:aria key setup failed
EVP_R_BAD_DECRYPT:100:bad decrypt
EVP_R_BAD_KEY_LENGTH:195:bad key length
EVP_R_BUFFER_TOO_SMALL:155:buffer too small
EVP_R_CAMELLIA_KEY_SETUP_FAILED:157:camellia key setup failed
EVP_R_CIPHER_NOT_GCM_MODE:184:cipher not gcm mode
Expand Down
9 changes: 6 additions & 3 deletions crypto/evp/e_rc5.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,12 @@ static int rc5_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr)
static int r_32_12_16_init_key(EVP_CIPHER_CTX *ctx, const unsigned char *key,
const unsigned char *iv, int enc)
{
RC5_32_set_key(&data(ctx)->ks, EVP_CIPHER_CTX_key_length(ctx),
key, data(ctx)->rounds);
return 1;
if (EVP_CIPHER_CTX_key_length(ctx) > 255) {
EVPerr(EVP_F_R_32_12_16_INIT_KEY, EVP_R_BAD_KEY_LENGTH);
return 0;
}
return RC5_32_set_key(&data(ctx)->ks, EVP_CIPHER_CTX_key_length(ctx),
key, data(ctx)->rounds);
}

#endif
3 changes: 3 additions & 0 deletions crypto/evp/evp_err.c
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,8 @@ static const ERR_STRING_DATA EVP_str_functs[] = {
{ERR_PACK(ERR_LIB_EVP, EVP_F_POLY1305_CTRL, 0), "poly1305_ctrl"},
{ERR_PACK(ERR_LIB_EVP, EVP_F_RC2_MAGIC_TO_METH, 0), "rc2_magic_to_meth"},
{ERR_PACK(ERR_LIB_EVP, EVP_F_RC5_CTRL, 0), "rc5_ctrl"},
{ERR_PACK(ERR_LIB_EVP, EVP_F_R_32_12_16_INIT_KEY, 0),
"r_32_12_16_init_key"},
{ERR_PACK(ERR_LIB_EVP, EVP_F_S390X_AES_GCM_CTRL, 0), "s390x_aes_gcm_ctrl"},
{ERR_PACK(ERR_LIB_EVP, EVP_F_S390X_AES_GCM_TLS_CIPHER, 0),
"s390x_aes_gcm_tls_cipher"},
Expand All @@ -199,6 +201,7 @@ static const ERR_STRING_DATA EVP_str_reasons[] = {
{ERR_PACK(ERR_LIB_EVP, 0, EVP_R_ARIA_KEY_SETUP_FAILED),
"aria key setup failed"},
{ERR_PACK(ERR_LIB_EVP, 0, EVP_R_BAD_DECRYPT), "bad decrypt"},
{ERR_PACK(ERR_LIB_EVP, 0, EVP_R_BAD_KEY_LENGTH), "bad key length"},
{ERR_PACK(ERR_LIB_EVP, 0, EVP_R_BUFFER_TOO_SMALL), "buffer too small"},
{ERR_PACK(ERR_LIB_EVP, 0, EVP_R_CAMELLIA_KEY_SETUP_FAILED),
"camellia key setup failed"},
Expand Down
9 changes: 7 additions & 2 deletions crypto/rc5/rc5_skey.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@
#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;

if (len > 255)
return 0;

if ((rounds != RC5_16_ROUNDS) &&
(rounds != RC5_12_ROUNDS) && (rounds != RC5_8_ROUNDS))
rounds = RC5_16_ROUNDS;
Expand Down Expand Up @@ -58,4 +61,6 @@ void RC5_32_set_key(RC5_32_KEY *key, int len, const unsigned char *data,
if (++jj >= c)
jj = 0;
}

return 1;
}
25 changes: 20 additions & 5 deletions doc/man3/EVP_rc5_32_12_16_cbc.pod
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,26 @@ EVP_rc5_32_12_16_ofb()

RC5 encryption algorithm in CBC, CFB, ECB and OFB modes respectively. This is a
variable key length cipher with an additional "number of rounds" parameter. By
default the key length is set to 128 bits and 12 rounds.
default the key length is set to 128 bits and 12 rounds. Alternative key lengths
can be set using L<EVP_CIPHER_CTX_set_key_length(3)>. The maximum key length is
2040 bits.

The following rc5 specific I<ctrl>s are supported (see
L<EVP_CIPHER_CTX_ctrl(3)>).

=over 4

=item EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_SET_RC5_ROUNDS, rounds, NULL)

Sets the number of rounds to B<rounds>. This must be one of RC5_8_ROUNDS,
RC5_12_ROUNDS or RC5_16_ROUNDS.

=item EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_GET_RC5_ROUNDS, 0, &rounds)

Stores the number of rounds currently configured in B<*rounds> where B<rounds>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't the second be *rounds too?
Alternatively, say it is a pointer to an int.

is an int.

=back

=back

Expand All @@ -43,10 +62,6 @@ These functions return an B<EVP_CIPHER> structure that contains the
implementation of the symmetric cipher. See L<EVP_CIPHER_meth_new(3)> for
details of the B<EVP_CIPHER> structure.

=head1 BUGS

Currently the number of rounds in RC5 can only be set to 8, 12 or 16.
This is a limitation of the current RC5 code rather than the EVP interface.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still seems to be the case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but I documented the restriction of 8, 12 or 16 in the section on the EVP_CTRL_SET_RC5_ROUNDS ctrl. I didn't see this as a "bug" so the writing it in a bugs section seemed misplaced. The fact that its a limitation of the rc5 code rather than the evp interface is an implementation detail that doesn't seem relevant to the end user.


=head1 SEE ALSO

Expand Down
2 changes: 2 additions & 0 deletions include/openssl/evperr.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ int ERR_load_EVP_strings(void);
# define EVP_F_POLY1305_CTRL 216
# define EVP_F_RC2_MAGIC_TO_METH 109
# define EVP_F_RC5_CTRL 125
# define EVP_F_R_32_12_16_INIT_KEY 242
# define EVP_F_S390X_AES_GCM_CTRL 201
# define EVP_F_S390X_AES_GCM_TLS_CIPHER 208
# define EVP_F_SCRYPT_ALG 228
Expand All @@ -162,6 +163,7 @@ int ERR_load_EVP_strings(void);
# define EVP_R_AES_KEY_SETUP_FAILED 143
# define EVP_R_ARIA_KEY_SETUP_FAILED 176
# define EVP_R_BAD_DECRYPT 100
# define EVP_R_BAD_KEY_LENGTH 195
# define EVP_R_BUFFER_TOO_SMALL 155
# define EVP_R_CAMELLIA_KEY_SETUP_FAILED 157
# define EVP_R_CIPHER_NOT_GCM_MODE 184
Expand Down
4 changes: 2 additions & 2 deletions include/openssl/rc5.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ typedef struct rc5_key_st {
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);
Expand Down
7 changes: 5 additions & 2 deletions test/rc5test.c
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,8 @@ static int test_rc5_ecb(int n)
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)))
Expand All @@ -203,7 +204,9 @@ static int test_rc5_cbc(int n)

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,
Expand Down