Consolidate sequence counter incrementing code

The sequence counter was incremented in numerous different ways in
numerous different locations. We introduce a single function to do this
inside the record layer.

Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Hugo Landau <hlandau@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19424)
This commit is contained in:
Matt Caswell 2022-10-17 12:28:07 +01:00
parent 4f428e86d8
commit bed07b1875
11 changed files with 45 additions and 41 deletions

View File

@ -1490,6 +1490,7 @@ SSL_R_REQUIRED_COMPRESSION_ALGORITHM_MISSING:342:\
required compression algorithm missing
SSL_R_SCSV_RECEIVED_WHEN_RENEGOTIATING:345:scsv received when renegotiating
SSL_R_SCT_VERIFICATION_FAILED:208:sct verification failed
SSL_R_SEQUENCE_CTR_WRAPPED:326:sequence ctr wrapped
SSL_R_SERVERHELLO_TLSEXT:275:serverhello tlsext
SSL_R_SESSION_ID_CONTEXT_UNINITIALIZED:277:session id context uninitialized
SSL_R_SHUTDOWN_WHILE_IN_INIT:407:shutdown while in init

View File

@ -239,6 +239,7 @@
# define SSL_R_REQUIRED_COMPRESSION_ALGORITHM_MISSING 342
# define SSL_R_SCSV_RECEIVED_WHEN_RENEGOTIATING 345
# define SSL_R_SCT_VERIFICATION_FAILED 208
# define SSL_R_SEQUENCE_CTR_WRAPPED 326
# define SSL_R_SERVERHELLO_TLSEXT 275
# define SSL_R_SESSION_ID_CONTEXT_UNINITIALIZED 277
# define SSL_R_SHUTDOWN_WHILE_IN_INIT 407

View File

@ -810,8 +810,10 @@ int dtls_write_records(OSSL_RECORD_LAYER *rl, OSSL_RECORD_TEMPLATE *templates,
goto err;
}
/* TODO(RECLAYER): FIXME */
ssl3_record_sequence_update(rl->sequence);
if (!tls_increment_sequence_ctr(rl)) {
/* RLAYERfatal() already called */
goto err;
}
/* now let's set up wb */
SSL3_BUFFER_set_left(wb, SSL3_RECORD_get_length(&wr));

View File

@ -344,6 +344,8 @@ __owur int ssl3_cbc_digest_record(const EVP_MD *md,
const unsigned char *mac_secret,
size_t mac_secret_length, char is_sslv3);
int tls_increment_sequence_ctr(OSSL_RECORD_LAYER *rl);
int tls_default_read_n(OSSL_RECORD_LAYER *rl, size_t n, size_t max, int extend,
int clearold, size_t *readbytes);
int tls_get_more_records(OSSL_RECORD_LAYER *rl);

View File

@ -296,7 +296,9 @@ static int ssl3_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md,
EVP_MD_CTX_free(md_ctx);
}
ssl3_record_sequence_update(seq);
if (!tls_increment_sequence_ctr(rl))
return 0;
return 1;
}

View File

