[openssl-commits] [openssl] master update
Matt Caswell
matt at openssl.org
Thu Apr 30 22:29:20 UTC 2015
The branch master has been updated
via 34166d41892643a36ad2d1f53cc0025e2edc2a39 (commit)
via 3deeeeb61b0c5b9b5f0993a67b7967d2f85186da (commit)
via cb0f400b0cea2d2943f99b1e89c04ff6ed748cd5 (commit)
via c427570e5098e120cbcb66e799f85c317aac7b91 (commit)
via 29b0a15a480626544dd0c803d5de671552544de6 (commit)
via 9d9e37744cd5119f9921315864d1cd28717173cd (commit)
via b86d7dca69f5c80abd60896c8ed3039fc56210cc (commit)
via c8269881093324b881b81472be037055571f73f3 (commit)
via 873fb39f20b6763daba226b74e83fb194924c7bf (commit)
from 895cba195a0c8430dcc8d1aa22b75eccaaee8f49 (commit)
- Log -----------------------------------------------------------------
commit 34166d41892643a36ad2d1f53cc0025e2edc2a39
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>
commit 3deeeeb61b0c5b9b5f0993a67b7967d2f85186da
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>
commit cb0f400b0cea2d2943f99b1e89c04ff6ed748cd5
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>
commit c427570e5098e120cbcb66e799f85c317aac7b91
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>
commit 29b0a15a480626544dd0c803d5de671552544de6
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>
commit 9d9e37744cd5119f9921315864d1cd28717173cd
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>
commit b86d7dca69f5c80abd60896c8ed3039fc56210cc
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>
commit c8269881093324b881b81472be037055571f73f3
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>
commit 873fb39f20b6763daba226b74e83fb194924c7bf
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>
-----------------------------------------------------------------------
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/rsa/rsa_pmeth.c | 8 ++++++-
include/openssl/evp.h | 3 +++
ssl/record/ssl3_record.c | 7 ++++--
ssl/s3_both.c | 2 +-
ssl/s3_cbc.c | 14 ++++++++++--
ssl/ssl_locl.h | 1 -
ssl/ssl_sess.c | 2 +-
16 files changed, 78 insertions(+), 43 deletions(-)
diff --git a/apps/speed.c b/apps/speed.c
index 720ab1c..08ab9c5 100644
--- a/apps/speed.c
+++ b/apps/speed.c
@@ -2456,7 +2456,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;
@@ -2491,7 +2491,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 55cc7fc..9ea7c5a 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 e39aa71..e1f5b69 100644
--- a/crypto/ec/eck_prn.c
+++ b/crypto/ec/eck_prn.c
@@ -345,12 +345,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 7b4d84f..0b7838e 100644
--- a/crypto/evp/e_aes.c
+++ b/crypto/evp/e_aes.c
@@ -1327,7 +1327,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 960be3c..7f2848e 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 bea8f6d..3b6827a 100644
--- a/crypto/evp/e_aes_cbc_hmac_sha256.c
+++ b/crypto/evp/e_aes_cbc_hmac_sha256.c
@@ -817,6 +817,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 =
@@ -832,8 +837,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 7c4bd34..1ba690d 100644
--- a/crypto/evp/e_rc4_hmac_md5.c
+++ b/crypto/evp/e_rc4_hmac_md5.c
@@ -257,7 +257,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 682a914..053c1d8 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/rsa/rsa_pmeth.c b/crypto/rsa/rsa_pmeth.c
index 0aaca9e..91dc668 100644
--- a/crypto/rsa/rsa_pmeth.c
+++ b/crypto/rsa/rsa_pmeth.c
@@ -195,8 +195,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/include/openssl/evp.h b/include/openssl/evp.h
index 0d26fd3..4df3ce7 100644
--- a/include/openssl/evp.h
+++ b/include/openssl/evp.h
@@ -426,6 +426,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/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index cfd8290..33d0b30 100644
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -658,7 +658,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 ? RECORD_LAYER_get_write_sequence(&s->rlayer)
: RECORD_LAYER_get_read_sequence(&s->rlayer);
@@ -684,7 +684,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;
diff --git a/ssl/s3_both.c b/ssl/s3_both.c
index d0cb763..bf5e8c7 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 b20c564..ac0c5f3 100644
--- a/ssl/s3_cbc.c
+++ b/ssl/s3_cbc.c
@@ -397,12 +397,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 8b4c615..9ae1a07 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -2073,7 +2073,6 @@ void dtls1_set_message_header(SSL *s,
__owur int dtls1_write_app_data_bytes(SSL *s, int type, const void *buf, int len);
__owur int dtls1_send_change_cipher_spec(SSL *s, int a, int b);
-__owur int dtls1_send_finished(SSL *s, int a, int b, const char *sender, int slen);
__owur int dtls1_read_failed(SSL *s, int code);
__owur int dtls1_buffer_message(SSL *s, int ccs);
__owur int dtls1_retransmit_message(SSL *s, unsigned short seq,
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index cec5905..34b6fac 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -439,7 +439,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) {
More information about the openssl-commits
mailing list