Skip to content

Commit

Permalink
fixup! Adds DTLS 1.3 ACK message functionality
Browse files Browse the repository at this point in the history
  • Loading branch information
@fwh-dc
fwh-dc committed Aug 7, 2024
1 parent 60ac73b commit 15fc23d
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 44 deletions.
17 changes: 11 additions & 6 deletions doc/designs/dtlsv1_3/dtlsv1_3-main.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,17 @@ This is enforced by the macro `SSL_CONNECTION_MIDDLEBOX_IS_ENABLED(sc)`.
The DTLSv1.3 implementation uses the label "dtls1.3" as described by RFC9147
section 5.9.

#### DTLS ACK records (RFC9147 Section 7)

ACK's are sent for KeyUpdates, NewSessionTicket and Finish (client).

Notes on RFC9147 Section 7.1:
* The implementation does not offer any logic to determine that there is disruption
when receiving messages which means it will not send ACKs for the example given
in RFC9147 Figure 12.
* ACKs are always sent immediately after receiving a message to be ACK'ed.
* Empty ACKs are never sent.

Implementation progress
-----------------------

Expand All @@ -80,8 +91,6 @@ is not covered by these workitems and must be implemented separately.

| Summary | #PR |
|-----------------------------------------------------|----------------|
| ACK messages | - |
| Use HelloRetryRequest instead of HelloVerifyRequest | #22985, #22400 |
| Message transcript | - |
| DTLSv1.3 epoch | #23553 |
| ClientHello | #23320 |
Expand Down Expand Up @@ -149,10 +158,6 @@ random value:
> the EndOfEarlyData message is omitted both from the wire and the handshake
> transcript
#### ACK messages

See section 7 and 8 of RFC 9147.

### List of DTLSv1.3 requirements

