s2n_connection bitfield potentially shared by s2n_send and s2n_recv

[lrstewart]

Problem:

Our documentation states that s2n_send and s2n_recv can be called at the same time from different threads. In general, we're pretty careful about keeping their state separate. However, I don't think we give the same consideration to s2n_connection's bitfield, which could lead to conflicts if two threads try to update different flags in the same field at the same time. For example, key_update_pending and close_notify_received are both set by s2n_recv.

From a quick review, I think at the moment only s2n_recv is setting flags in the bitfield, which would mean we don't have a problem right now. But if we're not careful, we could create a problem by adding a flag set by s2n_send.

Solution:

Minimally, audit the bitfield and ensure we don't have a problem, then add documentation. We could either decide that neither s2n_send nor s2n_recv can use the bitfield, or we can decide that only s2n_recv can use it (what I think is the current situation).

I'd love to have actual enforcement that s2n_send and s2n_recv don't use the same variables. I feel like that should be possible to enforce via some kind of static analysis tool.

• 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?