rust bindings: provide poll_recv with uninitialized destination buffer

[Mark-Simulacrum]

Security issue notifications

If you discover a potential security issue in s2n we ask that you notify AWS Security via our vulnerability reporting page. Please do not create a public github issue.

Problem:

The current poll_recv method requires that the buffer passed to it is initialized (&mut [u8]), though in practice should never read from the buffer, only write to it. It should probably be updated to take &mut [MaybeUninit<u8>], so that callers don't have to initialize their buffers.

Solution:

A description of the possible solution in terms of S2N architecture. Highlight and explain any potentially controversial design decisions taken.

• Does this change what S2N sends over the wire? No
• Does this change any public APIs? Yes
• Which versions of TLS will this impact? n/a

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?

In theory it would probably be nice to add vectored I/O poll_ methods as well, but that's a separate issue.

[lrstewart]

What's the benefit of this change? Using MaybeUninit adds complexity when calling the method, and I'm not sure how much of a benefit not initializing the buffer is.

[Mark-Simulacrum]

Yeah, the benefit may be small or non-existent, we don't have benchmarks to indicate otherwise at this point. s2n-tls-tokio (and our own custom bindings) are currently using initialize_unfilled which in general should perform fine, but seems like an unfortunate addition of complexity.

I think I've read in the past that for some use cases buffer initialization is a significant overhead, but I don't have references off hand for that. I think the main reason to offer MaybeUninit here is that the cost of doing so is pretty low, and if such a case arises in the future a compatibility break to support it (or an extra method) would be unfortunate.

Do you know if s2n-tls (in C) is zeroing out the passed buffers (perhaps as a security mitigation against accidental leakage or similar) at any point?

[maddeleine]

Looks like this was actually completed in this PR: https://github.com/aws/s2n-tls/pull/3662