Do we still need s2n_ensure_memmove_trace?

[lrstewart]

Problem:

s2n_ensure_memmove_trace / s2n_ensure_memcpy_trace used to write a custom debug string, but is now just a basic wrapper around memmove that checks its return value. We may just be able to remove it, although doing so may require some work in the CBMC proofs.

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

[maddeleine]

Semi-related: Ever since this pr: #4447 I keep on seeing a memmove related compile warning in my cmake runs. Probably good to get that fixed along with this issue.

/home/ubuntu/s2n/tests/unit/s2n_safety_test.c: In function ‘failure_memcpy’:
/home/ubuntu/s2n/./utils/s2n_ensure.h:67:23: warning: ‘src’ may be used uninitialized [-Wmaybe-uninitialized]
   67 |             void *r = s2n_ensure_memmove_trace((d), (s), (__tmp_n)); \
      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/ubuntu/s2n/./utils/s2n_safety_macros.h:360:63: note: in expansion of macro ‘__S2N_ENSURE_SAFE_MEMMOVE’
  360 | #define POSIX_CHECKED_MEMCPY(destination, source, len)        __S2N_ENSURE_SAFE_MEMMOVE((destination), (source), (len), POSIX_ENSURE_REF)
      |                                                               ^~~~~~~~~~~~~~~~~~~~~~~~~
/home/ubuntu/s2n/tests/unit/s2n_safety_test.c:134:5: note: in expansion of macro ‘POSIX_CHECKED_MEMCPY’
  134 |     POSIX_CHECKED_MEMCPY(ptr, src, 1024);
      |     ^~~~~~~~~~~~~~~~~~~~
/home/ubuntu/s2n/./utils/s2n_ensure.h:93:7: note: by argument 2 of type ‘const void *’ to ‘s2n_ensure_memmove_trace’ declared here
   93 | void *s2n_ensure_memmove_trace(void *to, const void *from, size_t size);
      |       ^~~~~~~~~~~~~~~~~~~~~~~~
/home/ubuntu/s2n/tests/unit/s2n_safety_test.c:131:10: note: ‘src’ declared here
  131 |     char src[1024];
      |          ^~~