@ -122,14 +122,8 @@ static int tls13_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs,
for (loop = 0; loop < SEQ_NUM_SIZE; loop++)
iv[offset + loop] = staticiv[offset + loop] ^ seq[loop];
/* Increment the sequence counter */
for (loop = SEQ_NUM_SIZE; loop > 0; loop--) {
++seq[loop - 1];
if (seq[loop - 1] != 0)
break;
}
if (loop == 0) {
/* Sequence has wrapped */
if (!tls_increment_sequence_ctr(rl)) {
/* RLAYERfatal already called */
return 0;
}

View File

@ -162,7 +162,7 @@ static int tls1_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs,
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, tmpr, provided;
int pad = 0, tmpr, provided;
size_t bs, ctr, padnum, loop;
unsigned char padval;
const EVP_CIPHER *enc;
@ -247,10 +247,9 @@ static int tls1_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs,
memcpy(buf[ctr], dtlsseq, 8);
} else {
memcpy(buf[ctr], seq, 8);
for (i = 7; i >= 0; i--) { /* increment */
++seq[i];
if (seq[i] != 0)
break;
if (!tls_increment_sequence_ctr(rl)) {
/* RLAYERfatal already called */
return 0;
}
}
@ -324,7 +323,6 @@ static int tls1_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs,
}
if (!rl->isdtls && rl->tlstree) {
unsigned char *seq;
int decrement_seq = 0;
/*
@ -335,8 +333,9 @@ static int tls1_cipher(OSSL_RECORD_LAYER *rl, SSL3_RECORD *recs, size_t n_recs,
if (sending && !rl->use_etm)
decrement_seq = 1;
seq = rl->sequence;
if (EVP_CIPHER_CTX_ctrl(ds, EVP_CTRL_TLSTREE, decrement_seq, seq) <= 0) {
if (EVP_CIPHER_CTX_ctrl(ds, EVP_CTRL_TLSTREE, decrement_seq,
rl->sequence) <= 0) {
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
return 0;
}
@ -455,7 +454,6 @@ static int tls1_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md,
unsigned char *seq = rl->sequence;
EVP_MD_CTX *hash;
size_t md_size;
int i;
EVP_MD_CTX *hmac = NULL, *mac_ctx;
unsigned char header[13];
int t;
@ -526,13 +524,11 @@ static int tls1_mac(OSSL_RECORD_LAYER *rl, SSL3_RECORD *rec, unsigned char *md,
BIO_dump_indent(trc_out, rec->data, rec->length, 4);
} OSSL_TRACE_END(TLS);
if (!rl->isdtls) {
for (i = 7; i >= 0; i--) {
++seq[i];
if (seq[i] != 0)
break;
}
if (!rl->isdtls && !tls_increment_sequence_ctr(rl)) {
/* RLAYERfatal already called */
goto end;
}
OSSL_TRACE_BEGIN(TLS) {
BIO_printf(trc_out, "md:\n");
BIO_dump_indent(trc_out, md, md_size, 4);

View File

@ -247,6 +247,24 @@ static int tls_release_read_buffer(OSSL_RECORD_LAYER *rl)
return 1;
}
int tls_increment_sequence_ctr(OSSL_RECORD_LAYER *rl)
{
int i;
/* Increment the sequence counter */
for (i = SEQ_NUM_SIZE; i > 0; i--) {
++(rl->sequence[i - 1]);
if (rl->sequence[i - 1] != 0)
break;
}
if (i == 0) {
/* Sequence has wrapped */
RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, SSL_R_SEQUENCE_CTR_WRAPPED);
return 0;
}
return 1;
}
/*
* Return values are as per SSL_read()
*/
@ -753,7 +771,7 @@ int tls_get_more_records(OSSL_RECORD_LAYER *rl)
* If in encrypt-then-mac mode calculate mac from encrypted record. All
* the details below are public so no timing details can leak.
*/
if (rl->use_etm && rl->md_ctx) {
if (rl->use_etm && rl->md_ctx != NULL) {
unsigned char *mac;
for (j = 0; j < num_recs; j++) {
@ -838,8 +856,6 @@ int tls_get_more_records(OSSL_RECORD_LAYER *rl)
if (rl->enc_ctx != NULL
&& !rl->use_etm
&& EVP_MD_CTX_get0_md(rl->md_ctx) != NULL) {
/* rl->md_ctx != NULL => mac_size != -1 */
for (j = 0; j < num_recs; j++) {
SSL_MAC_BUF *thismb = &macbufs[j];

View File

@ -999,17 +999,6 @@ int ssl3_read_bytes(SSL *ssl, int type, int *recvd_type, unsigned char *buf,
}
}
void ssl3_record_sequence_update(unsigned char *seq)
{
int i;
for (i = 7; i >= 0; i--) {
++seq[i];
if (seq[i] != 0)
break;
}
}
/*
* Returns true if the current rrec was sent in SSLv2 backwards compatible
* format and false otherwise.

View File

@ -26,7 +26,6 @@
#define DTLS_RECORD_LAYER_get_r_epoch(rl) ((rl)->d->r_epoch)
int dtls_buffer_record(SSL_CONNECTION *s, TLS_RECORD *rec);
void ssl3_record_sequence_update(unsigned char *seq);
/* Macros/functions provided by the SSL3_BUFFER component */

View File

@ -372,6 +372,8 @@ static const ERR_STRING_DATA SSL_str_reasons[] = {
"scsv received when renegotiating"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_SCT_VERIFICATION_FAILED),
"sct verification failed"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_SEQUENCE_CTR_WRAPPED),
"sequence ctr wrapped"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_SERVERHELLO_TLSEXT), "serverhello tlsext"},
{ERR_PACK(ERR_LIB_SSL, 0, SSL_R_SESSION_ID_CONTEXT_UNINITIALIZED),
"session id context uninitialized"},