diff --git a/netcode.c b/netcode.c index 0ab9671..8d6065a 100755 --- a/netcode.c +++ b/netcode.c @@ -5498,6 +5498,112 @@ do } \ } while(0) +static void test_crypto_aead_vectors() +{ + // Known-answer test for the two AEAD primitives netcode relies on, exercising + // whichever implementation the running CPU selects (reference / SSSE3 / AVX2 for + // ChaCha20, donna / SSE2 for Poly1305). Expected ciphertext was generated from + // libsodium reference output and is unchanged across 1.0.20 -> 1.0.22. Do not + // edit these arrays by hand: a golden failure here means the vendored crypto no + // longer agrees with upstream, which would break every other netcode + // implementation on the wire. + // + // Declared static on purpose. yojimbo vendors this file and compiles it with + // NETCODE_ENABLE_TESTS=1 while defining its own test_crypto_aead_vectors in + // test.cpp; internal linkage keeps the two from colliding. + + static const uint8_t kat_key[32] = { + 0x40,0x41,0x42,0x43,0x44,0x45,0x46,0x47,0x48,0x49,0x4a,0x4b, + 0x4c,0x4d,0x4e,0x4f,0x50,0x51,0x52,0x53,0x54,0x55,0x56,0x57, + 0x58,0x59,0x5a,0x5b,0x5c,0x5d,0x5e,0x5f, + }; + static const uint8_t kat_ad[12] = { + 0xc0,0xc1,0xc2,0xc3,0xc4,0xc5,0xc6,0xc7,0xc8,0xc9,0xca,0xcb, + }; + static const uint8_t kat_msg[58] = { + 0x79,0x6f,0x6a,0x69,0x6d,0x62,0x6f,0x20,0x76,0x65,0x6e,0x64,0x6f,0x72,0x65,0x64,0x20,0x6c,0x69,0x62, + 0x73,0x6f,0x64,0x69,0x75,0x6d,0x20,0x41,0x45,0x41,0x44,0x20,0x6b,0x6e,0x6f,0x77,0x6e,0x2d,0x61,0x6e, + 0x73,0x77,0x65,0x72,0x20,0x74,0x65,0x73,0x74,0x20,0x76,0x65,0x63,0x74,0x6f,0x72,0x21,0x21, + }; + static const uint8_t kat_npub_ietf[12] = { + 0xa0,0xa1,0xa2,0xa3,0xa4,0xa5,0xa6,0xa7,0xa8,0xa9,0xaa,0xab, + }; + static const uint8_t kat_ct_ietf[74] = { + 0xd5,0xae,0xb1,0x85,0x15,0x8b,0x07,0xb3,0x01,0x15,0xf0,0x59, + 0xb4,0x4e,0x9d,0x45,0x91,0x58,0xab,0xff,0xaf,0xbd,0x81,0x4f, + 0xbf,0x52,0xc2,0x4c,0xa1,0x5e,0x60,0x5f,0x58,0x63,0x31,0x96, + 0xda,0x90,0x07,0x63,0xb9,0x0c,0x21,0x46,0xf2,0xe4,0x65,0x96, + 0x7a,0x81,0x7f,0xa2,0x5d,0xd1,0x79,0xf6,0x9b,0x18,0x5d,0xe0, + 0xb6,0x57,0x93,0xbe,0x8c,0xb5,0xa9,0x75,0x98,0xa4,0x6f,0xd5, + 0xbe,0x9d, + }; + static const uint8_t kat_npub_xchacha[24] = { + 0x10,0x11,0x12,0x13,0x14,0x15,0x16,0x17,0x18,0x19,0x1a,0x1b, + 0x1c,0x1d,0x1e,0x1f,0x20,0x21,0x22,0x23,0x24,0x25,0x26,0x27, + }; + static const uint8_t kat_ct_xchacha[74] = { + 0x2b,0x24,0x83,0x2a,0x6c,0x9e,0x21,0x02,0x2a,0x14,0x32,0x56, + 0x4b,0x27,0x37,0x92,0x24,0x40,0xa9,0x92,0xd3,0x53,0xa7,0xa5, + 0x64,0xd3,0x8e,0x0c,0x75,0x79,0x75,0x3f,0xca,0x82,0xfa,0x85, + 0xf0,0xa6,0xac,0x08,0x9a,0x25,0xf1,0x8f,0x42,0x20,0x70,0x8e, + 0x38,0x25,0xd1,0x08,0x45,0x81,0x75,0x18,0xe4,0xd1,0x88,0xbd, + 0x92,0xfa,0x84,0xdc,0xd6,0xa3,0x9a,0x67,0x52,0x91,0x62,0xf4, + 0x86,0x7b, + }; + + uint8_t c[128]; + uint8_t m[128]; + unsigned long long clen = 0; + unsigned long long mlen = 0; + + // ChaCha20-Poly1305 (IETF) -- the construction netcode uses on the wire + + check( crypto_aead_chacha20poly1305_ietf_encrypt( c, &clen, kat_msg, sizeof( kat_msg ), kat_ad, sizeof( kat_ad ), NULL, kat_npub_ietf, kat_key ) == 0 ); + check( clen == sizeof( kat_ct_ietf ) ); + check( memcmp( c, kat_ct_ietf, (size_t) clen ) == 0 ); + check( crypto_aead_chacha20poly1305_ietf_decrypt( m, &mlen, NULL, c, clen, kat_ad, sizeof( kat_ad ), kat_npub_ietf, kat_key ) == 0 ); + check( mlen == sizeof( kat_msg ) ); + check( memcmp( m, kat_msg, (size_t) mlen ) == 0 ); + + // a tampered tag must be rejected. this is the path ACQUIRE_FENCE guards: the + // plaintext must not be produced before authentication has completed. + + c[0] ^= 0x01; + check( crypto_aead_chacha20poly1305_ietf_decrypt( m, &mlen, NULL, c, clen, kat_ad, sizeof( kat_ad ), kat_npub_ietf, kat_key ) != 0 ); + + // XChaCha20-Poly1305 + + check( crypto_aead_xchacha20poly1305_ietf_encrypt( c, &clen, kat_msg, sizeof( kat_msg ), kat_ad, sizeof( kat_ad ), NULL, kat_npub_xchacha, kat_key ) == 0 ); + check( clen == sizeof( kat_ct_xchacha ) ); + check( memcmp( c, kat_ct_xchacha, (size_t) clen ) == 0 ); + check( crypto_aead_xchacha20poly1305_ietf_decrypt( m, &mlen, NULL, c, clen, kat_ad, sizeof( kat_ad ), kat_npub_xchacha, kat_key ) == 0 ); + check( mlen == sizeof( kat_msg ) ); + check( memcmp( m, kat_msg, (size_t) mlen ) == 0 ); + c[0] ^= 0x01; + check( crypto_aead_xchacha20poly1305_ietf_decrypt( m, &mlen, NULL, c, clen, kat_ad, sizeof( kat_ad ), kat_npub_xchacha, kat_key ) != 0 ); + + // the constant-time comparison that checks the Poly1305 tag. libsodium 1.0.21 + // hardened crypto_verify_n against the compiler optimising it into something + // branchy; these assertions pin its contract. + + uint8_t a[64], b[64]; + int i; + for ( i = 0; i < 64; i++ ) { a[i] = (uint8_t) i; b[i] = (uint8_t) i; } + check( crypto_verify_16( a, b ) == 0 ); + check( crypto_verify_32( a, b ) == 0 ); + check( crypto_verify_64( a, b ) == 0 ); + for ( i = 0; i < 8; i++ ) + { + b[0] = (uint8_t) ( a[0] ^ ( 1u << i ) ); + check( crypto_verify_16( a, b ) == -1 ); + check( crypto_verify_32( a, b ) == -1 ); + check( crypto_verify_64( a, b ) == -1 ); + } + b[0] = a[0]; + b[63] = (uint8_t) ( a[63] ^ 0x80 ); + check( crypto_verify_64( a, b ) == -1 ); +} + static void test_queue() { struct netcode_packet_queue_t queue; @@ -9511,6 +9617,7 @@ void netcode_test() { //while ( 1 ) { + RUN_TEST( test_crypto_aead_vectors ); RUN_TEST( test_queue ); RUN_TEST( test_endian ); RUN_TEST( test_address ); diff --git a/sodium/NOTES.md b/sodium/NOTES.md index 8344d12..20a4d64 100644 --- a/sodium/NOTES.md +++ b/sodium/NOTES.md @@ -31,11 +31,11 @@ the system `sodium` library. See BUILDING.md. ## Baseline -The crypto sources track **libsodium 1.0.20**. netcode/yojimbo only use a tiny -slice of libsodium, and upstream has not changed those sources since 1.0.17 — the -ChaCha20, Poly1305, and AEAD implementations are byte-identical across 1.0.17 → -1.0.20. This subset has been verified to match libsodium 1.0.20 test vectors (see -"Validation" below). +The crypto sources track **libsodium 1.0.22**. netcode/yojimbo only use a tiny +slice of libsodium, and the AEAD/ChaCha20/Poly1305 implementations were byte-identical +across 1.0.17 → 1.0.20; 1.0.21/1.0.22 then made three hardening changes inside the +slice, all incorporated (see the review log). This subset is verified against libsodium +test vectors by `test_crypto_aead_vectors` in netcode.c (see "Validation"). ## Upstream tracking @@ -62,15 +62,43 @@ Reviewing a new release means: ### Review log -- **1.0.22 (reviewed; vendored still 1.0.20).** 1.0.21 and 1.0.22 are mostly outside the - included slice — the ed25519 small-order-point fix, ipcrypt, XOF/SHA-3, ML-KEM768 / - X-Wing, and assorted build/platform work do not touch the ChaCha20/Poly1305/AEAD code - netcode uses. **One item does:** 1.0.20-stable/1.0.21 added memory fences after MAC - verification in the AEAD path (a speculative-access hardening — plaintext must not be - read before authentication completes). This appears to touch the ChaCha20-Poly1305 - decrypt code in this subset and is **pending incorporation** on the next re-vendor; - it is a defense-in-depth hardening, not a functional or interop change, so the vendored - 1.0.20 remains correct and interoperable in the meantime. +- **1.0.22 (reviewed AND incorporated, 2026-07-25).** The vendored slice now carries the + 1.0.22 text. Most of 1.0.21/1.0.22 is outside the slice — the ed25519 small-order-point + fix, ipcrypt, XOF/SHA-3, ML-KEM768 / X-Wing and assorted build work do not touch the + ChaCha20/Poly1305/AEAD code netcode uses. + + **THREE items were inside it, not one.** The earlier review of this release found only + the first and recorded the other two as absent; they were found by diffing the trees + file-by-file rather than reading the changelog. The changelog is a summary, and a + summary is not a diff: + + 1. **`ACQUIRE_FENCE` after MAC verification** in all three AEAD decrypt paths + (chacha20poly1305, chacha20poly1305_ietf, xchacha20poly1305_ietf), placed between + the authentication-failure return and the plaintext stream-xor. Speculative-access + hardening: plaintext must not be produced before authentication completes. + 2. **`crypto_verify_n` constant-time hardening** (`verify.c`) — an inline-asm + optimisation barrier plus a `volatile optblocker_u16` threaded through the final + reduction, and `uint_fast16_t` narrowed to `uint16_t`. This is the function that + compares the **Poly1305 tag**, so this is arguably the most security-relevant of + the three: it defends the constant-time comparison against a compiler turning it + branchy, which would reintroduce a timing side channel. + 3. **poly1305 SSE2 final reduction** — the same `optblocker_u64` treatment. Upstream + did NOT change the donna64 copy, and this amalgamation's changed line is confirmed + inside the `poly1305_state_internal__sse2` section, so only the SSE2 copy moved. + + All three are defence-in-depth: none changes ciphertext, tags, or interop. Verified by + a differential test — the patched amalgamation and the pristine 1.0.20 baseline were + each built against the same harness and produced byte-identical output over 200 + randomised encrypt / decrypt / tamper-reject rounds for both AEADs, plus the + `crypto_verify_*` contract. The harness was then proven capable of failing by injecting + the most plausible transcription error in this patch (`>> 3` for `>> 2` in the + optblocker reduction), which it caught immediately. + + **Deliberately NOT applied:** the remaining 1.0.20 → 1.0.22 deltas inside the slice are + comment-only or cosmetic and are enumerated here so the difference is known rather than + forgotten — `/* LCOV_EXCL_LINE */` coverage annotations in `stream_chacha20.c`, + `utils.c` and the AEAD files; `defined _AIX` → `defined(_AIX)`; and the removal of an + unused `uint32_t t32` in `utils.c`. None affects generated code. ## What is included @@ -114,7 +142,12 @@ On MSVC, AVX2 additionally requires building with `/arch:AVX2` (which predefines - All files amalgamated into `sodium.h` + `sodium.c` (see "Structure" above). - Optional `NETCODE_CRYPTO_LOGS` debug prints in the implementation selectors. - SIMD feature macros (`HAVE_*INTRIN_H`, `HAVE_TI_MODE`) are derived from the - compiler's own target macros (in `sodium.h`) instead of from autoconf. The SIMD + compiler's own target macros (in `sodium.h`) instead of from autoconf. Since + 2026-07-25 the same is done for `HAVE_GCC_MEMORY_FENCES` and `HAVE_INLINE_ASM`, + which gate `ACQUIRE_FENCE` and the `crypto_verify_n` asm barrier. On GCC/Clang both + are enabled; on MSVC `ACQUIRE_FENCE` falls back to `(void) 0`, which is exactly + upstream's behaviour when autoconf detects no fence primitive — correct and + interoperable, without that one hardening. The SIMD code uses the same `#pragma clang attribute` / `#pragma GCC target` idiom as upstream 1.0.20 so it compiles without global `-m` flags. @@ -135,9 +168,17 @@ The crypto here is checked several ways: architectures. 3. The full yojimbo test suite passes linked against this subset (exercises netcode's real connect-token / packet encryption paths). -4. `test.cpp` includes `test_crypto_aead_vectors()`, a known-answer test that runs - in CI on every platform — so whichever implementation the target CPU selects is - exercised. +4. `netcode.c` includes `test_crypto_aead_vectors()`, a known-answer test registered + first in `netcode_test()`, so it runs in CI on every platform and whichever + implementation the target CPU selects is exercised. It pins the golden ciphertext + for both AEADs, the tamper-rejection path, and the `crypto_verify_*` contract. + + *(Added 2026-07-25. This document previously told the reader to re-run + `test_crypto_aead_vectors` from `test.cpp` — but that test lived in **yojimbo**, not + here, and these notes had been copied across. The prescribed validation did not + exist in this repo. It does now, and it is declared `static` so it cannot collide + with yojimbo's own copy when yojimbo vendors this file with + `NETCODE_ENABLE_TESTS=1`.)* If you change anything in this directory, re-run `bin/test` and make sure `test_crypto_aead_vectors` passes. diff --git a/sodium/sodium.c b/sodium/sodium.c index a9c81ac..a21ac6d 100644 --- a/sodium/sodium.c +++ b/sodium/sodium.c @@ -918,6 +918,8 @@ crypto_verify_n(const unsigned char *x_, const unsigned char *y_, #else +static volatile uint16_t optblocker_u16; + static inline int crypto_verify_n(const unsigned char *x_, const unsigned char *y_, const int n) @@ -926,13 +928,19 @@ crypto_verify_n(const unsigned char *x_, const unsigned char *y_, (const volatile unsigned char *volatile) x_; const volatile unsigned char *volatile y = (const volatile unsigned char *volatile) y_; - volatile uint_fast16_t d = 0U; - int i; + volatile uint16_t d = 0U; + int i; for (i = 0; i < n; i++) { d |= x[i] ^ y[i]; } - return (1 & ((d - 1) >> 8)) - 1; +# ifdef HAVE_INLINE_ASM + __asm__ __volatile__("" : "+r"(d) :); +# endif + d--; + d = ((d >> 13) ^ optblocker_u16) >> 2; + + return (int) d - 1; } #endif @@ -4995,6 +5003,8 @@ poly1305_init_ext(poly1305_state_internal_t *st, const unsigned char key[32], st->leftover = 0U; } +static volatile uint64_t optblocker_u64; + static POLY1305_NOINLINE void poly1305_blocks(poly1305_state_internal_t *st, const unsigned char *m, unsigned long long bytes) @@ -5546,7 +5556,7 @@ poly1305_blocks(poly1305_state_internal_t *st, const unsigned char *m, g1 &= 0xfffffffffff; g2 = h2 + c - ((uint64_t) 1 << 42); - c = (g2 >> 63) - 1; + c = (((g2 >> 61) ^ optblocker_u64) >> 2) - 1; nc = ~c; h0 = (h0 & nc) | (g0 & c); h1 = (h1 & nc) | (g1 & c); @@ -6062,6 +6072,7 @@ crypto_aead_chacha20poly1305_decrypt_detached(unsigned char *m, memset(m, 0, mlen); return -1; } + ACQUIRE_FENCE; crypto_stream_chacha20_xor_ic(m, c, mlen, npub, 1U, k); return 0; @@ -6146,6 +6157,7 @@ crypto_aead_chacha20poly1305_ietf_decrypt_detached(unsigned char *m, memset(m, 0, mlen); return -1; } + ACQUIRE_FENCE; crypto_stream_chacha20_ietf_xor_ic(m, c, mlen, npub, 1U, k); return 0; @@ -6358,6 +6370,7 @@ _decrypt_detached(unsigned char *m, memset(m, 0, mlen); return -1; } + ACQUIRE_FENCE; crypto_stream_chacha20_ietf_ext_xor_ic(m, c, mlen, npub, 1U, k); return 0; diff --git a/sodium/sodium.h b/sodium/sodium.h index 5f849ed..37c6590 100644 --- a/sodium/sodium.h +++ b/sodium/sodium.h @@ -758,6 +758,31 @@ int crypto_stream_chacha20_ietf_ext_xor_ic(unsigned char *c, const unsigned char # define HAVE_TI_MODE 1 #endif +/* Optimisation barriers used by the constant-time code and by the AEAD decrypt + paths (libsodium 1.0.21/1.0.22). Upstream gets these from autoconf; this + amalgamation derives them from the compiler's own macros, as it already does + for HAVE_TI_MODE and the SIMD intrinsics above. + + ACQUIRE_FENCE is placed after MAC verification and before the plaintext is + produced, so a speculative read cannot reach unauthenticated plaintext. + Falling back to (void) 0 matches upstream's behaviour when autoconf detects + no fence primitive: correct and interoperable, without the hardening. */ +#if !defined(HAVE_GCC_MEMORY_FENCES) && (defined(__GNUC__) || defined(__clang__)) +# define HAVE_GCC_MEMORY_FENCES 1 +#endif +#if !defined(HAVE_INLINE_ASM) && (defined(__GNUC__) || defined(__clang__)) && \ + !defined(_MSC_VER) +# define HAVE_INLINE_ASM 1 +#endif + +#ifdef HAVE_GCC_MEMORY_FENCES +# define ACQUIRE_FENCE __atomic_thread_fence(__ATOMIC_ACQUIRE) +#elif defined(HAVE_C11_MEMORY_FENCES) +# define ACQUIRE_FENCE atomic_thread_fence(memory_order_acquire) +#else +# define ACQUIRE_FENCE (void) 0 +#endif + #ifdef HAVE_TI_MODE # if defined(__SIZEOF_INT128__) typedef unsigned __int128 uint128_t; @@ -1391,7 +1416,7 @@ int _sodium_alloc_init(void); #define sodium_version_H -#define SODIUM_VERSION_STRING "1.0.20" +#define SODIUM_VERSION_STRING "1.0.22" #define SODIUM_LIBRARY_VERSION_MAJOR 26 #define SODIUM_LIBRARY_VERSION_MINOR 2