From 81345b84505e9c23c156b2c7a1e655a204bd3e9a Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Mon, 30 Sep 2024 11:16:33 -0400 Subject: [PATCH 1/2] Bump module version and Bazel dependencies Not sure what cadence we'll settle to for these snapshots, but let's kick off another one. I've gone ahead and updated the dependencies from https://registry.bazel.build, because it seemed natural to just do those at the same time. Change-Id: Ifa609a80c110a1cbf196f9f1a23fb8d533b029f9 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71667 Reviewed-by: Bob Beck Commit-Queue: Bob Beck Auto-Submit: David Benjamin --- MODULE.bazel | 4 ++-- MODULE.bazel.lock | 14 ++++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/MODULE.bazel b/MODULE.bazel index 69a29fc26..de6b1a5b4 100644 --- a/MODULE.bazel +++ b/MODULE.bazel @@ -16,7 +16,7 @@ # the revision where we bump the version. module( name = "boringssl", - version = "0.20240913.0", + version = "0.20240930.0", compatibility_level = 2, ) @@ -33,5 +33,5 @@ module( bazel_dep(name = "googletest", version = "1.15.2") bazel_dep(name = "platforms", version = "0.0.10") -bazel_dep(name = "rules_cc", version = "0.0.9") +bazel_dep(name = "rules_cc", version = "0.0.10") bazel_dep(name = "rules_license", version = "1.0.0") diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index 38fdcfe47..0af39b5b8 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -22,7 +22,8 @@ "https://bcr.bazel.build/modules/bazel_skylib/1.4.1/MODULE.bazel": "a0dcb779424be33100dcae821e9e27e4f2901d9dfd5333efe5ac6a8d7ab75e1d", "https://bcr.bazel.build/modules/bazel_skylib/1.5.0/MODULE.bazel": "32880f5e2945ce6a03d1fbd588e9198c0a959bb42297b2cfaf1685b7bc32e138", "https://bcr.bazel.build/modules/bazel_skylib/1.6.1/MODULE.bazel": "8fdee2dbaace6c252131c00e1de4b165dc65af02ea278476187765e1a617b917", - "https://bcr.bazel.build/modules/bazel_skylib/1.6.1/source.json": "082ed5f9837901fada8c68c2f3ddc958bb22b6d654f71dd73f3df30d45d4b749", + "https://bcr.bazel.build/modules/bazel_skylib/1.7.1/MODULE.bazel": "3120d80c5861aa616222ec015332e5f8d3171e062e3e804a2a0253e1be26e59b", + "https://bcr.bazel.build/modules/bazel_skylib/1.7.1/source.json": "f121b43eeefc7c29efbd51b83d08631e2347297c95aac9764a701f2a6a2bb953", "https://bcr.bazel.build/modules/buildozer/7.1.2/MODULE.bazel": "2e8dd40ede9c454042645fd8d8d0cd1527966aa5c919de86661e62953cd73d84", "https://bcr.bazel.build/modules/buildozer/7.1.2/source.json": "c9028a501d2db85793a6996205c8de120944f50a0d570438fcae0457a5f9d1f8", "https://bcr.bazel.build/modules/google_benchmark/1.8.2/MODULE.bazel": "a70cf1bba851000ba93b58ae2f6d76490a9feb74192e57ab8e8ff13c34ec50cb", @@ -51,17 +52,20 @@ "https://bcr.bazel.build/modules/re2/2024-07-02/MODULE.bazel": "0eadc4395959969297cbcf31a249ff457f2f1d456228c67719480205aa306daa", "https://bcr.bazel.build/modules/re2/2024-07-02/source.json": "547d0111a9d4f362db32196fef805abbf3676e8d6afbe44d395d87816c1130ca", "https://bcr.bazel.build/modules/rules_cc/0.0.1/MODULE.bazel": "cb2aa0747f84c6c3a78dad4e2049c154f08ab9d166b1273835a8174940365647", + "https://bcr.bazel.build/modules/rules_cc/0.0.10/MODULE.bazel": "ec1705118f7eaedd6e118508d3d26deba2a4e76476ada7e0e3965211be012002", + "https://bcr.bazel.build/modules/rules_cc/0.0.10/source.json": "90310b16e0e7df0cf40f8d1dccd7d373360f42419a6bfbbf5bb013182dd70e84", "https://bcr.bazel.build/modules/rules_cc/0.0.2/MODULE.bazel": "6915987c90970493ab97393024c156ea8fb9f3bea953b2f3ec05c34f19b5695c", "https://bcr.bazel.build/modules/rules_cc/0.0.6/MODULE.bazel": "abf360251023dfe3efcef65ab9d56beefa8394d4176dd29529750e1c57eaa33f", "https://bcr.bazel.build/modules/rules_cc/0.0.8/MODULE.bazel": "964c85c82cfeb6f3855e6a07054fdb159aced38e99a5eecf7bce9d53990afa3e", "https://bcr.bazel.build/modules/rules_cc/0.0.9/MODULE.bazel": "836e76439f354b89afe6a911a7adf59a6b2518fafb174483ad78a2a2fde7b1c5", - "https://bcr.bazel.build/modules/rules_cc/0.0.9/source.json": "1f1ba6fea244b616de4a554a0f4983c91a9301640c8fe0dd1d410254115c8430", "https://bcr.bazel.build/modules/rules_foreign_cc/0.9.0/MODULE.bazel": "c9e8c682bf75b0e7c704166d79b599f93b72cfca5ad7477df596947891feeef6", "https://bcr.bazel.build/modules/rules_java/4.0.0/MODULE.bazel": "5a78a7ae82cd1a33cef56dc578c7d2a46ed0dca12643ee45edbb8417899e6f74", + "https://bcr.bazel.build/modules/rules_java/7.6.1/MODULE.bazel": "2f14b7e8a1aa2f67ae92bc69d1ec0fa8d9f827c4e17ff5e5f02e91caa3b2d0fe", "https://bcr.bazel.build/modules/rules_java/7.6.5/MODULE.bazel": "481164be5e02e4cab6e77a36927683263be56b7e36fef918b458d7a8a1ebadb1", "https://bcr.bazel.build/modules/rules_java/7.6.5/source.json": "a805b889531d1690e3c72a7a7e47a870d00323186a9904b36af83aa3d053ee8d", "https://bcr.bazel.build/modules/rules_jvm_external/4.4.2/MODULE.bazel": "a56b85e418c83eb1839819f0b515c431010160383306d13ec21959ac412d2fe7", - "https://bcr.bazel.build/modules/rules_jvm_external/4.4.2/source.json": "a075731e1b46bc8425098512d038d416e966ab19684a10a34f4741295642fc35", + "https://bcr.bazel.build/modules/rules_jvm_external/5.2/MODULE.bazel": "d9351ba35217ad0de03816ef3ed63f89d411349353077348a45348b096615036", + "https://bcr.bazel.build/modules/rules_jvm_external/5.2/source.json": "10572111995bc349ce31c78f74b3c147f6b3233975c7fa5eff9211f6db0d34d9", "https://bcr.bazel.build/modules/rules_license/0.0.3/MODULE.bazel": "627e9ab0247f7d1e05736b59dbb1b6871373de5ad31c3011880b4133cafd4bd0", "https://bcr.bazel.build/modules/rules_license/0.0.7/MODULE.bazel": "088fbeb0b6a419005b89cf93fe62d9517c0a2b8bb56af3244af65ecfe37e7d5d", "https://bcr.bazel.build/modules/rules_license/1.0.0/MODULE.bazel": "a7fda60eefdf3d8c827262ba499957e4df06f659330bbe6cdbdb975b768bb65c", @@ -80,7 +84,9 @@ "https://bcr.bazel.build/modules/rules_python/0.33.2/source.json": "e539592cd3aae4492032cecea510e46ca16eeb972271560b922cae9893944e2f", "https://bcr.bazel.build/modules/rules_python/0.4.0/MODULE.bazel": "9208ee05fd48bf09ac60ed269791cf17fb343db56c8226a720fbb1cdf467166c", "https://bcr.bazel.build/modules/stardoc/0.5.1/MODULE.bazel": "1a05d92974d0c122f5ccf09291442580317cdd859f07a8655f1db9a60374f9f8", - "https://bcr.bazel.build/modules/stardoc/0.5.1/source.json": "a96f95e02123320aa015b956f29c00cb818fa891ef823d55148e1a362caacf29", + "https://bcr.bazel.build/modules/stardoc/0.5.3/MODULE.bazel": "c7f6948dae6999bf0db32c1858ae345f112cacf98f174c7a8bb707e41b974f1c", + "https://bcr.bazel.build/modules/stardoc/0.7.0/MODULE.bazel": "05e3d6d30c099b6770e97da986c53bd31844d7f13d41412480ea265ac9e8079c", + "https://bcr.bazel.build/modules/stardoc/0.7.0/source.json": "e3c524bf2ef20992539ce2bc4a2243f4853130209ee831689983e28d05769099", "https://bcr.bazel.build/modules/upb/0.0.0-20220923-a547704/MODULE.bazel": "7298990c00040a0e2f121f6c32544bab27d4452f80d9ce51349b1a28f3005c43", "https://bcr.bazel.build/modules/upb/0.0.0-20220923-a547704/source.json": "f1ef7d3f9e0e26d4b23d1c39b5f5de71f584dd7d1b4ef83d9bbba6ec7a6a6459", "https://bcr.bazel.build/modules/zlib/1.2.11/MODULE.bazel": "07b389abc85fdbca459b69e2ec656ae5622873af3f845e1c9d80fe179f3effa0", From 0eda639cb78a5cf0b479910d8c9a039e47ad36fe Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Tue, 24 Sep 2024 10:59:03 -0400 Subject: [PATCH 2/2] Fix the endianness of DTLS 1.3 ChaCha20 record number encryption 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 Commit-Queue: Bob Beck Auto-Submit: David Benjamin --- ssl/dtls_record.cc | 5 +++++ ssl/ssl_aead_ctx.cc | 13 +++++-------- ssl/test/runner/conn.go | 10 ++++------ 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/ssl/dtls_record.cc b/ssl/dtls_record.cc index 8ca7b63e8..ba06acf96 100644 --- a/ssl/dtls_record.cc +++ b/ssl/dtls_record.cc @@ -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. diff --git a/ssl/ssl_aead_ctx.cc b/ssl/ssl_aead_ctx.cc index 4f532e90d..5079ca2d1 100644 --- a/ssl/ssl_aead_ctx.cc +++ b/ssl/ssl_aead_ctx.cc @@ -483,20 +483,17 @@ bool ChaChaRecordNumberEncrypter::SetKey(Span key) { bool ChaChaRecordNumberEncrypter::GenerateMask(Span out, Span sample) { - Array 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 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; } diff --git a/ssl/test/runner/conn.go b/ssl/test/runner/conn.go index 38b135412..92293028c 100644 --- a/ssl/test/runner/conn.go +++ b/ssl/test/runner/conn.go @@ -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 }