From: jsing Date: Tue, 26 Jan 2021 14:22:19 +0000 (+0000) Subject: Move sequence numbers into the new TLSv1.2 record layer. X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=f2284ad0cdee0d2243e19a1dda1588956f603b5d;p=openbsd Move sequence numbers into the new TLSv1.2 record layer. This allows for all of the DTLS sequence number save/restore code to be removed. ok inoguchi@ "whee!" tb@ --- diff --git a/lib/libssl/d1_both.c b/lib/libssl/d1_both.c index 8c4fec589f6..ba4e9edd8d1 100644 --- a/lib/libssl/d1_both.c +++ b/lib/libssl/d1_both.c @@ -1,4 +1,4 @@ -/* $OpenBSD: d1_both.c,v 1.65 2021/01/19 19:07:39 jsing Exp $ */ +/* $OpenBSD: d1_both.c,v 1.66 2021/01/26 14:22:19 jsing Exp $ */ /* * DTLS implementation written by Nagendra Modadugu * (nagendra@cs.stanford.edu) for the OpenSSL project 2005. @@ -1009,7 +1009,6 @@ dtls1_retransmit_message(SSL *s, unsigned short seq, unsigned long frag_off, unsigned long header_length; unsigned char seq64be[8]; struct dtls1_retransmit_state saved_state; - unsigned char save_write_sequence[8]; /* OPENSSL_assert(s->internal->init_num == 0); @@ -1059,14 +1058,6 @@ dtls1_retransmit_message(SSL *s, unsigned short seq, unsigned long frag_off, if (!tls12_record_layer_use_write_epoch(s->internal->rl, D1I(s)->w_epoch)) return 0; - if (frag->msg_header.saved_retransmit_state.epoch == - saved_state.epoch - 1) { - memcpy(save_write_sequence, S3I(s)->write_sequence, - sizeof(S3I(s)->write_sequence)); - memcpy(S3I(s)->write_sequence, D1I(s)->last_write_sequence, - sizeof(S3I(s)->write_sequence)); - } - ret = dtls1_do_write(s, frag->msg_header.is_ccs ? SSL3_RT_CHANGE_CIPHER_SPEC : SSL3_RT_HANDSHAKE); @@ -1077,14 +1068,6 @@ dtls1_retransmit_message(SSL *s, unsigned short seq, unsigned long frag_off, if (!tls12_record_layer_use_write_epoch(s->internal->rl, D1I(s)->w_epoch)) return 0; - if (frag->msg_header.saved_retransmit_state.epoch == - saved_state.epoch - 1) { - memcpy(D1I(s)->last_write_sequence, S3I(s)->write_sequence, - sizeof(S3I(s)->write_sequence)); - memcpy(S3I(s)->write_sequence, save_write_sequence, - sizeof(S3I(s)->write_sequence)); - } - D1I(s)->retransmitting = 0; (void)BIO_flush(SSL_get_wbio(s)); diff --git a/lib/libssl/d1_pkt.c b/lib/libssl/d1_pkt.c index bde13c99e5e..bbf2e8e538f 100644 --- a/lib/libssl/d1_pkt.c +++ b/lib/libssl/d1_pkt.c @@ -1,4 +1,4 @@ -/* $OpenBSD: d1_pkt.c,v 1.90 2021/01/19 19:07:39 jsing Exp $ */ +/* $OpenBSD: d1_pkt.c,v 1.91 2021/01/26 14:22:19 jsing Exp $ */ /* * DTLS implementation written by Nagendra Modadugu * (nagendra@cs.stanford.edu) for the OpenSSL project 2005. @@ -205,9 +205,6 @@ dtls1_copy_record(SSL *s, DTLS1_RECORD_DATA_INTERNAL *rdata) memcpy(&(S3I(s)->rbuf), &(rdata->rbuf), sizeof(SSL3_BUFFER_INTERNAL)); memcpy(&(S3I(s)->rrec), &(rdata->rrec), sizeof(SSL3_RECORD_INTERNAL)); - /* Set proper sequence number for mac calculation */ - memcpy(&(S3I(s)->read_sequence[2]), &(rdata->packet[5]), 6); - return (1); } @@ -419,10 +416,6 @@ again: if (!CBS_get_u16(&header, &len)) goto again; - if (!CBS_write_bytes(&seq_no, &(S3I(s)->read_sequence[2]), - sizeof(S3I(s)->read_sequence) - 2, NULL)) - goto again; - if (!CBS_write_bytes(&seq_no, &rr->seq_num[2], sizeof(rr->seq_num) - 2, NULL)) goto again; @@ -1241,12 +1234,8 @@ dtls1_reset_seq_numbers(SSL *s, int rw) memcpy(&(D1I(s)->bitmap), &(D1I(s)->next_bitmap), sizeof(DTLS1_BITMAP)); memset(&(D1I(s)->next_bitmap), 0, sizeof(DTLS1_BITMAP)); - memset(S3I(s)->read_sequence, 0, sizeof(S3I(s)->read_sequence)); } else { D1I(s)->w_epoch++; tls12_record_layer_set_write_epoch(s->internal->rl, D1I(s)->w_epoch); - memcpy(D1I(s)->last_write_sequence, S3I(s)->write_sequence, - sizeof(S3I(s)->write_sequence)); - memset(S3I(s)->write_sequence, 0, sizeof(S3I(s)->write_sequence)); } } diff --git a/lib/libssl/ssl_lib.c b/lib/libssl/ssl_lib.c index 0537cf0e463..e0e0ae4ff11 100644 --- a/lib/libssl/ssl_lib.c +++ b/lib/libssl/ssl_lib.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_lib.c,v 1.240 2021/01/09 10:34:29 tb Exp $ */ +/* $OpenBSD: ssl_lib.c,v 1.241 2021/01/26 14:22:19 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -2612,8 +2612,6 @@ ssl_clear_cipher_read_state(SSL *s) s->read_hash = NULL; tls12_record_layer_clear_read_state(s->internal->rl); - tls12_record_layer_set_read_seq_num(s->internal->rl, - S3I(s)->read_sequence); if (s->internal->aead_read_ctx != NULL) { EVP_AEAD_CTX_cleanup(&s->internal->aead_read_ctx->ctx); @@ -2631,8 +2629,6 @@ ssl_clear_cipher_write_state(SSL *s) s->internal->write_hash = NULL; tls12_record_layer_clear_write_state(s->internal->rl); - tls12_record_layer_set_write_seq_num(s->internal->rl, - S3I(s)->write_sequence); if (s->internal->aead_write_ctx != NULL) { EVP_AEAD_CTX_cleanup(&s->internal->aead_write_ctx->ctx); diff --git a/lib/libssl/ssl_locl.h b/lib/libssl/ssl_locl.h index 01f2ebbab1c..4390361c663 100644 --- a/lib/libssl/ssl_locl.h +++ b/lib/libssl/ssl_locl.h @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_locl.h,v 1.316 2021/01/21 18:48:57 jsing Exp $ */ +/* $OpenBSD: ssl_locl.h,v 1.317 2021/01/26 14:22:20 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -491,10 +491,7 @@ void tls12_record_layer_write_epoch_done(struct tls12_record_layer *rl, uint16_t epoch); void tls12_record_layer_clear_read_state(struct tls12_record_layer *rl); void tls12_record_layer_clear_write_state(struct tls12_record_layer *rl); -void tls12_record_layer_set_read_seq_num(struct tls12_record_layer *rl, - uint8_t *seq_num); -void tls12_record_layer_set_write_seq_num(struct tls12_record_layer *rl, - uint8_t *seq_num); +void tls12_record_layer_reflect_seq_num(struct tls12_record_layer *rl); int tls12_record_layer_set_read_aead(struct tls12_record_layer *rl, SSL_AEAD_CTX *aead_ctx); int tls12_record_layer_set_write_aead(struct tls12_record_layer *rl, @@ -844,9 +841,6 @@ typedef struct ssl3_buffer_internal_st { } SSL3_BUFFER_INTERNAL; typedef struct ssl3_state_internal_st { - unsigned char read_sequence[SSL3_SEQUENCE_SIZE]; - unsigned char write_sequence[SSL3_SEQUENCE_SIZE]; - SSL3_BUFFER_INTERNAL rbuf; /* read IO goes into here */ SSL3_BUFFER_INTERNAL wbuf; /* write IO goes into here */ @@ -990,9 +984,6 @@ typedef struct dtls1_state_internal_st { unsigned short handshake_read_seq; - /* save last sequence number for retransmissions */ - unsigned char last_write_sequence[SSL3_SEQUENCE_SIZE]; - /* Received handshake records (processed and unprocessed) */ record_pqueue unprocessed_rcds; record_pqueue processed_rcds; diff --git a/lib/libssl/ssl_srvr.c b/lib/libssl/ssl_srvr.c index 000cac6785d..3551ee41ee0 100644 --- a/lib/libssl/ssl_srvr.c +++ b/lib/libssl/ssl_srvr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssl_srvr.c,v 1.89 2021/01/19 18:57:09 jsing Exp $ */ +/* $OpenBSD: ssl_srvr.c,v 1.90 2021/01/26 14:22:20 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -328,9 +328,8 @@ ssl3_accept(SSL *s) * stateless while listening. */ if (listen) { - memcpy(S3I(s)->write_sequence, - S3I(s)->read_sequence, - sizeof(S3I(s)->write_sequence)); + tls12_record_layer_reflect_seq_num( + s->internal->rl); } /* If we're just listening, stop here */ diff --git a/lib/libssl/t1_enc.c b/lib/libssl/t1_enc.c index 875aae36b03..a0b377389bd 100644 --- a/lib/libssl/t1_enc.c +++ b/lib/libssl/t1_enc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: t1_enc.c,v 1.129 2021/01/19 19:07:39 jsing Exp $ */ +/* $OpenBSD: t1_enc.c,v 1.130 2021/01/26 14:22:20 jsing Exp $ */ /* Copyright (C) 1995-1998 Eric Young (eay@cryptsoft.com) * All rights reserved. * @@ -494,7 +494,7 @@ tls1_change_cipher_state(SSL *s, int which) const unsigned char *client_write_iv, *server_write_iv; const unsigned char *mac_secret, *key, *iv; int mac_secret_size, key_len, iv_len; - unsigned char *key_block, *seq; + unsigned char *key_block; const EVP_CIPHER *cipher; const EVP_AEAD *aead; char is_read, use_client_keys; @@ -517,15 +517,6 @@ tls1_change_cipher_state(SSL *s, int which) use_client_keys = ((which == SSL3_CHANGE_CIPHER_CLIENT_WRITE) || (which == SSL3_CHANGE_CIPHER_SERVER_READ)); - /* - * Reset sequence number to zero - for DTLS this is handled in - * dtls1_reset_seq_numbers(). - */ - if (!SSL_is_dtls(s)) { - seq = is_read ? S3I(s)->read_sequence : S3I(s)->write_sequence; - memset(seq, 0, SSL3_SEQUENCE_SIZE); - } - if (aead != NULL) { key_len = EVP_AEAD_key_length(aead); iv_len = SSL_CIPHER_AEAD_FIXED_NONCE_LEN(S3I(s)->hs.new_cipher); @@ -569,14 +560,10 @@ tls1_change_cipher_state(SSL *s, int which) if (!tls12_record_layer_change_read_cipher_state(s->internal->rl, mac_secret, mac_secret_size, key, key_len, iv, iv_len)) goto err; - tls12_record_layer_set_read_seq_num(s->internal->rl, - S3I(s)->read_sequence); } else { if (!tls12_record_layer_change_write_cipher_state(s->internal->rl, mac_secret, mac_secret_size, key, key_len, iv, iv_len)) goto err; - tls12_record_layer_set_write_seq_num(s->internal->rl, - S3I(s)->write_sequence); } if (aead != NULL) { diff --git a/lib/libssl/tls12_record_layer.c b/lib/libssl/tls12_record_layer.c index b45a625fd48..01044432863 100644 --- a/lib/libssl/tls12_record_layer.c +++ b/lib/libssl/tls12_record_layer.c @@ -1,4 +1,4 @@ -/* $OpenBSD: tls12_record_layer.c,v 1.14 2021/01/20 07:05:25 tb Exp $ */ +/* $OpenBSD: tls12_record_layer.c,v 1.15 2021/01/26 14:22:20 jsing Exp $ */ /* * Copyright (c) 2020 Joel Sing * @@ -23,6 +23,7 @@ struct tls12_record_protection { uint16_t epoch; + uint8_t seq_num[SSL3_SEQUENCE_SIZE]; int stream_mac; @@ -37,8 +38,6 @@ struct tls12_record_protection { EVP_CIPHER_CTX *cipher_ctx; EVP_MD_CTX *hash_ctx; - - uint8_t *seq_num; }; static struct tls12_record_protection * @@ -47,13 +46,23 @@ tls12_record_protection_new(void) return calloc(1, sizeof(struct tls12_record_protection)); } +static void +tls12_record_protection_clear(struct tls12_record_protection *rp) +{ + memset(rp->seq_num, 0, sizeof(rp->seq_num)); + + freezero(rp->mac_key, rp->mac_key_len); + rp->mac_key = NULL; + rp->mac_key_len = 0; +} + static void tls12_record_protection_free(struct tls12_record_protection *rp) { if (rp == NULL) return; - freezero(rp->mac_key, rp->mac_key_len); + tls12_record_protection_clear(rp); freezero(rp, sizeof(struct tls12_record_protection)); } @@ -294,32 +303,24 @@ void tls12_record_layer_clear_read_state(struct tls12_record_layer *rl) { tls12_record_layer_set_read_state(rl, NULL, NULL, NULL, 0); - tls12_record_layer_set_read_mac_key(rl, NULL, 0); - rl->read->seq_num = NULL; + tls12_record_protection_clear(rl->read); } void tls12_record_layer_clear_write_state(struct tls12_record_layer *rl) { tls12_record_layer_set_write_state(rl, NULL, NULL, NULL, 0); - rl->write->seq_num = NULL; + tls12_record_protection_clear(rl->write); tls12_record_protection_free(rl->write_previous); rl->write_previous = NULL; } void -tls12_record_layer_set_read_seq_num(struct tls12_record_layer *rl, - uint8_t *seq_num) +tls12_record_layer_reflect_seq_num(struct tls12_record_layer *rl) { - rl->read->seq_num = seq_num; -} - -void -tls12_record_layer_set_write_seq_num(struct tls12_record_layer *rl, - uint8_t *seq_num) -{ - rl->write->seq_num = seq_num; + memcpy(rl->write->seq_num, rl->read->seq_num, + sizeof(rl->write->seq_num)); } int @@ -391,6 +392,8 @@ tls12_record_layer_change_read_cipher_state(struct tls12_record_layer *rl, if ((read_new = tls12_record_protection_new()) == NULL) goto err; + /* Read sequence number gets reset to zero. */ + /* XXX - change cipher state. */ tls12_record_protection_free(rl->read_current); @@ -416,6 +419,8 @@ tls12_record_layer_change_write_cipher_state(struct tls12_record_layer *rl, if ((write_new = tls12_record_protection_new()) == NULL) goto err; + /* Write sequence number gets reset to zero. */ + /* XXX - change cipher state. */ if (rl->dtls) { @@ -434,6 +439,7 @@ tls12_record_layer_change_write_cipher_state(struct tls12_record_layer *rl, return ret; } + static int tls12_record_layer_build_seq_num(struct tls12_record_layer *rl, CBB *cbb, uint16_t epoch, uint8_t *seq_num, size_t seq_num_len) @@ -896,7 +902,7 @@ tls12_record_layer_open_record(struct tls12_record_layer *rl, uint8_t *buf, uint8_t content_type; CBS_init(&cbs, buf, buf_len); - CBS_init(&seq_num, rl->read->seq_num, SSL3_SEQUENCE_SIZE); + CBS_init(&seq_num, rl->read->seq_num, sizeof(rl->read->seq_num)); if (!CBS_get_u8(&cbs, &content_type)) return 0; @@ -912,6 +918,9 @@ tls12_record_layer_open_record(struct tls12_record_layer *rl, uint8_t *buf, */ if (!CBS_get_bytes(&cbs, &seq_num, SSL3_SEQUENCE_SIZE)) return 0; + if (!CBS_write_bytes(&seq_num, rl->read->seq_num, + sizeof(rl->read->seq_num), NULL)) + return 0; } if (!CBS_get_u16_length_prefixed(&cbs, &fragment)) return 0; @@ -1096,7 +1105,7 @@ tls12_record_layer_seal_record(struct tls12_record_layer *rl, if (!CBB_init(&seq_num_cbb, SSL3_SEQUENCE_SIZE)) goto err; if (!tls12_record_layer_build_seq_num(rl, &seq_num_cbb, rl->write->epoch, - rl->write->seq_num, SSL3_SEQUENCE_SIZE)) + rl->write->seq_num, sizeof(rl->write->seq_num))) goto err; if (!CBB_finish(&seq_num_cbb, &seq_num_data, &seq_num_len)) goto err;