summaryrefslogtreecommitdiff
path: root/ssl
diff options
context:
space:
mode:
authorMatt Caswell <matt@openssl.org>2020-06-03 17:42:01 +0100
committerMatt Caswell <matt@openssl.org>2020-07-06 09:26:00 +0100
commitec27e619e86c6ce4dfa905044eb4737eeba28a9d (patch)
tree463fa1af1ce6d48b1c20f62c06fbacfbed92b68b /ssl
parent1b726e9b91a032298dc96ad117b23e18e1583246 (diff)
downloadopenssl-new-ec27e619e86c6ce4dfa905044eb4737eeba28a9d.tar.gz
Move MAC removal responsibility to the various protocol "enc" functions
For CBC ciphersuites using Mac-then-encrypt we have to be careful about removing the MAC from the record in constant time. Currently that happens immediately before MAC verification. Instead we move this responsibility to the various protocol "enc" functions so that MAC removal is handled at the same time as padding removal. Reviewed-by: Shane Lontis <shane.lontis@oracle.com> (Merged from https://github.com/openssl/openssl/pull/12288)
Diffstat (limited to 'ssl')
-rw-r--r--ssl/record/rec_layer_d1.c2
-rw-r--r--ssl/record/rec_layer_s3.c5
-rw-r--r--ssl/record/record.h15
-rw-r--r--ssl/record/record_local.h17
-rw-r--r--ssl/record/ssl3_record.c668
-rw-r--r--ssl/record/ssl3_record_tls13.c41
-rw-r--r--ssl/ssl_lib.c25
-rw-r--r--ssl/ssl_local.h2
8 files changed, 375 insertions, 400 deletions
diff --git a/ssl/record/rec_layer_d1.c b/ssl/record/rec_layer_d1.c
index 9a82e3ffa2..866ef18381 100644
--- a/ssl/record/rec_layer_d1.c
+++ b/ssl/record/rec_layer_d1.c
@@ -939,7 +939,7 @@ int do_dtls1_write(SSL *s, int type, const unsigned char *buf,
if (eivlen)
SSL3_RECORD_add_length(&wr, eivlen);
- if (s->method->ssl3_enc->enc(s, &wr, 1, 1) < 1) {
+ if (s->method->ssl3_enc->enc(s, &wr, 1, 1, NULL, mac_size) < 1) {
if (!ossl_statem_in_error(s)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DO_DTLS1_WRITE,
ERR_R_INTERNAL_ERROR);
diff --git a/ssl/record/rec_layer_s3.c b/ssl/record/rec_layer_s3.c
index fac3506b19..8ea16672b6 100644
--- a/ssl/record/rec_layer_s3.c
+++ b/ssl/record/rec_layer_s3.c
@@ -1044,7 +1044,7 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
* We haven't actually negotiated the version yet, but we're trying to
* send early data - so we need to use the tls13enc function.
*/
- if (tls13_enc(s, wr, numpipes, 1) < 1) {
+ if (tls13_enc(s, wr, numpipes, 1, NULL, mac_size) < 1) {
if (!ossl_statem_in_error(s)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DO_SSL3_WRITE,
ERR_R_INTERNAL_ERROR);
@@ -1053,7 +1053,8 @@ int do_ssl3_write(SSL *s, int type, const unsigned char *buf,
}
} else {
if (!BIO_get_ktls_send(s->wbio)) {
- if (s->method->ssl3_enc->enc(s, wr, numpipes, 1) < 1) {
+ if (s->method->ssl3_enc->enc(s, wr, numpipes, 1, NULL,
+ mac_size) < 1) {
if (!ossl_statem_in_error(s)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DO_SSL3_WRITE,
ERR_R_INTERNAL_ERROR);
diff --git a/ssl/record/record.h b/ssl/record/record.h
index 0504a6f959..234656bf93 100644
--- a/ssl/record/record.h
+++ b/ssl/record/record.h
@@ -178,6 +178,12 @@ typedef struct record_layer_st {
* *
*****************************************************************************/
+struct ssl_mac_buf_st {
+ unsigned char *mac;
+ int alloced;
+};
+typedef struct ssl_mac_buf_st SSL_MAC_BUF;
+
#define MIN_SSL2_RECORD_LEN 9
#define RECORD_LAYER_set_read_ahead(rl, ra) ((rl)->read_ahead = (ra))
@@ -213,13 +219,16 @@ __owur int ssl3_read_bytes(SSL *s, int type, int *recvd_type,
unsigned char *buf, size_t len, int peek,
size_t *readbytes);
__owur int ssl3_setup_buffers(SSL *s);
-__owur int ssl3_enc(SSL *s, SSL3_RECORD *inrecs, size_t n_recs, int send);
+__owur int ssl3_enc(SSL *s, SSL3_RECORD *inrecs, size_t n_recs, int send,
+ SSL_MAC_BUF *mac, size_t macsize);
__owur int n_ssl3_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int send);
__owur int ssl3_write_pending(SSL *s, int type, const unsigned char *buf, size_t len,
size_t *written);
-__owur int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int send);
+__owur int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending,
+ SSL_MAC_BUF *mac, size_t macsize);
__owur int tls1_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int send);
-__owur int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int send);
+__owur int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int send,
+ SSL_MAC_BUF *mac, size_t macsize);
int DTLS_RECORD_LAYER_new(RECORD_LAYER *rl);
void DTLS_RECORD_LAYER_free(RECORD_LAYER *rl);
void DTLS_RECORD_LAYER_clear(RECORD_LAYER *rl);
diff --git a/ssl/record/record_local.h b/ssl/record/record_local.h
index f7734d832b..dc92732243 100644
--- a/ssl/record/record_local.h
+++ b/ssl/record/record_local.h
@@ -107,13 +107,16 @@ void SSL3_RECORD_set_seq_num(SSL3_RECORD *r, const unsigned char *seq_num);
int ssl3_get_record(SSL *s);
__owur int ssl3_do_compress(SSL *ssl, SSL3_RECORD *wr);
__owur int ssl3_do_uncompress(SSL *ssl, SSL3_RECORD *rr);
-int ssl3_cbc_copy_mac(unsigned char *out,
- const SSL3_RECORD *rec, size_t md_size);
-__owur int ssl3_cbc_remove_padding(SSL3_RECORD *rec,
- size_t block_size, size_t mac_size);
-__owur int tls1_cbc_remove_padding(const SSL *s,
- SSL3_RECORD *rec,
- size_t block_size, size_t mac_size);
+__owur int ssl3_cbc_remove_padding_and_mac(SSL *s,
+ SSL3_RECORD *rec,
+ unsigned char **mac,
+ int *alloced,
+ size_t block_size, size_t mac_size);
+__owur int tls1_cbc_remove_padding_and_mac(const SSL *s,
+ SSL3_RECORD *rec,
+ unsigned char **mac,
+ int *alloced,
+ size_t block_size, size_t mac_size);
int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap);
__owur int dtls1_get_record(SSL *s);
int early_data_count_ok(SSL *s, size_t length, size_t overhead, int send);
diff --git a/ssl/record/ssl3_record.c b/ssl/record/ssl3_record.c
index a2f7f848d1..3b1007f574 100644
--- a/ssl/record/ssl3_record.c
+++ b/ssl/record/ssl3_record.c
@@ -32,6 +32,14 @@ static const unsigned char ssl3_pad_2[48] = {
0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c, 0x5c
};
+static int ssl3_cbc_copy_mac(const SSL *s,
+ SSL3_RECORD *rec,
+ unsigned char **mac,
+ int *alloced,
+ size_t block_size,
+ size_t mac_size,
+ size_t good);
+
/*
* Clear the contents of an SSL3_RECORD but retain any memory allocated
*/
@@ -182,12 +190,13 @@ int ssl3_get_record(SSL *s)
unsigned char *p;
unsigned char md[EVP_MAX_MD_SIZE];
unsigned int version;
- size_t mac_size;
+ size_t mac_size = 0;
int imac_size;
size_t num_recs = 0, max_recs, j;
PACKET pkt, sslv2pkt;
- size_t first_rec_len;
int is_ktls_left;
+ SSL_MAC_BUF *macbufs = NULL;
+ int ret = -1;
rr = RECORD_LAYER_get_rrec(&s->rlayer);
rbuf = RECORD_LAYER_get_rbuf(&s->rlayer);
@@ -526,20 +535,28 @@ int ssl3_get_record(SSL *s)
if (BIO_get_ktls_recv(s->rbio) && !is_ktls_left)
goto skip_decryption;
+ /* TODO(size_t): convert this to do size_t properly */
+ if (s->read_hash != NULL) {
+ const EVP_MD *tmpmd = EVP_MD_CTX_md(s->read_hash);
+
+ if (tmpmd != NULL) {
+ imac_size = EVP_MD_size(tmpmd);
+ if (!ossl_assert(imac_size >= 0 && imac_size <= EVP_MAX_MD_SIZE)) {
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GET_RECORD,
+ ERR_LIB_EVP);
+ return -1;
+ }
+ mac_size = (size_t)imac_size;
+ }
+ }
+
/*
* If in encrypt-then-mac mode calculate mac from encrypted record. All
* the details below are public so no timing details can leak.
*/
if (SSL_READ_ETM(s) && s->read_hash) {
unsigned char *mac;
- /* TODO(size_t): convert this to do size_t properly */
- imac_size = EVP_MD_CTX_size(s->read_hash);
- if (!ossl_assert(imac_size >= 0 && imac_size <= EVP_MAX_MD_SIZE)) {
- SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GET_RECORD,
- ERR_LIB_EVP);
- return -1;
- }
- mac_size = (size_t)imac_size;
+
for (j = 0; j < num_recs; j++) {
thisrr = &rr[j];
@@ -557,27 +574,39 @@ int ssl3_get_record(SSL *s)
return -1;
}
}
+ /*
+ * We've handled the mac now - there is no MAC inside the encrypted
+ * record
+ */
+ mac_size = 0;
}
- first_rec_len = rr[0].length;
+ if (mac_size > 0) {
+ macbufs = OPENSSL_zalloc(sizeof(*macbufs) * num_recs);
+ if (macbufs == NULL) {
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GET_RECORD,
+ ERR_R_MALLOC_FAILURE);
+ return -1;
+ }
+ }
- enc_err = s->method->ssl3_enc->enc(s, rr, num_recs, 0);
+ enc_err = s->method->ssl3_enc->enc(s, rr, num_recs, 0, macbufs, mac_size);
/*-
* enc_err is:
- * 0: (in non-constant time) if the record is publicly invalid.
- * 1: if the padding is valid
- * -1: if the padding is invalid
+ * 0: if the record is publicly invalid, or an internal error, or AEAD
+ * decryption failed, or ETM decryption failed.
+ * 1: Success or MTE decryption failed (MAC will be randomised)
*/
if (enc_err == 0) {
if (ossl_statem_in_error(s)) {
/* SSLfatal() already got called */
- return -1;
+ goto end;
}
if (num_recs == 1 && ossl_statem_skip_early_data(s)) {
/*
- * Valid early_data that we cannot decrypt might fail here as
- * publicly invalid. We treat it like an empty record.
+ * Valid early_data that we cannot decrypt will fail here. We treat
+ * it like an empty record.
*/
thisrr = &rr[0];
@@ -585,18 +614,19 @@ int ssl3_get_record(SSL *s)
if (!early_data_count_ok(s, thisrr->length,
EARLY_DATA_CIPHERTEXT_OVERHEAD, 0)) {
/* SSLfatal() already called */
- return -1;
+ goto end;
}
thisrr->length = 0;
thisrr->read = 1;
RECORD_LAYER_set_numrpipes(&s->rlayer, 1);
RECORD_LAYER_reset_read_sequence(&s->rlayer);
- return 1;
+ ret = 1;
+ goto end;
}
SSLfatal(s, SSL_AD_BAD_RECORD_MAC, SSL_F_SSL3_GET_RECORD,
- SSL_R_BLOCK_CIPHER_PAD_IS_WRONG);
- return -1;
+ SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC);
+ goto end;
}
OSSL_TRACE_BEGIN(TLS) {
BIO_printf(trc_out, "dec %lu\n", (unsigned long)rr[0].length);
@@ -608,93 +638,24 @@ int ssl3_get_record(SSL *s)
(s->enc_read_ctx != NULL) &&
(!SSL_READ_ETM(s) && EVP_MD_CTX_md(s->read_hash) != NULL)) {
/* s->read_hash != NULL => mac_size != -1 */
- unsigned char *mac = NULL;
- unsigned char mac_tmp[EVP_MAX_MD_SIZE];
-
- mac_size = EVP_MD_CTX_size(s->read_hash);
- if (!ossl_assert(mac_size <= EVP_MAX_MD_SIZE)) {
- SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GET_RECORD,
- ERR_R_INTERNAL_ERROR);
- return -1;
- }
for (j = 0; j < num_recs; j++) {
+ SSL_MAC_BUF *thismb = &macbufs[j];
thisrr = &rr[j];
- /*
- * orig_len is the length of the record before any padding was
- * removed. This is public information, as is the MAC in use,
- * therefore we can safely process the record in a different amount
- * of time if it's too short to possibly contain a MAC.
- */
- if (thisrr->orig_len < mac_size ||
- /* CBC records must have a padding length byte too. */
- (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE &&
- thisrr->orig_len < mac_size + 1)) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_SSL3_GET_RECORD,
- SSL_R_LENGTH_TOO_SHORT);
- return -1;
- }
-
- if (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE) {
- /*
- * We update the length so that the TLS header bytes can be
- * constructed correctly but we need to extract the MAC in
- * constant time from within the record, without leaking the
- * contents of the padding bytes.
- */
- mac = mac_tmp;
- if (!ssl3_cbc_copy_mac(mac_tmp, thisrr, mac_size)) {
- SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GET_RECORD,
- ERR_R_INTERNAL_ERROR);
- return -1;
- }
- thisrr->length -= mac_size;
- } else {
- /*
- * In this case there's no padding, so |rec->orig_len| equals
- * |rec->length| and we checked that there's enough bytes for
- * |mac_size| above.
- */
- thisrr->length -= mac_size;
- mac = &thisrr->data[thisrr->length];
- }
i = s->method->ssl3_enc->mac(s, thisrr, md, 0 /* not send */ );
- if (i == 0 || mac == NULL
- || CRYPTO_memcmp(md, mac, (size_t)mac_size) != 0)
- enc_err = -1;
+ if (i == 0 || thismb == NULL || thismb->mac == NULL
+ || CRYPTO_memcmp(md, thismb->mac, (size_t)mac_size) != 0)
+ enc_err = 0;
if (thisrr->length > SSL3_RT_MAX_COMPRESSED_LENGTH + mac_size)
- enc_err = -1;
+ enc_err = 0;
}
}
- if (enc_err < 0) {
+ if (enc_err == 0) {
if (ossl_statem_in_error(s)) {
/* We already called SSLfatal() */
- return -1;
- }
- if (num_recs == 1 && ossl_statem_skip_early_data(s)) {
- /*
- * We assume this is unreadable early_data - we treat it like an
- * empty record
- */
-
- /*
- * The record length may have been modified by the mac check above
- * so we use the previously saved value
- */
- if (!early_data_count_ok(s, first_rec_len,
- EARLY_DATA_CIPHERTEXT_OVERHEAD, 0)) {
- /* SSLfatal() already called */
- return -1;
- }
-
- thisrr = &rr[0];
- thisrr->length = 0;
- thisrr->read = 1;
- RECORD_LAYER_set_numrpipes(&s->rlayer, 1);
- RECORD_LAYER_reset_read_sequence(&s->rlayer);
- return 1;
+ goto end;
}
/*
* A separate 'decryption_failed' alert was introduced with TLS 1.0,
@@ -705,7 +666,7 @@ int ssl3_get_record(SSL *s)
*/
SSLfatal(s, SSL_AD_BAD_RECORD_MAC, SSL_F_SSL3_GET_RECORD,
SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC);
- return -1;
+ goto end;
}
skip_decryption:
@@ -718,12 +679,12 @@ int ssl3_get_record(SSL *s)
if (thisrr->length > SSL3_RT_MAX_COMPRESSED_LENGTH) {
SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_SSL3_GET_RECORD,
SSL_R_COMPRESSED_LENGTH_TOO_LONG);
- return -1;
+ goto end;
}
if (!ssl3_do_uncompress(s, thisrr)) {
SSLfatal(s, SSL_AD_DECOMPRESSION_FAILURE, SSL_F_SSL3_GET_RECORD,
SSL_R_BAD_DECOMPRESSION);
- return -1;
+ goto end;
}
}
@@ -736,7 +697,7 @@ int ssl3_get_record(SSL *s)
|| thisrr->type != SSL3_RT_APPLICATION_DATA) {
SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_SSL3_GET_RECORD,
SSL_R_BAD_RECORD_TYPE);
- return -1;
+ goto end;
}
/* Strip trailing padding */
@@ -751,7 +712,7 @@ int ssl3_get_record(SSL *s)
&& thisrr->type != SSL3_RT_HANDSHAKE) {
SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_SSL3_GET_RECORD,
SSL_R_BAD_RECORD_TYPE);
- return -1;
+ goto end;
}
if (s->msg_callback)
s->msg_callback(0, s->version, SSL3_RT_INNER_CONTENT_TYPE,
@@ -768,13 +729,13 @@ int ssl3_get_record(SSL *s)
&& thisrr->length == 0) {
SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_SSL3_GET_RECORD,
SSL_R_BAD_LENGTH);
- return -1;
+ goto end;
}
if (thisrr->length > SSL3_RT_MAX_PLAIN_LENGTH && !BIO_get_ktls_recv(s->rbio)) {
SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_SSL3_GET_RECORD,
SSL_R_DATA_LENGTH_TOO_LONG);
- return -1;
+ goto end;
}
/* If received packet overflows current Max Fragment Length setting */
@@ -783,7 +744,7 @@ int ssl3_get_record(SSL *s)
&& !BIO_get_ktls_recv(s->rbio)) {
SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_SSL3_GET_RECORD,
SSL_R_DATA_LENGTH_TOO_LONG);
- return -1;
+ goto end;
}
thisrr->off = 0;
@@ -802,7 +763,7 @@ int ssl3_get_record(SSL *s)
> MAX_EMPTY_RECORDS) {
SSLfatal(s, SSL_AD_UNEXPECTED_MESSAGE, SSL_F_SSL3_GET_RECORD,
SSL_R_RECORD_TOO_SMALL);
- return -1;
+ goto end;
}
} else {
RECORD_LAYER_reset_empty_record_count(&s->rlayer);
@@ -814,12 +775,21 @@ int ssl3_get_record(SSL *s)
if (thisrr->type == SSL3_RT_APPLICATION_DATA
&& !early_data_count_ok(s, thisrr->length, 0, 0)) {
/* SSLfatal already called */
- return -1;
+ goto end;
}
}
RECORD_LAYER_set_numrpipes(&s->rlayer, num_recs);
- return 1;
+ ret = 1;
+ end:
+ if (macbufs != NULL) {
+ for (j = 0; j < num_recs; j++) {
+ if (macbufs[j].alloced)
+ OPENSSL_free(macbufs[j].mac);
+ }
+ OPENSSL_free(macbufs);
+ }
+ return ret;
}
int ssl3_do_uncompress(SSL *ssl, SSL3_RECORD *rr)
@@ -866,23 +836,21 @@ int ssl3_do_compress(SSL *ssl, SSL3_RECORD *wr)
}
/*-
- * ssl3_enc encrypts/decrypts |n_recs| records in |inrecs|. Will call
- * SSLfatal() for internal errors, but not otherwise.
+ * ssl3_enc encrypts/decrypts |n_recs| records in |inrecs|. Calls SSLfatal on
+ * internal error, but not otherwise. It is the responsibility of the caller to
+ * report a bad_record_mac
*
* Returns:
- * 0: (in non-constant time) if the record is publicly invalid (i.e. too
- * short etc).
- * 1: if the record's padding is valid / the encryption was successful.
- * -1: if the record's padding is invalid or, if sending, an internal error
- * occurred.
+ * 0: if the record is publicly invalid, or an internal error
+ * 1: Success or Mac-then-encrypt decryption failed (MAC will be randomised)
*/
-int ssl3_enc(SSL *s, SSL3_RECORD *inrecs, size_t n_recs, int sending)
+int ssl3_enc(SSL *s, SSL3_RECORD *inrecs, size_t n_recs, int sending,
+ SSL_MAC_BUF *mac, size_t macsize)
{
SSL3_RECORD *rec;
EVP_CIPHER_CTX *ds;
size_t l, i;
- size_t bs, mac_size = 0;
- int imac_size;
+ size_t bs;
const EVP_CIPHER *enc;
rec = inrecs;
@@ -930,52 +898,50 @@ int ssl3_enc(SSL *s, SSL3_RECORD *inrecs, size_t n_recs, int sending)
}
if (!sending) {
- if (l == 0 || l % bs != 0)
+ if (l == 0 || l % bs != 0) {
+ /* Publicly invalid */
return 0;
+ }
/* otherwise, rec->length >= bs */
}
/* TODO(size_t): Convert this call */
- if (EVP_Cipher(ds, rec->data, rec->input, (unsigned int)l) < 1)
- return -1;
-
- if (EVP_MD_CTX_md(s->read_hash) != NULL) {
- /* TODO(size_t): convert me */
- imac_size = EVP_MD_CTX_size(s->read_hash);
- if (imac_size < 0) {
- SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_ENC,
- ERR_R_INTERNAL_ERROR);
- return -1;
- }
- mac_size = (size_t)imac_size;
+ if (EVP_Cipher(ds, rec->data, rec->input, (unsigned int)l) < 1) {
+ /* Shouldn't happen */
+ SSLfatal(s, SSL_AD_BAD_RECORD_MAC, 0, ERR_R_INTERNAL_ERROR);
+ return 0;
}
- if ((bs != 1) && !sending)
- return ssl3_cbc_remove_padding(rec, bs, mac_size);
+
+ if (!sending)
+ return !ssl3_cbc_remove_padding_and_mac(s, rec,
+ (mac != NULL) ? &mac->mac : NULL,
+ (mac != NULL) ? &mac->alloced : NULL,
+ bs,
+ macsize);
}
return 1;
}
#define MAX_PADDING 256
/*-
- * tls1_enc encrypts/decrypts |n_recs| in |recs|. Will call SSLfatal() for
- * internal errors, but not otherwise.
+ * tls1_enc encrypts/decrypts |n_recs| in |recs|. Calls SSLfatal on internal
+ * error, but not otherwise. It is the responsibility of the caller to report
+ * a bad_record_mac - if appropriate (DTLS just drops the record).
*
* Returns:
- * 0: (in non-constant time) if the record is publicly invalid (i.e. too
- * short etc).
- * 1: if the record's padding is valid / the encryption was successful.
- * -1: if the record's padding/AEAD-authenticator is invalid or, if sending,
- * an internal error occurred.
+ * 0: if the record is publicly invalid, or an internal error, or AEAD
+ * decryption failed, or Encrypt-then-mac decryption failed.
+ * 1: Success or Mac-then-encrypt decryption failed (MAC will be randomised)
*/
-int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
+int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending,
+ SSL_MAC_BUF *macs, size_t macsize)
{
EVP_CIPHER_CTX *ds;
size_t reclen[SSL_MAX_PIPELINES];
unsigned char buf[SSL_MAX_PIPELINES][EVP_AEAD_TLS1_AAD_LEN];
- int i, pad = 0, ret, tmpr;
- size_t bs, mac_size = 0, ctr, padnum, loop;
+ int i, pad = 0, tmpr;
+ size_t bs, ctr, padnum, loop;
unsigned char padval;
- int imac_size;
const EVP_CIPHER *enc;
int tlstree_enc = sending ? (s->mac_flags & SSL_MAC_FLAG_WRITE_MAC_TLSTREE)
: (s->mac_flags & SSL_MAC_FLAG_READ_MAC_TLSTREE);
@@ -992,7 +958,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
if (!ossl_assert(n >= 0)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
}
ds = s->enc_write_ctx;
@@ -1016,12 +982,12 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
*/
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
} else if (RAND_bytes_ex(s->ctx->libctx, recs[ctr].input,
ivlen) <= 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
}
}
@@ -1032,7 +998,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
if (!ossl_assert(n >= 0)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
}
ds = s->enc_read_ctx;
@@ -1047,7 +1013,6 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
memmove(recs[ctr].data, recs[ctr].input, recs[ctr].length);
recs[ctr].input = recs[ctr].data;
}
- ret = 1;
} else {
bs = EVP_CIPHER_block_size(EVP_CIPHER_CTX_cipher(ds));
@@ -1060,7 +1025,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
*/
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
SSL_R_PIPELINE_FAILURE);
- return -1;
+ return 0;
}
}
for (ctr = 0; ctr < n_recs; ctr++) {
@@ -1100,7 +1065,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
if (pad <= 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
if (sending) {
@@ -1116,7 +1081,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
if (padnum > MAX_PADDING) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
/* we need to add 'padnum' padding bytes of value padval */
padval = (unsigned char)(padnum - 1);
@@ -1127,8 +1092,10 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
}
if (!sending) {
- if (reclen[ctr] == 0 || reclen[ctr] % bs != 0)
+ if (reclen[ctr] == 0 || reclen[ctr] % bs != 0) {
+ /* Publicly invalid */
return 0;
+ }
}
}
if (n_recs > 1) {
@@ -1142,7 +1109,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
(int)n_recs, data) <= 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
SSL_R_PIPELINE_FAILURE);
- return -1;
+ return 0;
}
/* Set the input buffers */
for (ctr = 0; ctr < n_recs; ctr++) {
@@ -1154,7 +1121,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
(int)n_recs, reclen) <= 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
SSL_R_PIPELINE_FAILURE);
- return -1;
+ return 0;
}
}
@@ -1175,7 +1142,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
if (EVP_CIPHER_CTX_ctrl(ds, EVP_CTRL_TLSTREE, decrement_seq, seq) <= 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
}
@@ -1185,8 +1152,10 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
if ((EVP_CIPHER_flags(EVP_CIPHER_CTX_cipher(ds))
& EVP_CIPH_FLAG_CUSTOM_CIPHER)
? (tmpr < 0)
- : (tmpr == 0))
- return -1; /* AEAD can fail to verify MAC */
+ : (tmpr == 0)) {
+ /* AEAD can fail to verify MAC */
+ return 0;
+ }
if (sending == 0) {
if (EVP_CIPHER_mode(enc) == EVP_CIPH_GCM_MODE) {
@@ -1204,29 +1173,18 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
}
}
- ret = 1;
- if (!SSL_READ_ETM(s) && EVP_MD_CTX_md(s->read_hash) != NULL) {
- imac_size = EVP_MD_CTX_size(s->read_hash);
- if (imac_size < 0) {
- SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS1_ENC,
- ERR_R_INTERNAL_ERROR);
- return -1;
- }
- mac_size = (size_t)imac_size;
- }
- if ((bs != 1) && !sending) {
- int tmpret;
+ if (!sending) {
for (ctr = 0; ctr < n_recs; ctr++) {
- tmpret = tls1_cbc_remove_padding(s, &recs[ctr], bs, mac_size);
/*
- * If tmpret == 0 then this means publicly invalid so we can
- * short circuit things here. Otherwise we must respect constant
- * time behaviour.
+ * If using Mac-then-encrypt, then this will succeed but with a
+ * random MAC if padding is invalid
*/
- if (tmpret == 0)
+ if (!tls1_cbc_remove_padding_and_mac(s, &recs[ctr],
+ (macs != NULL) ? &macs[ctr].mac : NULL,
+ (macs != NULL) ? &macs[ctr].alloced : NULL,
+ bs,
+ macsize))
return 0;
- ret = constant_time_select_int(constant_time_eq_int(tmpret, 1),
- ret, -1);
}
}
if (pad && !sending) {
@@ -1235,7 +1193,7 @@ int tls1_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
}
}
}
- return ret;
+ return 1;
}
int n_ssl3_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int sending)
@@ -1448,16 +1406,20 @@ int tls1_mac(SSL *ssl, SSL3_RECORD *rec, unsigned char *md, int sending)
/*-
* ssl3_cbc_remove_padding removes padding from the decrypted, SSLv3, CBC
- * record in |rec| by updating |rec->length| in constant time.
+ * record in |rec| by updating |rec->length| in constant time. It also extracts
+ * the MAC from the underlying record.
*
* block_size: the block size of the cipher used to encrypt the record.
* returns:
- * 0: (in non-constant time) if the record is publicly invalid.
- * 1: if the padding was valid
- * -1: otherwise.
+ * 0: if the record is publicly invalid.
+ * 1: if the record is publicly valid. If the padding removal fails then the
+ * MAC returned is random.
*/
-int ssl3_cbc_remove_padding(SSL3_RECORD *rec,
- size_t block_size, size_t mac_size)
+int ssl3_cbc_remove_padding_and_mac(SSL *s,
+ SSL3_RECORD *rec,
+ unsigned char **mac,
+ int *alloced,
+ size_t block_size, size_t mac_size)
{
size_t padding_length;
size_t good;
@@ -1474,86 +1436,95 @@ int ssl3_cbc_remove_padding(SSL3_RECORD *rec,
/* SSLv3 requires that the padding is minimal. */
good &= constant_time_ge_s(block_size, padding_length + 1);
rec->length -= good & (padding_length + 1);
- return constant_time_select_int_s(good, 1, -1);
+
+ return ssl3_cbc_copy_mac(s, rec, mac, alloced, block_size, mac_size, good);
}
/*-
* tls1_cbc_remove_padding removes the CBC padding from the decrypted, TLS, CBC
- * record in |rec| in constant time and returns 1 if the padding is valid and
- * -1 otherwise. It also removes any explicit IV from the start of the record
- * without leaking any timing about whether there was enough space after the
- * padding was removed.
+ * record in |rec| in constant time. It also removes any explicit IV from the
+ * start of the record without leaking any timing about whether there was enough
+ * space after the padding was removed, as well as extracting the embedded MAC
+ * (also in constant time). For Mac-then-encrypt, if the padding is invalid then
+ * a success result will occur and a randomised MAC will be returned.
*
* block_size: the block size of the cipher used to encrypt the record.
* returns:
- * 0: (in non-constant time) if the record is publicly invalid.
- * 1: if the padding was valid
- * -1: otherwise.
+ * 0: if the record is publicly invalid, or an internal error
+ * 1: Success or Mac-then-encrypt decryption failed (MAC will be randomised)
*/
-int tls1_cbc_remove_padding(const SSL *s,
- SSL3_RECORD *rec,
- size_t block_size, size_t mac_size)
+int tls1_cbc_remove_padding_and_mac(const SSL *s,
+ SSL3_RECORD *rec,
+ unsigned char **mac,
+ int *alloced,
+ size_t block_size, size_t mac_size)
{
size_t good;
size_t padding_length, to_check, i;
- const size_t overhead = 1 /* padding length byte */ + mac_size;
- /* Check if version requires explicit IV */
- if (SSL_USE_EXPLICIT_IV(s)) {
- /*
- * These lengths are all public so we can test them in non-constant
- * time.
- */
- if (overhead + block_size > rec->length)
- return 0;
- /* We can now safely skip explicit IV */
- rec->data += block_size;
- rec->input += block_size;
- rec->length -= block_size;
- rec->orig_len -= block_size;
- } else if (overhead > rec->length)
+ size_t overhead = ((block_size == 1) ? 0 : 1) /* padding length byte */
+ + (SSL_USE_EXPLICIT_IV(s) ? block_size : 0)
+ + mac_size;
+
+ /*
+ * These lengths are all public so we can test them in non-constant
+ * time.
+ */
+ if (overhead > rec->length)
return 0;
- padding_length = rec->data[rec->length - 1];
+ if (block_size != 1) {
+ if (SSL_USE_EXPLICIT_IV(s)) {
+ rec->data += block_size;
+ rec->input += block_size;
+ rec->length -= block_size;
+ rec->orig_len -= block_size;
+ overhead -= block_size;
+ }
- if (EVP_CIPHER_flags(EVP_CIPHER_CTX_cipher(s->enc_read_ctx)) &
- EVP_CIPH_FLAG_AEAD_CIPHER) {
- /* padding is already verified */
- rec->length -= padding_length + 1;
- return 1;
- }
+ padding_length = rec->data[rec->length - 1];
- good = constant_time_ge_s(rec->length, overhead + padding_length);
- /*
- * The padding consists of a length byte at the end of the record and
- * then that many bytes of padding, all with the same value as the length
- * byte. Thus, with the length byte included, there are i+1 bytes of
- * padding. We can't check just |padding_length+1| bytes because that
- * leaks decrypted information. Therefore we always have to check the
- * maximum amount of padding possible. (Again, the length of the record
- * is public information so we can use it.)
- */
- to_check = 256; /* maximum amount of padding, inc length byte. */
- if (to_check > rec->length)
- to_check = rec->length;
+ if (EVP_CIPHER_flags(EVP_CIPHER_CTX_cipher(s->enc_read_ctx)) &
+ EVP_CIPH_FLAG_AEAD_CIPHER) {
+ /* padding is already verified and we don't need to check the MAC */
+ rec->length -= padding_length + 1 + mac_size;
+ *mac = NULL;
+ *alloced = 0;
+ return 1;
+ }
- for (i = 0; i < to_check; i++) {
- unsigned char mask = constant_time_ge_8_s(padding_length, i);
- unsigned char b = rec->data[rec->length - 1 - i];
+ good = constant_time_ge_s(rec->length, overhead + padding_length);
/*
- * The final |padding_length+1| bytes should all have the value
- * |padding_length|. Therefore the XOR should be zero.
+ * The padding consists of a length byte at the end of the record and
+ * then that many bytes of padding, all with the same value as the
+ * length byte. Thus, with the length byte included, there are i+1 bytes
+ * of padding. We can't check just |padding_length+1| bytes because that
+ * leaks decrypted information. Therefore we always have to check the
+ * maximum amount of padding possible. (Again, the length of the record
+ * is public information so we can use it.)
*/
- good &= ~(mask & (padding_length ^ b));
- }
+ to_check = 256; /* maximum amount of padding, inc length byte. */
+ if (to_check > rec->length)
+ to_check = rec->length;
- /*
- * If any of the final |padding_length+1| bytes had the wrong value, one
- * or more of the lower eight bits of |good| will be cleared.
- */
- good = constant_time_eq_s(0xff, good & 0xff);
- rec->length -= good & (padding_length + 1);
+ for (i = 0; i < to_check; i++) {
+ unsigned char mask = constant_time_ge_8_s(padding_length, i);
+ unsigned char b = rec->data[rec->length - 1 - i];
+ /*
+ * The final |padding_length+1| bytes should all have the value
+ * |padding_length|. Therefore the XOR should be zero.
+ */
+ good &= ~(mask & (padding_length ^ b));
+ }
+
+ /*
+ * If any of the final |padding_length+1| bytes had the wrong value, one
+ * or more of the lower eight bits of |good| will be cleared.
+ */
+ good = constant_time_eq_s(0xff, good & 0xff);
+ rec->length -= good & (padding_length + 1);
+ }
- return constant_time_select_int_s(good, 1, -1);
+ return ssl3_cbc_copy_mac(s, rec, mac, alloced, block_size, mac_size, good);
}
/*-
@@ -1561,9 +1532,6 @@ int tls1_cbc_remove_padding(const SSL *s,
* constant time (independent of the concrete value of rec->length, which may
* vary within a 256-byte window).
*
- * ssl3_cbc_remove_padding or tls1_cbc_remove_padding must be called prior to
- * this function.
- *
* On entry:
* rec->orig_len >= md_size
* md_size <= EVP_MAX_MD_SIZE
@@ -1576,8 +1544,13 @@ int tls1_cbc_remove_padding(const SSL *s,
*/
#define CBC_MAC_ROTATE_IN_PLACE
-int ssl3_cbc_copy_mac(unsigned char *out,
- const SSL3_RECORD *rec, size_t md_size)
+static int ssl3_cbc_copy_mac(const SSL *s,
+ SSL3_RECORD *rec,
+ unsigned char **mac,
+ int *alloced,
+ size_t block_size,
+ size_t mac_size,
+ size_t good)
{
#if defined(CBC_MAC_ROTATE_IN_PLACE)
unsigned char rotated_mac_buf[64 + EVP_MAX_MD_SIZE];
@@ -1585,12 +1558,14 @@ int ssl3_cbc_copy_mac(unsigned char *out,
#else
unsigned char rotated_mac[EVP_MAX_MD_SIZE];
#endif
+ unsigned char randmac[EVP_MAX_MD_SIZE];
+ unsigned char *out;
/*
* mac_end is the index of |rec->data| just after the end of the MAC.
*/
size_t mac_end = rec->length;
- size_t mac_start = mac_end - md_size;
+ size_t mac_start = mac_end - mac_size;
size_t in_mac;
/*
* scan_start contains the number of bytes that we can ignore because the
@@ -1600,21 +1575,51 @@ int ssl3_cbc_copy_mac(unsigned char *out,
size_t i, j;
size_t rotate_offset;
- if (!ossl_assert(rec->orig_len >= md_size
- && md_size <= EVP_MAX_MD_SIZE))
+ if (!ossl_assert(rec->orig_len >= mac_size
+ && mac_size <= EVP_MAX_MD_SIZE))
+ return 0;
+
+ /* If no MAC then nothing to be done */
+ if (mac_size == 0) {
+ /* No MAC so we can do this in non-constant time */
+ if (good == 0)
+ return 0;
+ return 1;
+ }
+
+ rec->length -= mac_size;
+
+ if (block_size == 1) {
+ /* There's no padding so the position of the MAC is fixed */
+ if (mac != NULL)
+ *mac = &rec->data[rec->length];
+ if (alloced != NULL)
+ *alloced = 0;
+ return 1;
+ }
+
+ /* Create the random MAC we will emit if padding is bad */
+ if (!RAND_bytes_ex(s->ctx->libctx, randmac, mac_size))
+ return 0;
+
+ if (!ossl_assert(mac != NULL && alloced != NULL))
+ return 0;
+ *mac = out = OPENSSL_malloc(mac_size);
+ if (*mac == NULL)
return 0;
+ *alloced = 1;
#if defined(CBC_MAC_ROTATE_IN_PLACE)
rotated_mac = rotated_mac_buf + ((0 - (size_t)rotated_mac_buf) & 63);
#endif
/* This information is public so it's safe to branch based on it. */
- if (rec->orig_len > md_size + 255 + 1)
- scan_start = rec->orig_len - (md_size + 255 + 1);
+ if (rec->orig_len > mac_size + 255 + 1)
+ scan_start = rec->orig_len - (mac_size + 255 + 1);
in_mac = 0;
rotate_offset = 0;
- memset(rotated_mac, 0, md_size);
+ memset(rotated_mac, 0, mac_size);
for (i = scan_start, j = 0; i < rec->orig_len; i++) {
size_t mac_started = constant_time_eq_s(i, mac_start);
size_t mac_ended = constant_time_lt_s(i, mac_end);
@@ -1624,27 +1629,35 @@ int ssl3_cbc_copy_mac(unsigned char *out,
in_mac &= mac_ended;
rotate_offset |= j & mac_started;
rotated_mac[j++] |= b & in_mac;
- j &= constant_time_lt_s(j, md_size);
+ j &= constant_time_lt_s(j, mac_size);
}
/* Now rotate the MAC */
#if defined(CBC_MAC_ROTATE_IN_PLACE)
j = 0;
- for (i = 0; i < md_size; i++) {
+ for (i = 0; i < mac_size; i++) {
/* in case cache-line is 32 bytes, touch second line */
((volatile unsigned char *)rotated_mac)[rotate_offset ^ 32];
- out[j++] = rotated_mac[rotate_offset++];
- rotate_offset &= constant_time_lt_s(rotate_offset, md_size);
+
+ /* If the padding wasn't good we emit a random MAC */
+ out[j++] = constant_time_select_8((unsigned char)(good & 0xff),
+ rotated_mac[rotate_offset++],
+ randmac[i]);
+ rotate_offset &= constant_time_lt_s(rotate_offset, mac_size);
}
#else
- memset(out, 0, md_size);
- rotate_offset = md_size - rotate_offset;
- rotate_offset &= constant_time_lt_s(rotate_offset, md_size);
- for (i = 0; i < md_size; i++) {
- for (j = 0; j < md_size; j++)
+ memset(out, 0, mac_size);
+ rotate_offset = mac_size - rotate_offset;
+ rotate_offset &= constant_time_lt_s(rotate_offset, mac_size);
+ for (i = 0; i < mac_size; i++) {
+ for (j = 0; j < mac_size; j++)
out[j] |= rotated_mac[i] & constant_time_eq_8_s(j, rotate_offset);
rotate_offset++;
- rotate_offset &= constant_time_lt_s(rotate_offset, md_size);
+ rotate_offset &= constant_time_lt_s(rotate_offset, mac_size);
+
+ /* If the padding wasn't good we emit a random MAC */
+ out[i] = constant_time_select_8((unsigned char)(good & 0xff), out[i],
+ randmac[i]);
}
#endif
@@ -1658,9 +1671,11 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
SSL_SESSION *sess;
SSL3_RECORD *rr;
int imac_size;
- size_t mac_size;
+ size_t mac_size = 0;
unsigned char md[EVP_MAX_MD_SIZE];
size_t max_plain_length = SSL3_RT_MAX_PLAIN_LENGTH;
+ SSL_MAC_BUF macbuf = { NULL, 0 };
+ int ret = 0;
rr = RECORD_LAYER_get_rrec(&s->rlayer);
sess = s->session;
@@ -1694,14 +1709,24 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
rr->data = rr->input;
rr->orig_len = rr->length;
+ /* TODO(size_t): convert this to do size_t properly */
+ if (s->read_hash != NULL) {
+ const EVP_MD *tmpmd = EVP_MD_CTX_md(s->read_hash);
+
+ if (tmpmd != NULL) {
+ imac_size = EVP_MD_size(tmpmd);
+ if (!ossl_assert(imac_size >= 0 && imac_size <= EVP_MAX_MD_SIZE)) {
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_SSL3_GET_RECORD,
+ ERR_LIB_EVP);
+ return -1;
+ }
+ mac_size = (size_t)imac_size;
+ }
+ }
+
if (SSL_READ_ETM(s) && s->read_hash) {
unsigned char *mac;
- mac_size = EVP_MD_CTX_size(s->read_hash);
- if (!ossl_assert(mac_size <= EVP_MAX_MD_SIZE)) {
- SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DTLS1_PROCESS_RECORD,
- ERR_R_INTERNAL_ERROR);
- return 0;
- }
+
if (rr->orig_len < mac_size) {
SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_DTLS1_PROCESS_RECORD,
SSL_R_LENGTH_TOO_SHORT);
@@ -1715,24 +1740,30 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
SSL_R_DECRYPTION_FAILED_OR_BAD_RECORD_MAC);
return 0;
}
+ /*
+ * We've handled the mac now - there is no MAC inside the encrypted
+ * record
+ */
+ mac_size = 0;
}
- enc_err = s->method->ssl3_enc->enc(s, rr, 1, 0);
+ enc_err = s->method->ssl3_enc->enc(s, rr, 1, 0, &macbuf, mac_size);
+
/*-
* enc_err is:
- * 0: (in non-constant time) if the record is publicly invalid.
- * 1: if the padding is valid
- * -1: if the padding is invalid
+ * 0: if the record is publicly invalid, or an internal error, or AEAD
+ * decryption failed, or ETM decryption failed.
+ * 1: Success or MTE decryption failed (MAC will be randomised)
*/
if (enc_err == 0) {
if (ossl_statem_in_error(s)) {
/* SSLfatal() got called */
- return 0;
+ goto end;
}
/* For DTLS we simply ignore bad packets. */
rr->length = 0;
RECORD_LAYER_reset_packet_length(&s->rlayer);
- return 0;
+ goto end;
}
OSSL_TRACE_BEGIN(TLS) {
BIO_printf(trc_out, "dec %zd\n", rr->length);
@@ -1743,75 +1774,20 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
if ((sess != NULL) && !SSL_READ_ETM(s) &&
(s->enc_read_ctx != NULL) && (EVP_MD_CTX_md(s->read_hash) != NULL)) {
/* s->read_hash != NULL => mac_size != -1 */
- unsigned char *mac = NULL;
- unsigned char mac_tmp[EVP_MAX_MD_SIZE];
-
- /* TODO(size_t): Convert this to do size_t properly */
- imac_size = EVP_MD_CTX_size(s->read_hash);
- if (imac_size < 0) {
- SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DTLS1_PROCESS_RECORD,
- ERR_LIB_EVP);
- return 0;
- }
- mac_size = (size_t)imac_size;
- if (!ossl_assert(mac_size <= EVP_MAX_MD_SIZE)) {
- SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DTLS1_PROCESS_RECORD,
- ERR_R_INTERNAL_ERROR);
- return 0;
- }
-
- /*
- * orig_len is the length of the record before any padding was
- * removed. This is public information, as is the MAC in use,
- * therefore we can safely process the record in a different amount
- * of time if it's too short to possibly contain a MAC.
- */
- if (rr->orig_len < mac_size ||
- /* CBC records must have a padding length byte too. */
- (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE &&
- rr->orig_len < mac_size + 1)) {
- SSLfatal(s, SSL_AD_DECODE_ERROR, SSL_F_DTLS1_PROCESS_RECORD,
- SSL_R_LENGTH_TOO_SHORT);
- return 0;
- }
-
- if (EVP_CIPHER_CTX_mode(s->enc_read_ctx) == EVP_CIPH_CBC_MODE) {
- /*
- * We update the length so that the TLS header bytes can be
- * constructed correctly but we need to extract the MAC in
- * constant time from within the record, without leaking the
- * contents of the padding bytes.
- */
- mac = mac_tmp;
- if (!ssl3_cbc_copy_mac(mac_tmp, rr, mac_size)) {
- SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_DTLS1_PROCESS_RECORD,
- ERR_R_INTERNAL_ERROR);
- return 0;
- }
- rr->length -= mac_size;
- } else {
- /*
- * In this case there's no padding, so |rec->orig_len| equals
- * |rec->length| and we checked that there's enough bytes for
- * |mac_size| above.
- */
- rr->length -= mac_size;
- mac = &rr->data[rr->length];
- }
i = s->method->ssl3_enc->mac(s, rr, md, 0 /* not send */ );
- if (i == 0 || mac == NULL
- || CRYPTO_memcmp(md, mac, mac_size) != 0)
- enc_err = -1;
+ if (i == 0 || macbuf.mac == NULL
+ || CRYPTO_memcmp(md, macbuf.mac, mac_size) != 0)
+ enc_err = 0;
if (rr->length > SSL3_RT_MAX_COMPRESSED_LENGTH + mac_size)
- enc_err = -1;
+ enc_err = 0;
}
- if (enc_err < 0) {
+ if (enc_err == 0) {
/* decryption failed, silently discard message */
rr->length = 0;
RECORD_LAYER_reset_packet_length(&s->rlayer);
- return 0;
+ goto end;
}
/* r->length is now just compressed */
@@ -1819,12 +1795,12 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
if (rr->length > SSL3_RT_MAX_COMPRESSED_LENGTH) {
SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_DTLS1_PROCESS_RECORD,
SSL_R_COMPRESSED_LENGTH_TOO_LONG);
- return 0;
+ goto end;
}
if (!ssl3_do_uncompress(s, rr)) {
SSLfatal(s, SSL_AD_DECOMPRESSION_FAILURE,
SSL_F_DTLS1_PROCESS_RECORD, SSL_R_BAD_DECOMPRESSION);
- return 0;
+ goto end;
}
}
@@ -1836,7 +1812,7 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
if (rr->length > max_plain_length) {
SSLfatal(s, SSL_AD_RECORD_OVERFLOW, SSL_F_DTLS1_PROCESS_RECORD,
SSL_R_DATA_LENGTH_TOO_LONG);
- return 0;
+ goto end;
}
rr->off = 0;
@@ -1855,7 +1831,11 @@ int dtls1_process_record(SSL *s, DTLS1_BITMAP *bitmap)
/* Mark receipt of record. */
dtls1_record_bitmap_update(s, bitmap);
- return 1;
+ ret = 1;
+ end:
+ if (macbuf.alloced)
+ OPENSSL_free(macbuf.mac);
+ return ret;
}
/*
diff --git a/ssl/record/ssl3_record_tls13.c b/ssl/record/ssl3_record_tls13.c
index f18da2db74..910b6a5862 100644
--- a/ssl/record/ssl3_record_tls13.c
+++ b/ssl/record/ssl3_record_tls13.c
@@ -12,17 +12,16 @@
#include "internal/cryptlib.h"
/*-
- * tls13_enc encrypts/decrypts |n_recs| in |recs|. Will call SSLfatal() for
- * internal errors, but not otherwise.
+ * tls13_enc encrypts/decrypts |n_recs| in |recs|. Calls SSLfatal on internal
+ * error, but not otherwise. It is the responsibility of the caller to report
+ * a bad_record_mac.
*
* Returns:
- * 0: (in non-constant time) if the record is publicly invalid (i.e. too
- * short etc).
- * 1: if the record encryption was successful.
- * -1: if the record's AEAD-authenticator is invalid or, if sending,
- * an internal error occurred.
+ * 0: On failure
+ * 1: if the record encryption/decryption was successful.
*/
-int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
+int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending,
+ SSL_MAC_BUF *mac, size_t macsize)
{
EVP_CIPHER_CTX *ctx;
unsigned char iv[EVP_MAX_IV_LENGTH], recheader[SSL3_RT_HEADER_LENGTH];
@@ -39,7 +38,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
/* TODO(TLS1.3): Support pipelining */
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
if (sending) {
@@ -75,7 +74,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
&& s->psksession->ext.max_early_data > 0)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
alg_enc = s->psksession->cipher->algorithm_enc;
}
@@ -87,7 +86,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
if (!ossl_assert(s->s3.tmp.new_cipher != NULL)) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
alg_enc = s->s3.tmp.new_cipher->algorithm_enc;
}
@@ -101,7 +100,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
NULL) <= 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
} else if (alg_enc & SSL_AESGCM) {
taglen = EVP_GCM_TLS_TAG_LEN;
@@ -110,7 +109,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
} else {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
if (!sending) {
@@ -128,7 +127,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
/* Should not happen */
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
offset = ivlen - SEQ_NUM_SIZE;
memcpy(iv, staticiv, offset);
@@ -143,7 +142,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
}
if (loop == 0) {
/* Sequence has wrapped */
- return -1;
+ return 0;
}
/* TODO(size_t): lenu/lenf should be a size_t but EVP doesn't support it */
@@ -151,7 +150,9 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
|| (!sending && EVP_CIPHER_CTX_ctrl(ctx, EVP_CTRL_AEAD_SET_TAG,
taglen,
rec->data + rec->length) <= 0)) {
- return -1;
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC,
+ ERR_R_INTERNAL_ERROR);
+ return 0;
}
/* Set up the AAD */
@@ -162,8 +163,10 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
|| !WPACKET_get_total_written(&wpkt, &hdrlen)
|| hdrlen != SSL3_RT_HEADER_LENGTH
|| !WPACKET_finish(&wpkt)) {
+ SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC,
+ ERR_R_INTERNAL_ERROR);
WPACKET_cleanup(&wpkt);
- return -1;
+ return 0;
}
/*
@@ -179,7 +182,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
(unsigned int)rec->length) <= 0
|| EVP_CipherFinal_ex(ctx, rec->data + lenu, &lenf) <= 0
|| (size_t)(lenu + lenf) != rec->length) {
- return -1;
+ return 0;
}
if (sending) {
/* Add the tag */
@@ -187,7 +190,7 @@ int tls13_enc(SSL *s, SSL3_RECORD *recs, size_t n_recs, int sending)
rec->data + rec->length) <= 0) {
SSLfatal(s, SSL_AD_INTERNAL_ERROR, SSL_F_TLS13_ENC,
ERR_R_INTERNAL_ERROR);
- return -1;
+ return 0;
}
rec->length += taglen;
}
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index a252761ca4..c3174a7c91 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -34,51 +34,37 @@ DEFINE_STACK_OF(OCSP_RESPID)
DEFINE_STACK_OF(SRTP_PROTECTION_PROFILE)
DEFINE_STACK_OF(SCT)
-static int ssl_undefined_function_1(SSL *ssl, SSL3_RECORD *r, size_t s, int t)
+static int ssl_undefined_function_1(SSL *ssl, SSL3_RECORD *r, size_t s, int t,
+ SSL_MAC_BUF *mac, size_t macsize)
{
- (void)r;
- (void)s;
- (void)t;
return ssl_undefined_function(ssl);
}
static int ssl_undefined_function_2(SSL *ssl, SSL3_RECORD *r, unsigned char *s,
int t)
{
- (void)r;
- (void)s;
- (void)t;
return ssl_undefined_function(ssl);
}
static int ssl_undefined_function_3(SSL *ssl, unsigned char *r,
unsigned char *s, size_t t, size_t *u)
{
- (void)r;
- (void)s;
- (void)t;
- (void)u;
return ssl_undefined_function(ssl);
}
static int ssl_undefined_function_4(SSL *ssl, int r)
{
- (void)r;
return ssl_undefined_function(ssl);
}
static size_t ssl_undefined_function_5(SSL *ssl, const char *r, size_t s,
unsigned char *t)
{
- (void)r;
- (void)s;
- (void)t;
return ssl_undefined_function(ssl);
}
static int ssl_undefined_function_6(int r)
{
- (void)r;
return ssl_undefined_function(NULL);
}
@@ -86,13 +72,6 @@ static int ssl_undefined_function_7(SSL *ssl, unsigned char *r, size_t s,
const char *t, size_t u,
const unsigned char *v, size_t w, int x)
{
- (void)r;
- (void)s;
- (void)t;
- (void)u;
- (void)v;
- (void)w;
- (void)x;
return ssl_undefined_function(ssl);
}
diff --git a/ssl/ssl_local.h b/ssl/ssl_local.h
index 58bc1f99c4..17b856fabc 100644
--- a/ssl/ssl_local.h
+++ b/ssl/ssl_local.h
@@ -2069,7 +2069,7 @@ typedef struct cert_st {
* of a mess of functions, but hell, think of it as an opaque structure :-)
*/
typedef struct ssl3_enc_method {
- int (*enc) (SSL *, SSL3_RECORD *, size_t, int);
+ int (*enc) (SSL *, SSL3_RECORD *, size_t, int, SSL_MAC_BUF *, size_t);
int (*mac) (SSL *, SSL3_RECORD *, unsigned char *, int);
int (*setup_key_block) (SSL *);
int (*generate_master_secret) (SSL *, unsigned char *, unsigned char *,