Here's a list of requirements from RFC 9147 together with their implementation status
Expand Down
64 changes: 29 additions & 35 deletions ssl/record/methods/ssl3_meth.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,11 +218,22 @@ static const unsigned char ssl3_pad_2[48] = {
static int ssl3_mac(OSSL_RECORD_LAYER *rl, TLS_RL_RECORD *rec, unsigned char *md,
int sending)
{
/*
* npad is, at most, 48 bytes and that's with MD5:
* 16 + 48 + 8 (sequence bytes) + 1 + 2 = 75.
*
* With SHA-1 (the largest hash speced for SSLv3) the hash size
* goes up 4, but npad goes down by 8, resulting in a smaller
* total size.
*/
unsigned char header[75];
WPACKET hdr;
size_t hdr_written;
unsigned char *mac_sec;
const EVP_MD_CTX *hash;
size_t md_size;
size_t npad;
int t;
int t, cbc_encrypted;

mac_sec = &(rl->mac_secret[0]);
hash = rl->md_ctx;
Expand All @@ -233,9 +244,21 @@ static int ssl3_mac(OSSL_RECORD_LAYER *rl, TLS_RL_RECORD *rec, unsigned char *md
md_size = t;
npad = (48 / md_size) * md_size;

if (!sending
&& EVP_CIPHER_CTX_get_mode(rl->enc_ctx) == EVP_CIPH_CBC_MODE
&& ssl3_cbc_record_digest_supported(hash)) {
cbc_encrypted = !sending
&& EVP_CIPHER_CTX_get_mode(rl->enc_ctx) == EVP_CIPH_CBC_MODE
&& ssl3_cbc_record_digest_supported(hash);

if (!WPACKET_init_static_len(&hdr, header, sizeof(header), 0)
|| !WPACKET_memcpy(&hdr, rl->mac_secret, md_size)
|| !WPACKET_memcpy(&hdr, ssl3_pad_1, npad)
|| !WPACKET_put_bytes_u64(&hdr, rl->sequence)
|| (!cbc_encrypted && !WPACKET_put_bytes_u8(&hdr, rec->type))
|| !WPACKET_put_bytes_u16(&hdr, rec->length)
|| !WPACKET_finish(&hdr)
|| !WPACKET_get_total_written(&hdr, &hdr_written))
return 0;

if (cbc_encrypted) {
#ifdef OPENSSL_NO_DEPRECATED_3_0
return 0;
#else
Expand All @@ -245,25 +268,6 @@ static int ssl3_mac(OSSL_RECORD_LAYER *rl, TLS_RL_RECORD *rec, unsigned char *md
* are hashing because that gives an attacker a timing-oracle.
*/

/*-
* npad is, at most, 48 bytes and that's with MD5:
* 16 + 48 + 8 (sequence bytes) + 1 + 2 = 75.
*
* With SHA-1 (the largest hash speced for SSLv3) the hash size
* goes up 4, but npad goes down by 8, resulting in a smaller
* total size.
*/
unsigned char header[75];
WPACKET hdr;

if (!WPACKET_init_static_len(&hdr, header, 75, 0)
|| !WPACKET_memcpy(&hdr, rl->mac_secret, md_size)
|| !WPACKET_memcpy(&hdr, ssl3_pad_1, npad)
|| !WPACKET_put_bytes_u64(&hdr, rl->sequence)
|| !WPACKET_put_bytes_u16(&hdr, rec->length)
|| !WPACKET_finish(&hdr))
return 0;

/* Final param == is SSLv3 */
if (ssl3_cbc_digest_record(EVP_MD_CTX_get0_md(hash),
md, &md_size,
Expand All @@ -274,28 +278,18 @@ static int ssl3_mac(OSSL_RECORD_LAYER *rl, TLS_RL_RECORD *rec, unsigned char *md
#endif
} else {
unsigned int md_size_u;
unsigned char rec_char, seq[SEQ_NUM_SIZE];
unsigned char *p_seq = seq, *p_md = md;
/* Chop the digest off the end :-) */
EVP_MD_CTX *md_ctx = EVP_MD_CTX_new();

if (md_ctx == NULL)
return 0;

rec_char = rec->type;
s2n(rec->length, p_md);
l2n8(rl->sequence, p_seq);

if (EVP_MD_CTX_copy_ex(md_ctx, hash) <= 0
|| EVP_DigestUpdate(md_ctx, mac_sec, md_size) <= 0
|| EVP_DigestUpdate(md_ctx, ssl3_pad_1, npad) <= 0
|| EVP_DigestUpdate(md_ctx, seq, 8) <= 0
|| EVP_DigestUpdate(md_ctx, &rec_char, 1) <= 0
|| EVP_DigestUpdate(md_ctx, md, 2) <= 0
|| EVP_DigestUpdate(md_ctx, header, hdr_written) <= 0
|| EVP_DigestUpdate(md_ctx, rec->input, rec->length) <= 0
|| EVP_DigestFinal_ex(md_ctx, md, NULL) <= 0
|| EVP_MD_CTX_copy_ex(md_ctx, hash) <= 0
|| EVP_DigestUpdate(md_ctx, mac_sec, md_size) <= 0
|| EVP_DigestUpdate(md_ctx, rl->mac_secret, md_size) <= 0
|| EVP_DigestUpdate(md_ctx, ssl3_pad_2, npad) <= 0
|| EVP_DigestUpdate(md_ctx, md, md_size) <= 0
|| EVP_DigestFinal_ex(md_ctx, md, &md_size_u) <= 0) {
Expand Down
5 changes: 2 additions & 3 deletions ssl/statem/statem_dtls.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1272,12 +1272,11 @@ int dtls1_retransmit_sent_messages(SSL_CONNECTION *s)
dtls_sent_msg *sent_msg = (dtls_sent_msg *)item->data;

if (SSL_CONNECTION_IS_DTLS13(s)) {
size_t idx = 0;
int ack_count = 0;
size_t idx = 0, ack_count = 0;

if (sent_msg->rec_nums_idx > 0) {
do {
ack_count += sent_msg->rec_nums[idx].acknowledged;
ack_count += (size_t)sent_msg->rec_nums[idx].acknowledged;
} while (++idx < sent_msg->rec_nums_idx);
}

Expand Down

0 comments on commit 15fc23d

Please sign in to comment.