aws / aws/s2n-tls

ktls test stuffer io doesn't clean up after itself

Open
#4,191 0 comments 0 reactions 0 assignees View on GitHub
priority/low s2n-core size/medium
Dominant language
C
Stars
4.8k
Forks
802
Avg merge
5d 22h
Merged PRs (30d)
33

Description

### Problem:
s2n_test_init_ktls_io_stuffer_send and s2n_test_init_ktls_io_stuffer allocate memory for the IO stuffers and set the sendmsg callback + ctx. s2n_ktls_io_stuffer_free and s2n_ktls_io_stuffer_pair_free just free the memory for the IO stuffer, leaving the sendmsg callback + ctx untouched. This can lead to unexpected behavior in later tests since the callback is still set, but the ctx for the current connection is potentially not properly set / doesn't match what the callback expects.

### Solution:
The free methods should set the callback back to the default or to a saved original value from before init.

This will likely also require some cleanup of the init and free methods. The send version doesn't share any logic in common with the send + recv version, which complicates things.

We also have the same problem when tests set the sendmsg callback manually. We probably need a DEFER_CLEANUP-based method to clean the callback up when the test completes.

* **Does this change what S2N sends over the wire?** If yes, explain.
* **Does this change any public APIs?** If yes, explain.
* **Which versions of TLS will this impact?**

### Requirements / Acceptance Criteria:

What must a solution address in order to solve the problem? How do we know the solution is complete?

* **RFC links:** Links to relevant RFC(s)
* **Related Issues:** Link any relevant issues
* **Will the Usage Guide or other documentation need to be updated?**
* **Testing:** How will this change be tested? Call out new integration tests, functional tests, or particularly interesting/important unit tests.
* **Will this change trigger SAW changes?** Changes to the state machine, the s2n_handshake_io code that controls state transitions, the DRBG, or the corking/uncorking logic could trigger SAW failures.
* **Should this change be fuzz tested?** Will it handle untrusted input? Create a separate issue to track the fuzzing work.

### Out of scope:

Is there anything the solution will intentionally NOT address?

[//]: # (NOTE: If you believe this might be a security issue, please email aws-security@amazon.com instead of creating a GitHub issue. For more details, see the AWS Vulnerability Reporting Guide: https://aws.amazon.com/security/vulnerability-reporting/ )

Contributor guide

Open the contributing guide

Research direction

Start with s2n_test_init_ktls_io_stuffer_send, s2n_test_init_ktls_io_stuffer, s2n_ktls_io_stuffer_free, and s2n_ktls_io_stuffer_pair_free, then trace how tests set sendmsg callbacks and contexts. Done means cleanup restores the prior or default callback and context, including manually configured callbacks, without leaking test state between connections.

Written by the indexing model from the issue text.

Assessment

Tech stack
c
Domain
testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
42/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.