ktls: config socket ULP #4066
ktls: config socket ULP #4066
Conversation
toidiu
force-pushed
the
ak-ktls0_config_socket
branch
5 times, most recently
from
ab5903e to
607a74b
Compare
toidiu
force-pushed
the
ak-ktls0_config_socket
branch
3 times, most recently
from
9de2265 to
588e394
Compare
toidiu
force-pushed
the
ak-ktls0_config_socket
branch
from
588e394 to
dbdc819
Compare
toidiu
force-pushed
the
ak-ktls0_config_socket
branch
from
c21995a to
c235257
Compare
toidiu
force-pushed
the
ak-ktls0_config_socket
branch
from
c235257 to
5e80fd0
Compare
|
|
||
| S2N_RESULT s2n_skip_handshake(struct s2n_connection *conn) |
There was a problem hiding this comment.
Since this is no longer a static test method, I'd probably change the name to s2n_connection_skip_handshake. Or maybe s2n_connection_set_complete? I definitely think the name could use a revisit ;)
tls/s2n_ktls.c
Outdated
| if (ktls_mode == S2N_KTLS_MODE_SEND && conn->ktls_send_enabled) { | ||
| RESULT_BAIL(S2N_ERR_KTLS_ALREADY_ENABLED); | ||
| } | ||
| if (ktls_mode == S2N_KTLS_MODE_RECV && conn->ktls_recv_enabled) { | ||
| RESULT_BAIL(S2N_ERR_KTLS_ALREADY_ENABLED); | ||
| } | ||
|
|
||
| int fd = 0; | ||
| RESULT_GUARD(s2n_ktls_retrieve_file_descriptor(conn, ktls_mode, &fd)); |
There was a problem hiding this comment.
Since all this logic differs for send vs recv, why not keep it in s2n_connection_ktls_enable_send / s2n_connection_ktls_enable_recv rather than in the common method they both call?
tests/unit/s2n_ktls_test.c
Outdated
|
|
||
| S2N_RESULT s2n_test_configure_mock_ktls_connection(struct s2n_connection *conn, int fd, bool complete_handshake) |
There was a problem hiding this comment.
This threw me a bit: If you're trying to test configuring a connection for ktls, why are you mocking the configuration? A better name might be "s2n_setup_valid_connection_for_ktls". I would also suggest removing the "fd" and "complete_handshake" args-- you never actually use "fd" for anything, and "complete_handshake" is the only option to produce a connection that's not valid for ktls. Instead, you can just set conn->handshake.message_number = 0 to reset the handshake.
tests/unit/s2n_ktls_test.c
Outdated
|
|
||
| S2N_RESULT s2n_test_configure_valid_ktls_connection(struct s2n_connection *conn) |
There was a problem hiding this comment.
#4066 (comment) I really think this shouldn't be "configure ktls connection". You're not actually configuring ktls. You're configuring a basic tls connection so that ktls can later be enabled.
tests/unit/s2n_ktls_test.c
Outdated
| /* write to conn->out buffer and assert error */ | ||
| EXPECT_SUCCESS(s2n_stuffer_write_bytes(&server_conn->out, &write_byte, 1)); |
There was a problem hiding this comment.
Nit: you can avoid write_byte if you just do
| /* write to conn->out buffer and assert error */ | |
| EXPECT_SUCCESS(s2n_stuffer_write_bytes(&server_conn->out, &write_byte, 1)); | |
| /* write to conn->out buffer and assert error */ | |
| EXPECT_SUCCESS(s2n_stuffer_write_uint8(&server_conn->out, 0)); |
Conversely if you want to only read one byte, s2n_stuffer_read_uint8 is the better choice. If you just want to clear it though, you just need to do s2n_stuffer_wipe().
toidiu
force-pushed
the
ak-ktls0_config_socket
branch
from
ef34e0f to
31b30fa
Compare
toidiu
force-pushed
the
ak-ktls0_config_socket
branch
from
31b30fa to
fe8dea1
Compare
Description of changes:
This PR adds entry points for enabling kTLS for a connection. enable_send/recv currently only attempt to enable the "tls" ULP on the socket. If this operation succeeds, its possible to enable kTLS by setting keys on the socket (following PR).
The most interesting part of this PR is the validation that needs to occur before we can enable kTLS. This is done before most kTLS operations; and I also added tests to make sure it behaves as expected.
Call-outs:
The following two functions will eventually be part of the public API:
int s2n_connection_ktls_enable_send(struct s2n_connection *conn);int s2n_connection_ktls_enable_recv(struct s2n_connection *conn);Testing:
Unit tests.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.