OERV-RUNTIME/boringssl
2
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Return the record number out of the DTLS record layer

Browse Source 
For now this is ignored by the caller, but ACK management will require
this value. To do this, I've introduced a DTLSRecordNumber type. This
isn't quite used in all the places that we could use, so I've left a
TODO about this.

As part of this, also fix some of the weirdness where some code would
mix up seq and epoch+seq across 1.2 and 1.3.

Fixing that up also revealed that actually we'd accidentally changed
SSL_get_read_sequence slightly in the epoch state refactor. This CL ends
up tweaking it slightly more in the DTLS 1.3 case (which doesn't work
anyway). I've updated the TODO comment accordingly.

Bug: 42290594
Change-Id: Ie7e7da02c432a137ae2314fb3557d1c3717952f3
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/72271
Reviewed-by: Nick Harper <nharper@chromium.org>
Commit-Queue: David Benjamin <davidben@google.com>
This commit is contained in:
David Benjamin
2024-10-24 22:32:21 +00:00
committed by Boringssl LUCI CQ
parent 781a72b2aa
commit 659da7c679
6 changed files with 120 additions and 70 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ssl/d1_both.cc
+9 -4
View File
@@ -412,8 +412,10 @@ bool dtls1_process_handshake_fragments(SSL *ssl, uint8_t *out_alert,
ssl_open_record_t dtls1_open_handshake(SSL *ssl, size_t *out_consumed,
uint8_t *out_alert, Span<uint8_t> in) {
uint8_t type;
DTLSRecordNumber record_number;
Span<uint8_t> record;
auto ret = dtls_open_record(ssl, &type, &record, out_consumed, out_alert, in);
auto ret = dtls_open_record(ssl, &type, &record_number, &record, out_consumed,
out_alert, in);
if (ret != ssl_open_record_success) {
return ret;
}
@@ -709,7 +711,8 @@ static enum seal_result_t seal_next_message(SSL *ssl, uint8_t *out,
return seal_no_progress;
}
if (!dtls_seal_record(ssl, out, out_len, max_out,
DTLSRecordNumber record_number;
if (!dtls_seal_record(ssl, &record_number, out, out_len, max_out,
SSL3_RT_CHANGE_CIPHER_SPEC, kChangeCipherSpec,
sizeof(kChangeCipherSpec), msg->epoch)) {
return seal_error;
@@ -763,8 +766,10 @@ static enum seal_result_t seal_next_message(SSL *ssl, uint8_t *out,
ssl_do_msg_callback(ssl, 1 /* write */, SSL3_RT_HANDSHAKE,
MakeSpan(frag, frag_len));
if (!dtls_seal_record(ssl, out, out_len, max_out, SSL3_RT_HANDSHAKE,
out + prefix, frag_len, msg->epoch)) {
DTLSRecordNumber record_number;
if (!dtls_seal_record(ssl, &record_number, out, out_len, max_out,
SSL3_RT_HANDSHAKE, out + prefix, frag_len,
msg->epoch)) {
return seal_error;
}
ssl/d1_pkt.cc
+7 -4
View File
@@ -149,8 +149,10 @@ ssl_open_record_t dtls1_open_app_data(SSL *ssl, Span<uint8_t> *out,
assert(!SSL_in_init(ssl));
uint8_t type;
DTLSRecordNumber record_number;
Span<uint8_t> record;
auto ret = dtls_open_record(ssl, &type, &record, out_consumed, out_alert, in);
auto ret = dtls_open_record(ssl, &type, &record_number, &record, out_consumed,
out_alert, in);
if (ret != ssl_open_record_success) {
return ret;
}
@@ -259,12 +261,13 @@ int dtls1_write_record(SSL *ssl, int type, Span<const uint8_t> in,
return -1;
}
DTLSRecordNumber record_number;
size_t ciphertext_len;
if (!buf->EnsureCap(dtls_seal_prefix_len(ssl, epoch),
in.size() + SSL_max_seal_overhead(ssl)) ||
!dtls_seal_record(ssl, buf->remaining().data(), &ciphertext_len,
buf->remaining().size(), type, in.data(), in.size(),
epoch)) {
!dtls_seal_record(ssl, &record_number, buf->remaining().data(),
&ciphertext_len, buf->remaining().size(), type,
in.data(), in.size(), epoch)) {
buf->Clear();
return -1;
}
ssl/dtls_record.cc
+39 -31
View File
@@ -123,8 +123,6 @@
BSSL_NAMESPACE_BEGIN
static constexpr uint64_t kMaxSequenceNumber = (uint64_t{1} << 48) - 1;
bool DTLSReplayBitmap::ShouldDiscard(uint64_t seq_num) const {
const size_t kWindowSize = map_.size();
@@ -167,6 +165,15 @@ static uint16_t dtls_record_version(const SSL *ssl) {
: ssl->s3->version;
}
static uint64_t dtls_aead_sequence(const SSL *ssl, DTLSRecordNumber num) {
// DTLS 1.3 uses the sequence number with the AEAD, while DTLS 1.2 uses the
// combined value. If the version is not known, the epoch is unencrypted and
// the value is ignored.
return (ssl->s3->version != 0 && ssl_protocol_version(ssl) >= TLS1_3_VERSION)
? num.sequence()
: num.combined();
}
// reconstruct_epoch finds the largest epoch that ends with the epoch bits from
// |wire_epoch| that is less than or equal to |current_epoch|, to match the
// epoch reconstruction algorithm described in RFC 9147 section 4.2.2.
@@ -186,7 +193,7 @@ uint64_t reconstruct_seqnum(uint16_t wire_seq, uint64_t seq_mask,
// bytes), no payload, and 16 byte AEAD overhead, sending 2^48 records would
// require 5 petabytes. This allows us to continue to pack a DTLS record
// number into an 8-byte structure.
assert(max_valid_seqnum <= kMaxSequenceNumber);
assert(max_valid_seqnum <= DTLSRecordNumber::kMaxSequence);
assert(seq_mask == 0xff || seq_mask == 0xffff);
uint64_t max_seqnum_plus_one = max_valid_seqnum + 1;
@@ -195,7 +202,7 @@ uint64_t reconstruct_seqnum(uint16_t wire_seq, uint64_t seq_mask,
// This addition cannot overflow. It is at most 2^48 + seq_mask. It, however,
// may exceed 2^48-1.
uint64_t seqnum = max_seqnum_plus_one + diff;
bool too_large = seqnum > kMaxSequenceNumber;
bool too_large = seqnum > DTLSRecordNumber::kMaxSequence;
// If the diff is larger than half the step size, then the closest seqnum
// to max_seqnum_plus_one (in Z_{2^64}) is seqnum minus step instead of
// seqnum.
@@ -205,7 +212,7 @@ uint64_t reconstruct_seqnum(uint16_t wire_seq, uint64_t seq_mask,
if (too_large || (closer_is_less && !would_underflow)) {
seqnum -= step;
}
assert(seqnum <= kMaxSequenceNumber);
assert(seqnum <= DTLSRecordNumber::kMaxSequence);
return seqnum;
}
@@ -215,11 +222,9 @@ static Span<uint8_t> cbs_to_writable_bytes(CBS cbs) {
struct ParsedDTLSRecord {
// read_epoch will be null if the record is for an unrecognized epoch. In that
// case, sequence may be zero if we are unable to decrypt the sequence number.
// case, |number| may be unset.
DTLSReadEpoch *read_epoch = nullptr;
// sequence includes the epoch for DTLS 1.2 records and does not include it
// for DTLS 1.3.
uint64_t sequence = 0;
DTLSRecordNumber number;
CBS header, body;
uint8_t type = 0;
uint16_t version = 0;
@@ -282,21 +287,24 @@ static bool parse_dtls13_record(SSL *ssl, CBS *in, ParsedDTLSRecord *out) {
writable_seq[i] ^= mask[i];
seq = (seq << 8) | writable_seq[i];
}
out->sequence = reconstruct_seqnum(seq, (1 << (seq_len * 8)) - 1,
out->read_epoch->bitmap.max_seq_num());
uint64_t full_seq = reconstruct_seqnum(
seq, (1 << (seq_len * 8)) - 1, out->read_epoch->bitmap.max_seq_num());
out->number = DTLSRecordNumber(epoch, full_seq);
}
return true;
}
static bool parse_dtls12_record(SSL *ssl, CBS *in, ParsedDTLSRecord *out) {
uint64_t epoch_and_seq;
if (!CBS_get_u16(in, &out->version) || //
!CBS_get_u64(in, &out->sequence) ||
!CBS_get_u64(in, &epoch_and_seq) ||
!CBS_get_u16_length_prefixed(in, &out->body)) {
return false;
}
out->number = DTLSRecordNumber::FromCombined(epoch_and_seq);
uint16_t epoch = static_cast<uint16_t>(out->sequence >> 48);
uint16_t epoch = out->number.epoch();
bool version_ok;
if (epoch == 0) {
// Only check the first byte. Enforcing beyond that can prevent decoding
@@ -345,6 +353,7 @@ static bool parse_dtls_record(SSL *ssl, CBS *cbs, ParsedDTLSRecord *out) {
}
enum ssl_open_record_t dtls_open_record(SSL *ssl, uint8_t *out_type,
DTLSRecordNumber *out_number,
Span<uint8_t> *out,
size_t *out_consumed,
uint8_t *out_alert, Span<uint8_t> in) {
@@ -368,7 +377,7 @@ enum ssl_open_record_t dtls_open_record(SSL *ssl, uint8_t *out_type,
ssl_do_msg_callback(ssl, 0 /* read */, SSL3_RT_HEADER, record.header);
if (record.read_epoch == nullptr ||
record.read_epoch->bitmap.ShouldDiscard(record.sequence)) {
record.read_epoch->bitmap.ShouldDiscard(record.number.sequence())) {
// Drop this record. It's from an unknown epoch or is a replay. Note that if
// the record is from next epoch, it could be buffered for later. For
// simplicity, drop it and expect retransmit to handle it later; DTLS must
@@ -377,9 +386,10 @@ enum ssl_open_record_t dtls_open_record(SSL *ssl, uint8_t *out_type,
return ssl_open_record_discard;
}
// discard the body in-place.
// Decrypt the body in-place.
if (!record.read_epoch->aead->Open(out, record.type, record.version,
record.sequence, record.header,
dtls_aead_sequence(ssl, record.number),
record.header,
cbs_to_writable_bytes(record.body))) {
// Bad packets are silently dropped in DTLS. See section 4.2.1 of RFC 6347.
// Clear the error queue of any errors decryption may have added. Drop the
@@ -416,7 +426,7 @@ enum ssl_open_record_t dtls_open_record(SSL *ssl, uint8_t *out_type,
} while (record.type == 0);
}
record.read_epoch->bitmap.Record(record.sequence);
record.read_epoch->bitmap.Record(record.number.sequence());
// TODO(davidben): Limit the number of empty records as in TLS? This is only
// useful if we also limit discarded packets.
@@ -428,6 +438,7 @@ enum ssl_open_record_t dtls_open_record(SSL *ssl, uint8_t *out_type,
ssl->s3->warning_alert_count = 0;
*out_type = record.type;
*out_number = record.number;
return ssl_open_record_success;
}
@@ -479,9 +490,9 @@ size_t dtls_seal_prefix_len(const SSL *ssl, uint16_t epoch) {
write_epoch->aead->ExplicitNonceLen();
}
bool dtls_seal_record(SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
uint8_t type, const uint8_t *in, size_t in_len,
uint16_t epoch) {
bool dtls_seal_record(SSL *ssl, DTLSRecordNumber *out_number, uint8_t *out,
size_t *out_len, size_t max_out, uint8_t type,
const uint8_t *in, size_t in_len, uint16_t epoch) {
const size_t prefix = dtls_seal_prefix_len(ssl, epoch);
if (buffers_alias(in, in_len, out, max_out) &&
(max_out < prefix || out + prefix != in)) {
@@ -499,7 +510,7 @@ bool dtls_seal_record(SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
const size_t record_header_len = dtls_record_header_write_len(ssl, epoch);
// Ensure the sequence number update does not overflow.
if (write_epoch->next_seq + 1 > kMaxSequenceNumber) {
if (write_epoch->next_seq + 1 > DTLSRecordNumber::kMaxSequence) {
OPENSSL_PUT_ERROR(SSL, ERR_R_OVERFLOW);
return false;
}
@@ -524,7 +535,7 @@ bool dtls_seal_record(SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
}
uint16_t record_version = dtls_record_version(ssl);
uint64_t aead_seq;
DTLSRecordNumber record_number(epoch, write_epoch->next_seq);
if (dtls13_header) {
// The first byte of the DTLS 1.3 record header has the following format:
// 0 1 2 3 4 5 6 7
@@ -549,25 +560,21 @@ bool dtls_seal_record(SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
// omit the length.
out[3] = ciphertext_len >> 8;
out[4] = ciphertext_len & 0xff;
// DTLS 1.3 uses the sequence number without the epoch for the AEAD.
aead_seq = write_epoch->next_seq;
} else {
out[0] = type;
out[1] = record_version >> 8;
out[2] = record_version & 0xff;
// DTLS 1.2 uses the sequence number with the epoch for the AEAD.
aead_seq = (uint64_t{epoch} << 48) | write_epoch->next_seq;
CRYPTO_store_u64_be(&out[3], aead_seq);
CRYPTO_store_u64_be(&out[3], record_number.combined());
out[11] = ciphertext_len >> 8;
out[12] = ciphertext_len & 0xff;
}
Span<const uint8_t> header = MakeConstSpan(out, record_header_len);
if (!write_epoch->aead->SealScatter(out + record_header_len, out + prefix,
out + prefix + in_len, type,
record_version, aead_seq, header, in,
in_len, extra_in, extra_in_len)) {
if (!write_epoch->aead->SealScatter(
out + record_header_len, out + prefix, out + prefix + in_len, type,
record_version, dtls_aead_sequence(ssl, record_number), header, in,
in_len, extra_in, extra_in_len)) {
return false;
}
@@ -587,6 +594,7 @@ bool dtls_seal_record(SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
out[2] ^= mask[1];
}
*out_number = record_number;
write_epoch->next_seq++;
*out_len = record_header_len + ciphertext_len;
ssl_do_msg_callback(ssl, 1 /* write */, SSL3_RT_HEADER, header);
ssl/internal.h
+33 -5
View File
@@ -1231,6 +1231,31 @@ OPENSSL_EXPORT uint64_t reconstruct_seqnum(uint16_t wire_seq, uint64_t seq_mask,
// Record layer.
// TODO(davidben): Use this type more extensively in the epoch state.
class DTLSRecordNumber {
public:
static constexpr uint64_t kMaxSequence = (uint64_t{1} << 48) - 1;
DTLSRecordNumber() = default;
DTLSRecordNumber(uint16_t epoch, uint64_t sequence) {
BSSL_CHECK(sequence <= kMaxSequence);
combined_ = (uint64_t{epoch} << 48) | sequence;
}
static DTLSRecordNumber FromCombined(uint64_t combined) {
return DTLSRecordNumber(combined);
}
uint64_t combined() const { return combined_; }
uint16_t epoch() const { return combined_ >> 48; }
uint64_t sequence() const { return combined_ & kMaxSequence; }
private:
explicit DTLSRecordNumber(uint64_t combined) : combined_(combined) {}
uint64_t combined_ = 0;
};
class RecordNumberEncrypter {
public:
static constexpr bool kAllowUniquePtr = true;
@@ -1309,8 +1334,10 @@ enum ssl_open_record_t tls_open_record(SSL *ssl, uint8_t *out_type,
// dtls_open_record implements |tls_open_record| for DTLS. It only returns
// |ssl_open_record_partial| if |in| was empty and sets |*out_consumed| to
// zero. The caller should read one packet and try again.
// zero. The caller should read one packet and try again. On success,
// |*out_number| is set to the record number of the record.
enum ssl_open_record_t dtls_open_record(SSL *ssl, uint8_t *out_type,
DTLSRecordNumber *out_number,
Span<uint8_t> *out,
size_t *out_consumed,
uint8_t *out_alert, Span<uint8_t> in);
@@ -1348,10 +1375,11 @@ size_t dtls_seal_prefix_len(const SSL *ssl, uint16_t epoch);
// dtls_seal_record implements |tls_seal_record| for DTLS. |epoch| selects which
// epoch's cipher state to use. Unlike |tls_seal_record|, |in| and |out| may
// alias but, if they do, |in| must be exactly |dtls_seal_prefix_len| bytes
// ahead of |out|.
bool dtls_seal_record(SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
uint8_t type, const uint8_t *in, size_t in_len,
uint16_t epoch);
// ahead of |out|. On success, |*out_number| is set to the record number of the
// record.
bool dtls_seal_record(SSL *ssl, DTLSRecordNumber *out_number, uint8_t *out,
size_t *out_len, size_t max_out, uint8_t type,
const uint8_t *in, size_t in_len, uint16_t epoch);
// ssl_process_alert processes |in| as an alert and updates |ssl|'s shutdown
// state. It returns one of |ssl_open_record_discard|, |ssl_open_record_error|,
ssl/ssl_lib.cc
+19 -19
View File
@@ -2952,20 +2952,24 @@ int SSL_get_ivs(const SSL *ssl, const uint8_t **out_read_iv,
uint64_t SSL_get_read_sequence(const SSL *ssl) {
if (SSL_is_dtls(ssl)) {
// TODO(crbug.com/42290608): The API for read sequences in DTLS 1.3 needs to
// reworked. In DTLS 1.3, the read epoch is updated once new keys are
// derived (before we receive a message encrypted with those keys), which
// results in the read epoch being ahead of the highest record received.
// Additionally, when we process a KeyUpdate, we will install new read keys
// for the new epoch, but we may receive messages from the old epoch for
// some time if the ACK gets lost or there is reordering.
// max_seq_num already includes the epoch. However, the current epoch may
// be one ahead of the highest record received, immediately after a key
// change.
// TODO(crbug.com/42290608): This API needs to reworked. Right at an epoch
// transition, it is possible that |read_epoch| has not received any
// records. We will then return that sequence 0 is the highest received, but
// this is not quite right.
//
// This is mostly moot in DTLS 1.2 because, after the handshake, we will
// never be in this state. In DTLS 1.3, there is a key transition
// immediately after the handshake, and in the steady state with KeyUpdate.
// While not yet implemented, DTLS 1.3 will handle key changes by having two
// epochs live at once (current and optional next), only cycling forward
// when we receive a record at the new epoch.
//
// When we implement this, this sequence 0 edge case will be gone, but
// replaced with a different issue: our record layer APIs have no way to
// report transition state. We'll likely need a new API for DTLS offload.
const DTLSReadEpoch *read_epoch = &ssl->d1->read_epoch;
assert(read_epoch->epoch >= read_epoch->bitmap.max_seq_num() >> 48);
return read_epoch->bitmap.max_seq_num();
return DTLSRecordNumber(read_epoch->epoch, read_epoch->bitmap.max_seq_num())
.combined();
}
return ssl->s3->read_sequence;
}
@@ -2973,12 +2977,8 @@ uint64_t SSL_get_read_sequence(const SSL *ssl) {
uint64_t SSL_get_write_sequence(const SSL *ssl) {
if (SSL_is_dtls(ssl)) {
const DTLSWriteEpoch *write_epoch = &ssl->d1->write_epoch;
uint64_t ret = write_epoch->next_seq;
if (SSL_is_dtls(ssl)) {
assert((ret >> 48) == 0);
ret |= uint64_t{write_epoch->epoch} << 48;
}
return ret;
return DTLSRecordNumber(write_epoch->epoch, write_epoch->next_seq)
.combined();
}
return ssl->s3->write_sequence;
ssl/ssl_test.cc
+13 -7
View File
@@ -2809,22 +2809,28 @@ TEST_P(SSLVersionTest, SequenceNumber) {
if (is_dtls()) {
if (version() == DTLS1_3_EXPERIMENTAL_VERSION) {
// Client and server write epochs should be at 3 (application data).
// Both client and server must be at epoch 3 (application data).
EXPECT_EQ(EpochFromSequence(client_read_seq), 3);
EXPECT_EQ(EpochFromSequence(client_write_seq), 3);
EXPECT_EQ(EpochFromSequence(server_read_seq), 3);
EXPECT_EQ(EpochFromSequence(server_write_seq), 3);
// TODO(crbug.com/42290608): The read sequences aren't checked because the
// SSL_get_read_sequence API needs to be reworked for DTLS 1.3.
// TODO(crbug.com/42290608): The next record to be written should exceed
// the largest received, but they'll actually be equal because the
// |SSL_get_read_sequence| API cannot represent DTLS key transitions.
EXPECT_GE(client_write_seq, server_read_seq);
EXPECT_GE(server_write_seq, client_read_seq);
} else {
// Both client and server must be at epoch 1.
EXPECT_EQ(EpochFromSequence(client_read_seq), 1);
EXPECT_EQ(EpochFromSequence(client_write_seq), 1);
EXPECT_EQ(EpochFromSequence(server_read_seq), 1);
EXPECT_EQ(EpochFromSequence(server_write_seq), 1);
}
// The next record to be written should exceed the largest received.
EXPECT_GT(client_write_seq, server_read_seq);
EXPECT_GT(server_write_seq, client_read_seq);
// The next record to be written should exceed the largest received.
EXPECT_GT(client_write_seq, server_read_seq);
EXPECT_GT(server_write_seq, client_read_seq);
}
} else {
// The next record to be written should equal the next to be received.
EXPECT_EQ(client_write_seq, server_read_seq);