[openssl-commits] [openssl] OpenSSL_1_0_2-stable update
Matt Caswell
matt at openssl.org
Thu Apr 30 22:29:31 UTC 2015
The branch OpenSSL_1_0_2-stable has been updated
via f296e411efc2d3ebbf37bdc9c1111e84a5982ec6 (commit)
via 5bea7975a6b8b83cce938618a9fcaaa248c10712 (commit)
via 9c5efc9c65be2d29fd01c02bba081b72ba025453 (commit)
via 75862f7741d52651eefc2ae71e81a0d0e9d4c5ec (commit)
via 99ceb2d40c70fa8405151669578afb3be1d7c8c6 (commit)
via abc7a266a38b3122977bbf9049c61b6297343004 (commit)
via 33c99f2c8169807660b46d49c3e735cfa09a6e0c (commit)
via 1a3701f4fe0530a40ec073cd78d02cfcc26c0f8e (commit)
via 4ce06271aac5ebddf02854191611613af5ec83f8 (commit)
from c5f8cd7bc661f90dc012c9d2bae1808a4281985f (commit)
- Log -----------------------------------------------------------------
commit f296e411efc2d3ebbf37bdc9c1111e84a5982ec6
Author: Matt Caswell <matt at openssl.org>
Date: Wed Apr 29 13:22:18 2015 +0100
Fix buffer overrun in RSA signing
The problem occurs in EVP_PKEY_sign() when using RSA with X931 padding.
It is only triggered if the RSA key size is smaller than the digest length.
So with SHA512 you can trigger the overflow with anything less than an RSA
512 bit key. I managed to trigger a 62 byte overflow when using a 16 bit RSA
key. This wasn't sufficient to cause a crash, although your mileage may
vary.
In practice RSA keys of this length are never used and X931 padding is very
rare. Even if someone did use an excessively short RSA key, the chances of
them combining that with a longer digest and X931 padding is very
small. For these reasons I do not believe there is a security implication to
this. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro at openssl.org>
(cherry picked from commit 34166d41892643a36ad2d1f53cc0025e2edc2a39)
commit 5bea7975a6b8b83cce938618a9fcaaa248c10712
Author: Matt Caswell <matt at openssl.org>
Date: Wed Apr 29 09:58:10 2015 +0100
Add sanity check to print_bin function
Add a sanity check to the print_bin function to ensure that the |off|
argument is positive. Thanks to Kevin Wojtysiak (Int3 Solutions) and
Paramjot Oberoi (Int3 Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro at openssl.org>
(cherry picked from commit 3deeeeb61b0c5b9b5f0993a67b7967d2f85186da)
commit 9c5efc9c65be2d29fd01c02bba081b72ba025453
Author: Matt Caswell <matt at openssl.org>
Date: Tue Apr 28 15:28:23 2015 +0100
Add sanity check to ssl_get_prev_session
Sanity check the |len| parameter to ensure it is positive. Thanks to Kevin
Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for
reporting this issue.
Reviewed-by: Andy Polyakov <appro at openssl.org>
(cherry picked from commit cb0f400b0cea2d2943f99b1e89c04ff6ed748cd5)
commit 75862f7741d52651eefc2ae71e81a0d0e9d4c5ec
Author: Matt Caswell <matt at openssl.org>
Date: Tue Apr 28 15:19:50 2015 +0100
Sanity check the return from final_finish_mac
The return value is checked for 0. This is currently safe but we should
really check for <= 0 since -1 is frequently used for error conditions.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro at openssl.org>
(cherry picked from commit c427570e5098e120cbcb66e799f85c317aac7b91)
Conflicts:
ssl/ssl_locl.h
commit 99ceb2d40c70fa8405151669578afb3be1d7c8c6
Author: Matt Caswell <matt at openssl.org>
Date: Mon Apr 27 15:41:42 2015 +0100
Add sanity check in ssl3_cbc_digest_record
For SSLv3 the code assumes that |header_length| > |md_block_size|. Whilst
this is true for all SSLv3 ciphersuites, this fact is far from obvious by
looking at the code. If this were not the case then an integer overflow
would occur, leading to a subsequent buffer overflow. Therefore I have
added an explicit sanity check to ensure header_length is always valid.
Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3
Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro at openssl.org>
(cherry picked from commit 29b0a15a480626544dd0c803d5de671552544de6)
commit abc7a266a38b3122977bbf9049c61b6297343004
Author: Matt Caswell <matt at openssl.org>
Date: Mon Apr 27 15:41:03 2015 +0100
Clarify logic in BIO_*printf functions
The static function dynamically allocates an output buffer if the output
grows larger than the static buffer that is normally used. The original
logic implied that |currlen| could be greater than |maxlen| which is
incorrect (and if so would cause a buffer overrun). Also the original
logic would call OPENSSL_malloc to create a dynamic buffer equal to the
size of the static buffer, and then immediately call OPENSSL_realloc to
make it bigger, rather than just creating a buffer than was big enough in
the first place. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot
Oberoi (Int3 Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro at openssl.org>
(cherry picked from commit 9d9e37744cd5119f9921315864d1cd28717173cd)
commit 33c99f2c8169807660b46d49c3e735cfa09a6e0c
Author: Matt Caswell <matt at openssl.org>
Date: Mon Apr 27 11:13:56 2015 +0100
Sanity check EVP_EncodeUpdate buffer len
There was already a sanity check to ensure the passed buffer length is not
zero. Extend this to ensure that it also not negative. Thanks to Kevin
Wojtysiak (Int3 Solutions) and Paramjot Oberoi (Int3 Solutions) for
reporting this issue.
Reviewed-by: Andy Polyakov <appro at openssl.org>
(cherry picked from commit b86d7dca69f5c80abd60896c8ed3039fc56210cc)
commit 1a3701f4fe0530a40ec073cd78d02cfcc26c0f8e
Author: Matt Caswell <matt at openssl.org>
Date: Mon Apr 27 11:07:06 2015 +0100
Sanity check EVP_CTRL_AEAD_TLS_AAD
The various implementations of EVP_CTRL_AEAD_TLS_AAD expect a buffer of at
least 13 bytes long. Add sanity checks to ensure that the length is at
least that. Also add a new constant (EVP_AEAD_TLS1_AAD_LEN) to evp.h to
represent this length. Thanks to Kevin Wojtysiak (Int3 Solutions) and
Paramjot Oberoi (Int3 Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro at openssl.org>
(cherry picked from commit c8269881093324b881b81472be037055571f73f3)
Conflicts:
ssl/record/ssl3_record.c
commit 4ce06271aac5ebddf02854191611613af5ec83f8
Author: Matt Caswell <matt at openssl.org>
Date: Mon Apr 27 11:04:56 2015 +0100
Sanity check DES_enc_write buffer length
Add a sanity check to DES_enc_write to ensure the buffer length provided
is not negative. Thanks to Kevin Wojtysiak (Int3 Solutions) and Paramjot
Oberoi (Int3 Solutions) for reporting this issue.
Reviewed-by: Andy Polyakov <appro at openssl.org>
(cherry picked from commit 873fb39f20b6763daba226b74e83fb194924c7bf)
-----------------------------------------------------------------------
Summary of changes:
apps/speed.c | 5 +++--
crypto/bio/b_print.c | 45 ++++++++++++++++++--------------------
crypto/des/enc_writ.c | 3 +++
crypto/ec/eck_prn.c | 4 +++-
crypto/evp/e_aes.c | 2 +-
crypto/evp/e_aes_cbc_hmac_sha1.c | 9 +++++---
crypto/evp/e_aes_cbc_hmac_sha256.c | 7 ++++--
crypto/evp/e_rc4_hmac_md5.c | 7 +++++-
crypto/evp/encode.c | 2 +-
crypto/evp/evp.h | 3 +++
crypto/rsa/rsa_pmeth.c | 8 ++++++-
ssl/s3_both.c | 2 +-
ssl/s3_cbc.c | 14 ++++++++++--
ssl/ssl_locl.h | 1 -
ssl/ssl_sess.c | 2 +-
ssl/t1_enc.c | 7 ++++--
16 files changed, 78 insertions(+), 43 deletions(-)
diff --git a/apps/speed.c b/apps/speed.c
index 8c350ee..3697b71 100644
--- a/apps/speed.c
+++ b/apps/speed.c
@@ -2791,7 +2791,7 @@ static void multiblock_speed(const EVP_CIPHER *evp_cipher)
print_message(alg_name, 0, mblengths[j]);
Time_F(START);
for (count = 0, run = 1; run && count < 0x7fffffff; count++) {
- unsigned char aad[13];
+ unsigned char aad[EVP_AEAD_TLS1_AAD_LEN];
EVP_CTRL_TLS1_1_MULTIBLOCK_PARAM mb_param;
size_t len = mblengths[j];
int packlen;
@@ -2826,7 +2826,8 @@ static void multiblock_speed(const EVP_CIPHER *evp_cipher)
aad[11] = len >> 8;
aad[12] = len;
pad = EVP_CIPHER_CTX_ctrl(&ctx,
- EVP_CTRL_AEAD_TLS1_AAD, 13, aad);
+ EVP_CTRL_AEAD_TLS1_AAD,
+ EVP_AEAD_TLS1_AAD_LEN, aad);
EVP_Cipher(&ctx, out, inp, len + pad);
}
}
diff --git a/crypto/bio/b_print.c b/crypto/bio/b_print.c
index 452e5cf..7c81e25 100644
--- a/crypto/bio/b_print.c
+++ b/crypto/bio/b_print.c
@@ -704,32 +704,29 @@ doapr_outch(char **sbuffer,
/* If we haven't at least one buffer, someone has doe a big booboo */
assert(*sbuffer != NULL || buffer != NULL);
- if (buffer) {
- while (*currlen >= *maxlen) {
- if (*buffer == NULL) {
- if (*maxlen == 0)
- *maxlen = 1024;
- *buffer = OPENSSL_malloc(*maxlen);
- if (!*buffer) {
- /* Panic! Can't really do anything sensible. Just return */
- return;
- }
- if (*currlen > 0) {
- assert(*sbuffer != NULL);
- memcpy(*buffer, *sbuffer, *currlen);
- }
- *sbuffer = NULL;
- } else {
- *maxlen += 1024;
- *buffer = OPENSSL_realloc(*buffer, *maxlen);
- if (!*buffer) {
- /* Panic! Can't really do anything sensible. Just return */
- return;
- }
+ /* |currlen| must always be <= |*maxlen| */
+ assert(*currlen <= *maxlen);
+
+ if (buffer && *currlen == *maxlen) {
+ *maxlen += 1024;
+ if (*buffer == NULL) {
+ *buffer = OPENSSL_malloc(*maxlen);
+ if (!*buffer) {
+ /* Panic! Can't really do anything sensible. Just return */
+ return;
+ }
+ if (*currlen > 0) {
+ assert(*sbuffer != NULL);
+ memcpy(*buffer, *sbuffer, *currlen);
+ }
+ *sbuffer = NULL;
+ } else {
+ *buffer = OPENSSL_realloc(*buffer, *maxlen);
+ if (!*buffer) {
+ /* Panic! Can't really do anything sensible. Just return */
+ return;
}
}
- /* What to do if *buffer is NULL? */
- assert(*sbuffer != NULL || *buffer != NULL);
}
if (*currlen < *maxlen) {
diff --git a/crypto/des/enc_writ.c b/crypto/des/enc_writ.c
index 25041f2..bfaabde 100644
--- a/crypto/des/enc_writ.c
+++ b/crypto/des/enc_writ.c
@@ -96,6 +96,9 @@ int DES_enc_write(int fd, const void *_buf, int len,
const unsigned char *cp;
static int start = 1;
+ if (len < 0)
+ return -1;
+
if (outbuf == NULL) {
outbuf = OPENSSL_malloc(BSIZE + HDRSIZE);
if (outbuf == NULL)
diff --git a/crypto/ec/eck_prn.c b/crypto/ec/eck_prn.c
index 515b262..df9b37a 100644
--- a/crypto/ec/eck_prn.c
+++ b/crypto/ec/eck_prn.c
@@ -346,12 +346,14 @@ static int print_bin(BIO *fp, const char *name, const unsigned char *buf,
if (buf == NULL)
return 1;
- if (off) {
+ if (off > 0) {
if (off > 128)
off = 128;
memset(str, ' ', off);
if (BIO_write(fp, str, off) <= 0)
return 0;
+ } else {
+ off = 0;
}
if (BIO_printf(fp, "%s", name) <= 0)
diff --git a/crypto/evp/e_aes.c b/crypto/evp/e_aes.c
index 8161b26..af4aa18 100644
--- a/crypto/evp/e_aes.c
+++ b/crypto/evp/e_aes.c
@@ -1227,7 +1227,7 @@ static int aes_gcm_ctrl(EVP_CIPHER_CTX *c, int type, int arg, void *ptr)
case EVP_CTRL_AEAD_TLS1_AAD:
/* Save the AAD for later use */
- if (arg != 13)
+ if (arg != EVP_AEAD_TLS1_AAD_LEN)
return 0;
memcpy(c->buf, ptr, arg);
gctx->tls_aad_len = arg;
diff --git a/crypto/evp/e_aes_cbc_hmac_sha1.c b/crypto/evp/e_aes_cbc_hmac_sha1.c
index e0127a9..a277d0f 100644
--- a/crypto/evp/e_aes_cbc_hmac_sha1.c
+++ b/crypto/evp/e_aes_cbc_hmac_sha1.c
@@ -845,7 +845,12 @@ static int aesni_cbc_hmac_sha1_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg,
case EVP_CTRL_AEAD_TLS1_AAD:
{
unsigned char *p = ptr;
- unsigned int len = p[arg - 2] << 8 | p[arg - 1];
+ unsigned int len;
+
+ if (arg != EVP_AEAD_TLS1_AAD_LEN)
+ return -1;
+
+ len = p[arg - 2] << 8 | p[arg - 1];
if (ctx->encrypt) {
key->payload_length = len;
@@ -862,8 +867,6 @@ static int aesni_cbc_hmac_sha1_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg,
AES_BLOCK_SIZE) & -AES_BLOCK_SIZE)
- len);
} else {
- if (arg > 13)
- arg = 13;
memcpy(key->aux.tls_aad, ptr, arg);
key->payload_length = arg;
diff --git a/crypto/evp/e_aes_cbc_hmac_sha256.c b/crypto/evp/e_aes_cbc_hmac_sha256.c
index 30398c7..b74bd80 100644
--- a/crypto/evp/e_aes_cbc_hmac_sha256.c
+++ b/crypto/evp/e_aes_cbc_hmac_sha256.c
@@ -813,6 +813,11 @@ static int aesni_cbc_hmac_sha256_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg,
unsigned char *p = ptr;
unsigned int len = p[arg - 2] << 8 | p[arg - 1];
+ if (arg != EVP_AEAD_TLS1_AAD_LEN)
+ return -1;
+
+ len = p[arg - 2] << 8 | p[arg - 1];
+
if (ctx->encrypt) {
key->payload_length = len;
if ((key->aux.tls_ver =
@@ -828,8 +833,6 @@ static int aesni_cbc_hmac_sha256_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg,
AES_BLOCK_SIZE) & -AES_BLOCK_SIZE)
- len);
} else {
- if (arg > 13)
- arg = 13;
memcpy(key->aux.tls_aad, ptr, arg);
key->payload_length = arg;
diff --git a/crypto/evp/e_rc4_hmac_md5.c b/crypto/evp/e_rc4_hmac_md5.c
index 80735d3..e6b0cdf 100644
--- a/crypto/evp/e_rc4_hmac_md5.c
+++ b/crypto/evp/e_rc4_hmac_md5.c
@@ -258,7 +258,12 @@ static int rc4_hmac_md5_ctrl(EVP_CIPHER_CTX *ctx, int type, int arg,
case EVP_CTRL_AEAD_TLS1_AAD:
{
unsigned char *p = ptr;
- unsigned int len = p[arg - 2] << 8 | p[arg - 1];
+ unsigned int len;
+
+ if (arg != EVP_AEAD_TLS1_AAD_LEN)
+ return -1;
+
+ len = p[arg - 2] << 8 | p[arg - 1];
if (!ctx->encrypt) {
len -= MD5_DIGEST_LENGTH;
diff --git a/crypto/evp/encode.c b/crypto/evp/encode.c
index 53cc586..c361d1f 100644
--- a/crypto/evp/encode.c
+++ b/crypto/evp/encode.c
@@ -137,7 +137,7 @@ void EVP_EncodeUpdate(EVP_ENCODE_CTX *ctx, unsigned char *out, int *outl,
unsigned int total = 0;
*outl = 0;
- if (inl == 0)
+ if (inl <= 0)
return;
OPENSSL_assert(ctx->length <= (int)sizeof(ctx->enc_data));
if ((ctx->num + inl) < ctx->length) {
diff --git a/crypto/evp/evp.h b/crypto/evp/evp.h
index 47abbac..4891133 100644
--- a/crypto/evp/evp.h
+++ b/crypto/evp/evp.h
@@ -424,6 +424,9 @@ struct evp_cipher_st {
# define EVP_CTRL_TLS1_1_MULTIBLOCK_DECRYPT 0x1b
# define EVP_CTRL_TLS1_1_MULTIBLOCK_MAX_BUFSIZE 0x1c
+/* RFC 5246 defines additional data to be 13 bytes in length */
+# define EVP_AEAD_TLS1_AAD_LEN 13
+
typedef struct {
unsigned char *out;
const unsigned char *inp;
diff --git a/crypto/rsa/rsa_pmeth.c b/crypto/rsa/rsa_pmeth.c
index ddda0dd..2036355 100644
--- a/crypto/rsa/rsa_pmeth.c
+++ b/crypto/rsa/rsa_pmeth.c
@@ -254,8 +254,14 @@ static int pkey_rsa_sign(EVP_PKEY_CTX *ctx, unsigned char *sig,
return ret;
ret = sltmp;
} else if (rctx->pad_mode == RSA_X931_PADDING) {
- if (!setup_tbuf(rctx, ctx))
+ if ((size_t)EVP_PKEY_size(ctx->pkey) < tbslen + 1) {
+ RSAerr(RSA_F_PKEY_RSA_SIGN, RSA_R_KEY_SIZE_TOO_SMALL);
+ return -1;
+ }
+ if (!setup_tbuf(rctx, ctx)) {
+ RSAerr(RSA_F_PKEY_RSA_SIGN, ERR_R_MALLOC_FAILURE);
return -1;
+ }
memcpy(rctx->tbuf, tbs, tbslen);
rctx->tbuf[tbslen] = RSA_X931_hash_id(EVP_MD_type(rctx->md));
ret = RSA_private_encrypt(tbslen + 1, rctx->tbuf,
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index c92fd72..019e21c 100644
--- a/ssl/s3_both.c
+++ b/ssl/s3_both.c
@@ -168,7 +168,7 @@ int ssl3_send_finished(SSL *s, int a, int b, const char *sender, int slen)
i = s->method->ssl3_enc->final_finish_mac(s,
sender, slen,
s->s3->tmp.finish_md);
- if (i == 0)
+ if (i <= 0)
return 0;
s->s3->tmp.finish_md_len = i;
memcpy(p, s->s3->tmp.finish_md, i);
diff --git a/ssl/s3_cbc.c b/ssl/s3_cbc.c
index f31dc04..c43402d 100644
--- a/ssl/s3_cbc.c
+++ b/ssl/s3_cbc.c
@@ -639,12 +639,22 @@ void ssl3_cbc_digest_record(const EVP_MD_CTX *ctx,
if (k > 0) {
if (is_sslv3) {
+ unsigned overhang;
+
/*
* The SSLv3 header is larger than a single block. overhang is
* the number of bytes beyond a single block that the header
- * consumes: either 7 bytes (SHA1) or 11 bytes (MD5).
+ * consumes: either 7 bytes (SHA1) or 11 bytes (MD5). There are no
+ * ciphersuites in SSLv3 that are not SHA1 or MD5 based and
+ * therefore we can be confident that the header_length will be
+ * greater than |md_block_size|. However we add a sanity check just
+ * in case
*/
- unsigned overhang = header_length - md_block_size;
+ if (header_length <= md_block_size) {
+ /* Should never happen */
+ return;
+ }
+ overhang = header_length - md_block_size;
md_transform(md_state.c, header);
memcpy(first_block, header + md_block_size, overhang);
memcpy(first_block + overhang, data, md_block_size - overhang);
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 79b85b9..fb65fed 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -1230,7 +1230,6 @@ int dtls1_write_app_data_bytes(SSL *s, int type, const void *buf, int len);
int dtls1_write_bytes(SSL *s, int type, const void *buf, int len);
int dtls1_send_change_cipher_spec(SSL *s, int a, int b);
-int dtls1_send_finished(SSL *s, int a, int b, const char *sender, int slen);
int dtls1_read_failed(SSL *s, int code);
int dtls1_buffer_message(SSL *s, int ccs);
int dtls1_retransmit_message(SSL *s, unsigned short seq,
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index dce9088..8b9945b 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -449,7 +449,7 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len,
int r;
#endif
- if (len > SSL_MAX_SSL_SESSION_ID_LENGTH)
+ if (len < 0 || len > SSL_MAX_SSL_SESSION_ID_LENGTH)
goto err;
if (session_id + len > limit) {
diff --git a/ssl/t1_enc.c b/ssl/t1_enc.c
index 0563191..e2a8f86 100644
--- a/ssl/t1_enc.c
+++ b/ssl/t1_enc.c
@@ -803,7 +803,7 @@ int tls1_enc(SSL *s, int send)
bs = EVP_CIPHER_block_size(ds->cipher);
if (EVP_CIPHER_flags(ds->cipher) & EVP_CIPH_FLAG_AEAD_CIPHER) {
- unsigned char buf[13], *seq;
+ unsigned char buf[EVP_AEAD_TLS1_AAD_LEN], *seq;
seq = send ? s->s3->write_sequence : s->s3->read_sequence;
@@ -827,7 +827,10 @@ int tls1_enc(SSL *s, int send)
buf[10] = (unsigned char)(s->version);
buf[11] = rec->length >> 8;
buf[12] = rec->length & 0xff;
- pad = EVP_CIPHER_CTX_ctrl(ds, EVP_CTRL_AEAD_TLS1_AAD, 13, buf);
+ pad = EVP_CIPHER_CTX_ctrl(ds, EVP_CTRL_AEAD_TLS1_AAD,
+ EVP_AEAD_TLS1_AAD_LEN, buf);
+ if (pad <= 0)
+ return -1;
if (send) {
l += pad;
rec->length += pad;
More information about the openssl-commits
mailing list