Fix missing return value checks.

Fixed various missing return value checks in ssl3_send_newsession_ticket.
Also a mem leak on error.

Reviewed-by: Richard Levitte <levitte@openssl.org>
This commit is contained in:
Matt Caswell 2015-02-26 11:53:55 +00:00
parent 366448ec5e
commit 687eaf27a7

View File

@ -3256,14 +3256,16 @@ int ssl3_send_server_certificate(SSL *s)
/* send a new session ticket (not necessarily for a new session) */ /* send a new session ticket (not necessarily for a new session) */
int ssl3_send_newsession_ticket(SSL *s) int ssl3_send_newsession_ticket(SSL *s)
{ {
unsigned char *senc = NULL;
EVP_CIPHER_CTX ctx;
HMAC_CTX hctx;
if (s->state == SSL3_ST_SW_SESSION_TICKET_A) { if (s->state == SSL3_ST_SW_SESSION_TICKET_A) {
unsigned char *p, *senc, *macstart; unsigned char *p, *macstart;
const unsigned char *const_p; const unsigned char *const_p;
int len, slen_full, slen; int len, slen_full, slen;
SSL_SESSION *sess; SSL_SESSION *sess;
unsigned int hlen; unsigned int hlen;
EVP_CIPHER_CTX ctx;
HMAC_CTX hctx;
SSL_CTX *tctx = s->initial_ctx; SSL_CTX *tctx = s->initial_ctx;
unsigned char iv[EVP_MAX_IV_LENGTH]; unsigned char iv[EVP_MAX_IV_LENGTH];
unsigned char key_name[16]; unsigned char key_name[16];
@ -3274,32 +3276,38 @@ int ssl3_send_newsession_ticket(SSL *s)
* Some length values are 16 bits, so forget it if session is too * Some length values are 16 bits, so forget it if session is too
* long * long
*/ */
if (slen_full > 0xFF00) if (slen_full == 0 || slen_full > 0xFF00)
return -1; return -1;
senc = OPENSSL_malloc(slen_full); senc = OPENSSL_malloc(slen_full);
if (!senc) if (!senc)
return -1; return -1;
EVP_CIPHER_CTX_init(&ctx);
HMAC_CTX_init(&hctx);
p = senc; p = senc;
i2d_SSL_SESSION(s->session, &p); if (!i2d_SSL_SESSION(s->session, &p))
goto err;
/* /*
* create a fresh copy (not shared with other threads) to clean up * create a fresh copy (not shared with other threads) to clean up
*/ */
const_p = senc; const_p = senc;
sess = d2i_SSL_SESSION(NULL, &const_p, slen_full); sess = d2i_SSL_SESSION(NULL, &const_p, slen_full);
if (sess == NULL) { if (sess == NULL)
OPENSSL_free(senc); goto err;
return -1;
}
sess->session_id_length = 0; /* ID is irrelevant for the ticket */ sess->session_id_length = 0; /* ID is irrelevant for the ticket */
slen = i2d_SSL_SESSION(sess, NULL); slen = i2d_SSL_SESSION(sess, NULL);
if (slen > slen_full) { /* shouldn't ever happen */ if (slen == 0 || slen > slen_full) { /* shouldn't ever happen */
OPENSSL_free(senc); SSL_SESSION_free(sess);
return -1; goto err;
} }
p = senc; p = senc;
i2d_SSL_SESSION(sess, &p); if (!i2d_SSL_SESSION(sess, &p)) {
SSL_SESSION_free(sess);
goto err;
}
SSL_SESSION_free(sess); SSL_SESSION_free(sess);
/*- /*-
@ -3313,26 +3321,26 @@ int ssl3_send_newsession_ticket(SSL *s)
if (!BUF_MEM_grow(s->init_buf, if (!BUF_MEM_grow(s->init_buf,
SSL_HM_HEADER_LENGTH(s) + 22 + EVP_MAX_IV_LENGTH + SSL_HM_HEADER_LENGTH(s) + 22 + EVP_MAX_IV_LENGTH +
EVP_MAX_BLOCK_LENGTH + EVP_MAX_MD_SIZE + slen)) EVP_MAX_BLOCK_LENGTH + EVP_MAX_MD_SIZE + slen))
return -1; goto err;
p = ssl_handshake_start(s); p = ssl_handshake_start(s);
EVP_CIPHER_CTX_init(&ctx);
HMAC_CTX_init(&hctx);
/* /*
* Initialize HMAC and cipher contexts. If callback present it does * Initialize HMAC and cipher contexts. If callback present it does
* all the work otherwise use generated values from parent ctx. * all the work otherwise use generated values from parent ctx.
*/ */
if (tctx->tlsext_ticket_key_cb) { if (tctx->tlsext_ticket_key_cb) {
if (tctx->tlsext_ticket_key_cb(s, key_name, iv, &ctx, if (tctx->tlsext_ticket_key_cb(s, key_name, iv, &ctx,
&hctx, 1) < 0) { &hctx, 1) < 0)
OPENSSL_free(senc); goto err;
return -1;
}
} else { } else {
RAND_pseudo_bytes(iv, 16); if (RAND_bytes(iv, 16) <= 0)
EVP_EncryptInit_ex(&ctx, EVP_aes_128_cbc(), NULL, goto err;
tctx->tlsext_tick_aes_key, iv); if (!EVP_EncryptInit_ex(&ctx, EVP_aes_128_cbc(), NULL,
HMAC_Init_ex(&hctx, tctx->tlsext_tick_hmac_key, 16, tctx->tlsext_tick_aes_key, iv))
EVP_sha256(), NULL); goto err;
if (!HMAC_Init_ex(&hctx, tctx->tlsext_tick_hmac_key, 16,
EVP_sha256(), NULL))
goto err;
memcpy(key_name, tctx->tlsext_tick_key_name, 16); memcpy(key_name, tctx->tlsext_tick_key_name, 16);
} }
@ -3353,14 +3361,19 @@ int ssl3_send_newsession_ticket(SSL *s)
memcpy(p, iv, EVP_CIPHER_CTX_iv_length(&ctx)); memcpy(p, iv, EVP_CIPHER_CTX_iv_length(&ctx));
p += EVP_CIPHER_CTX_iv_length(&ctx); p += EVP_CIPHER_CTX_iv_length(&ctx);
/* Encrypt session data */ /* Encrypt session data */
EVP_EncryptUpdate(&ctx, p, &len, senc, slen); if (!EVP_EncryptUpdate(&ctx, p, &len, senc, slen))
goto err;
p += len; p += len;
EVP_EncryptFinal(&ctx, p, &len); if (!EVP_EncryptFinal(&ctx, p, &len))
goto err;
p += len; p += len;
EVP_CIPHER_CTX_cleanup(&ctx);
HMAC_Update(&hctx, macstart, p - macstart); if (!HMAC_Update(&hctx, macstart, p - macstart))
HMAC_Final(&hctx, p, &hlen); goto err;
if (!HMAC_Final(&hctx, p, &hlen))
goto err;
EVP_CIPHER_CTX_cleanup(&ctx);
HMAC_CTX_cleanup(&hctx); HMAC_CTX_cleanup(&hctx);
p += hlen; p += hlen;
@ -3377,6 +3390,12 @@ int ssl3_send_newsession_ticket(SSL *s)
/* SSL3_ST_SW_SESSION_TICKET_B */ /* SSL3_ST_SW_SESSION_TICKET_B */
return ssl_do_write(s); return ssl_do_write(s);
err:
if (senc)
OPENSSL_free(senc);
EVP_CIPHER_CTX_cleanup(&ctx);
HMAC_CTX_cleanup(&hctx);
return -1;
} }
int ssl3_send_cert_status(SSL *s) int ssl3_send_cert_status(SSL *s)