s2n_stuffer_skip_write usage pattern

[baldwinmatt]

Problem:

A common pattern is:

GUARD(s2n_stuffer_skip_write(stuffer, bytes_to_write));
uint8_t* ptr = suffer->blob.data + stuffer->write_cursor - bytes_to_write;

which could be simplified.

Solution:

*ptr could be an *out parameter to s2n_stuffer_skip_write

• Does this change what S2N sends over the wire? No.
• Does this change any public APIs? No.
• 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?

• Testing: Existing tests should pass as is, however CBMC proofs will need to be updated to match thew new stuffer API
  • Will this change trigger SAW changes? No.
  • Should this change be fuzz tested? N/A

Out of scope:

Is there anything the solution will intentionally NOT address?

No

[danielsn]

Actually, would it make sense for the skip_write to return a s2n_blob_slice, so we carry around both the size, and the pointer together? https://github.com/awslabs/s2n/blob/0e74e54a14c4f48f95ebcb24cffd6690d0c11e4d/utils/s2n_blob.c#L55

[baldwinmatt]

Actually, would it make sense for the skip_write to return a s2n_blob_slice, so we carry around both the size, and the pointer together?

https://github.com/awslabs/s2n/blob/0e74e54a14c4f48f95ebcb24cffd6690d0c11e4d/utils/s2n_blob.c#L55

love it!

[sneznaj]

i could find this pattern 2 times

3 times POSIX_GUARD(s2n_stuffer_skip_write(stuffer, length)); uint8_t *data = (stuffer->blob.data) ? (stuffer->blob.data + stuffer->write_cursor - length) : NULL;

and 32 cases where s2n_stuffer_skip_write is used in another context.

and 39 cases where s2n_stuffer_skip_write is being tested without use of this pattern

[zawie]

Since this pattern occurs a minority of the time and is conceptually distinct, I propose to define a separate method to encapsulate this pattern: s2n_stuff_bypass_write(struct s2n_stuffer *stuffer, const uint32_t n, struct s2n_blob *bypassed_slice). Where bypassed_slice is a s2n_blob_slice as proposed by @danielsn.

We can pull out this common use case into a separate function without forcing the common use case to pass an output parameter. How does this sound?

[zakhaev26]

may i work on this issue and perform a PR? @danielsn @zawie