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 8c4fec5..ba4e9ed 100644 (file)
--- 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 bde13c9..bbf2e8e 100644 (file)
--- 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 0537cf0..e0e0ae4 100644 (file)
--- 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 01f2ebb..4390361 100644 (file)
--- 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 000cac6..3551ee4 100644 (file)
--- 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 875aae3..a0b3773 100644 (file)
--- 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 b45a625..0104443 100644 (file)
--- 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 <jsing@openbsd.org>
  *
@@ -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;