test(integration): add dynamic record sizing test#5608
test(integration): add dynamic record sizing test#5608kaukabrizvi merged 12 commits intoaws:mainfrom
Conversation
bindings/rust/standard/integration/src/features/dynamic_record_sizing.rs
Show resolved
Hide resolved
bindings/rust/standard/integration/src/features/dynamic_record_sizing.rs
Outdated
Show resolved
Hide resolved
bindings/rust/standard/integration/src/features/dynamic_record_sizing.rs
Outdated
Show resolved
Hide resolved
bindings/rust/standard/integration/src/features/dynamic_record_sizing.rs
Outdated
Show resolved
Hide resolved
| "{}: Expected small record during ramp-up, got {} bytes (max {})", | ||
| phase, | ||
| size, | ||
| SMALL_RECORD_MAX |
There was a problem hiding this comment.
Discussion: not particularly important, but how much utility are we getting out of these debug messages? I personally find them to be a waste of lines unless the assertion is particularly dense/inscrutable.
In cases like this which are looping, I'll just add a print statement to the top of the loop to make debugging easier. E.g. at the top of validate_dynamic_sizing you can add
fn validate_dynamic_sizing(record_sizes: &[u16], phase: Phase) {
println!("checking record sizes for {phase:?}");Because the println! is swallowed by default unless the test fails.
There was a problem hiding this comment.
Good point - the extra message doesn’t add much beyond the assertion. I wanted to show phase and size for clarity, but a top-level print handles the phase, and size can be checked on failure. I’ll adjust this in the next revision.
bindings/rust/standard/integration/src/features/dynamic_record_sizing.rs
Outdated
Show resolved
Hide resolved
bindings/rust/standard/integration/src/features/dynamic_record_sizing.rs
Outdated
Show resolved
Hide resolved
jmayclin
left a comment
There was a problem hiding this comment.
Looks good!
I do think we'll want to have coverage over both TLS 1.2 and TLS 1.3 here before we remove the old test tho. TLS 1.2 and TLS 1.3 have significant differences, (e.g. record formatting, padding handling, etc), so most integration tests generally benefit from both TLS 1.2 and TLS 1.3 coverage.
Approving, and then would vote to add TLS 1.2 coverage as a follow up, since that will require some changes to the handshake look.
bindings/rust/standard/integration/src/features/dynamic_record_sizing.rs
Show resolved
Hide resolved
e291c66 to
cde36ce
Compare
Goal
Add a dynamic record sizing integration test to the rust integration test suite in an effort to replace the tcpdump-based test in integv2 by moving to an in-memory, deterministic approach leveraging the Rust TLS harness.
Why
To eventually remove the tcpdump-based test in integv2. Adding this test is part of the larger effort for a new integration test approach. The record padding test was merged in #5451 and the codebuild/nix setup for the rust tests was merged in #5578.
How
This PR adds a new Rust-based integration test to validate dynamic record sizing using in-memory IO instead of external packet capture. The test runs in both orientations (s2n as client or server, with OpenSSL as the peer) and verifies record size behavior across three phases: ramp-up, steady state, and post-timeout. The old integv2 test relied on tcpdump to detect large packets on the wire. The new Rust test inspects exact TLS record boundaries and sizes directly in memory, providing determinism and more visibility into each phase of dynamic record sizing.
Callouts
Partial record handling: The final TLS record may be smaller than the expected "large" record size due to remainder data at the end of the stream. To avoid flakiness and keep the test deterministic, the validation logic skips the final record when checking record size transitions.
"Small" record size assumption: SMALL_RECORD_MAX is set to 1,500 bytes based on typical Ethernet MTU limits. Since this test uses in-memory IO rather than a real network interface, we're validating s2n's record sizing policy, not actual on-wire fragmentation.
Testing
Ran the tests with detailed logging to inspect that the current behavior matched what was expected:
Phase 1 (Initial ramp-up): 12 small records (1420 bytes) → then 11 large records (8109 bytes) once 16 KB threshold crossed.
Phase 2 (Steady state): 13 large records (8109 bytes), no small records.
Phase 3 (Post-timeout): After 2s sleep, same pattern as Phase 1 (12 small → 11 large).
Ran the tests across all supported libcryptos in the rust nix environment and in codebuild - passed.
Related
The record padding rust integration test was merged in #5451 and the codebuild/nix setup for the rust tests was merged in #5578.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.