Allocate s2n_crypto_parameters separately

[lrstewart]

Description of changes:

Move both s2n_crypto_parameters off of s2n_connection so that conn->initial can be freed after the handshake. conn->initial represents a plaintext context, and all data after the handshake should be encrypted and use conn->secure.

This has the nice side effect of cleaning up some of the worst s2n_connection setup code.

Memory information after this change:

After setup: 3kb active, 7kb allocated
After connections created: 63kb active, 66kb allocated
After handshake: 78kb active, 117kb allocated
After freeing handshake: 67kb active (11kb freed)
After io: 67kb active, 117kb allocated

Total active for each connection: ~31kb
conn structure: 4kb client, 4kb server
crypto_params: 2kb client, 2kb server
conn->in: 16kb client, 16kb server
conn->out: 9kb client, 8kb server
unaccounted for / potential cleanup: 3kb

Callouts

• Why not always free conn->initial after the handshake? I free conn->initial in s2n_connection_free_handshake because it's a fairly expensive structure to setup, so we only want applications that already deliberately free reusable handshake memory to free it. Otherwise, for applications calling s2n_connection_wipe, it'll just be reused for the next connection.

Testing:

I added a new test to verify we can send, recv, and wipe/reuse connections after both s2n_connection_free_handshake and s2n_connection_release_buffers. I couldn't find any existing test of that case. I can't imagine it's a common use case, but it should work.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[maddeleine]

What's happening here exactly? Why save a pointer to conn->secure/conn->initial?

[lrstewart]

The first half of s2n_connection_wipe is saving anything from the connection we want to keep, then we zero the connection structure, then we restore the things we saved. The stack pointers here are to save the conn->secure/conn->initial memory. Without them, the conn->secure and conn->initial pointers will be reset to zero/NULL and the memory they pointed to will be leaked.

[maddeleine]

Moving the initialization of the crypto params to s2n_connection_wipe means that there would be more frequent calls to allocate memory right? Do we need documentation for this kind of thing?

[lrstewart]

Only if s2n_connection_free_handshake is called, which is the point of that API. Otherwise, we just reuse the memory allocated by s2n_connection_new.