Skip to content

Commit

Permalink
Fix the endianness of DTLS 1.3 ChaCha20 record number encryption
Browse files Browse the repository at this point in the history
The spec is a little bit unclear, but it passes the counter portion as
bytes:

>  Mask = ChaCha20(sn_key, Ciphertext[0..3], Ciphertext[4..15])

And then RFC 8439 says:

>  A 32-bit block count parameter, treated as a 32-bit little-endian
>  integer.

So I believe this means that, formally, the block count parameter is a
[4]uint8, not a uint32, and then ChaCha20 internally reads it as
little-endian. Our API takes a uint32, so it is the caller's
responsibility to pick little-endian.

This also matches the QUIC construction.

While I'm here, avoid an unnecessary two-byte allocation on every DTLS
1.3 record decryption. Functions like these generally can generally work
in-place.

Bug: 42290594
Change-Id: I879944ca533d37a1599d2170a00193caecd01f42
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71547
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
  • Loading branch information
davidben authored and Boringssl LUCI CQ committed Oct 1, 2024
1 parent 81345b8 commit 0eda639
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 14 deletions.
5 changes: 5 additions & 0 deletions ssl/dtls_record.cc
Original file line number Diff line number Diff line change
Expand Up @@ -528,8 +528,13 @@ bool dtls_seal_record(SSL *ssl, uint8_t *out, size_t *out_len, size_t max_out,
// |0|0|1|0|1|1|E E|
// +-+-+-+-+-+-+-+-+
out[0] = 0x2c | (epoch & 0x3);
// We always use a two-byte sequence number. A one-byte sequence number
// would require coordinating with the application on ACK feedback to know
// that the peer is not too far behind.
out[1] = *seq >> 8;
out[2] = *seq & 0xff;
// TODO(crbug.com/42290594): When we know the record is last in the packet,
// 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.
Expand Down
13 changes: 5 additions & 8 deletions ssl/ssl_aead_ctx.cc
Original file line number Diff line number Diff line change
Expand Up @@ -483,20 +483,17 @@ bool ChaChaRecordNumberEncrypter::SetKey(Span<const uint8_t> key) {

bool ChaChaRecordNumberEncrypter::GenerateMask(Span<uint8_t> out,
Span<const uint8_t> sample) {
Array<uint8_t> zeroes;
if (!zeroes.Init(out.size())) {
return false;
}
OPENSSL_memset(zeroes.data(), 0, zeroes.size());
// RFC 9147 section 4.2.3 uses the first 4 bytes of the sample as the counter
// and the next 12 bytes as the nonce. If we have less than 4+12=16 bytes in
// the sample, then we'll read past the end of the |sample| buffer.
// the sample, then we'll read past the end of the |sample| buffer. The
// counter is interpreted as little-endian per RFC 8439.
if (sample.size() < 16) {
return false;
}
uint32_t counter = CRYPTO_load_u32_be(sample.data());
uint32_t counter = CRYPTO_load_u32_le(sample.data());
Span<const uint8_t> nonce = sample.subspan(4);
CRYPTO_chacha_20(out.data(), zeroes.data(), zeroes.size(), key_, nonce.data(),
OPENSSL_memset(out.data(), 0, out.size());
CRYPTO_chacha_20(out.data(), out.data(), out.size(), key_, nonce.data(),
counter);
return true;
}
Expand Down
10 changes: 4 additions & 6 deletions ssl/test/runner/conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -776,20 +776,18 @@ func newChachaRecordNumberEncrypter(key []byte) *chachaRecordNumberEncrypter {
}

func (c *chachaRecordNumberEncrypter) generateMask(sample []byte) []byte {
var counter uint32
nonce := make([]byte, 12)
var counter, nonce []byte
sampleReader := cryptobyte.String(sample)
if !sampleReader.ReadUint32(&counter) || !sampleReader.CopyBytes(nonce) {
if !sampleReader.ReadBytes(&counter, 4) || !sampleReader.ReadBytes(&nonce, 12) {
panic("chachaRecordNumberEncrypter.GenerateMask called with wrong size sample")
}
cipher, err := chacha20.NewUnauthenticatedCipher(c.key, nonce)
if err != nil {
panic("Failed to create chacha20 cipher for record number encryption")
}
cipher.SetCounter(counter)
zeroes := make([]byte, 2)
cipher.SetCounter(binary.LittleEndian.Uint32(counter))
out := make([]byte, 2)
cipher.XORKeyStream(out, zeroes)
cipher.XORKeyStream(out, out)
return out
}

Expand Down

0 comments on commit 0eda639

Please sign in to comment.