feat: getter for TLS1.2 master secrets #4470
Conversation
lrstewart
force-pushed
the
master_secret
branch
from
8633e70 to
1d0b7ac
Compare
| * @returns S2N_SUCCESS on success, S2N_FAILURE otherwise. `secret_bytes` | ||
| * will be set on success. | ||
| */ | ||
| S2N_API extern int s2n_connection_get_master_secret(const struct s2n_connection *conn, |
There was a problem hiding this comment.
Yay for less scary rust bindings 🥳
There was a problem hiding this comment.
Eeeeeeeeeh we still pass it a *mut pointer 🙃
I was just looking at this for our Send/Sync safety. No matter what we do with const, the bindings are still scary. You can get a *mut s2n_connection from a &Connection, because you can get a *mut s2n_connection from a &NonNull<s2n_connection>. The joys of FFI.
There was a problem hiding this comment.
Ya, the * mut isn't very pretty but I'm mostly just celebrating the fact that the rust connection parameter &self is way more obviously accurate for these bindings. Little wins 😄
| /* Test: self-talk */ | ||
| for (size_t i = 0; i < s2n_array_len(supported_versions); i++) { | ||
| const uint8_t version = supported_versions[i]; | ||
| if (s2n_is_in_fips_mode() && version == S2N_SSLv3) { |
There was a problem hiding this comment.
nit: It might be nice to add a comment on why this restriction is necessary.
There was a problem hiding this comment.
It's unrelated to what we're testing here and probably a bug: #4476 At least, the first blocker I found was clearly a bug.
Description of changes:
A customer needs the TLS1.2 master secret for compatibility reasons. I'm adding a getter to make it available, but limiting it to TLS1.2 where it can do less damage. After all, we already pass around the TLS1.2 master secret in session tickets, and TLS1.2 exporters would use the master secret too.
Callouts:
I'm intentionally not communicating the length of the master secret in any way except documentation. If we need to support secrets with variable lengths like TLS1.3, we can follow our usual pattern and add a s2n_connection_get_master_secret_length method. But I really don't want to support TLS1.3 master secrets, ever 😭
Testing:
New unit tests, including a self-talk to make sure the secret is actually available after the handshake.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.