ktls: set keys on socket and enable ktls #4071

toidiu · 2023-06-22T06:00:05Z

Description of changes:

The PR retrieves the key material from the connection and sets it on the socket; thus enabling kTLS. Remember since its only possible to enable kTLS after the handshake is complete, and we do not delete the secret material, its possible to re-create the key_material.

At this point all data is encrypted/decrypted by the kernel-tls and therefore it is safe to NULL out the conn->send/recv callbacks.

Call-outs:

Since s2n_ktls_enable... are now destructive operations (offload IO to kernel), I have gated the functions only to tests to prevent accidentally turning the feature on.

Feature probe

This PR also re-enables the kTLS feature probe. This is necessary since we are attempting to retrieve kTLS specific headers from linux/tls.h. Its possible use probe for these and determine if kTLS should even be supported on the host. The current feature probe is simple since we are only adding support for AES_GCM_128 and TLS1.2. We will probably want atleast one more feature probe for TLS1.3. TBD is if we will want a feature probe for different ciphers.

All of these are 2-way decisions so I have kept the name simple for now: S2N_KTLS_SUPPORTED.

Testing:

Unit tests

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

tls/s2n_ktls_parameters.h

tls/s2n_ktls.c

tls/s2n_ktls_parameters.h

tls/s2n_ktls.c

tls/s2n_ktls.h

tls/s2n_config.c

tests/unit/s2n_ktls_test.c

tls/s2n_config.h

tests/unit/s2n_ktls_test.c

tls/s2n_ktls.h

tls/s2n_ktls.c

lrstewart · 2023-07-24T18:46:23Z

tls/s2n_ktls.c

+    if (ret < 0) {
+        RESULT_BAIL(S2N_ERR_KTLS_ENABLE_CRYPTO);
    }


Nit

Suggested change

if (ret < 0) {

RESULT_BAIL(S2N_ERR_KTLS_ENABLE_CRYPTO);

}

POSIX_ENSURE(ret >= 0, S2N_ERR_KTLS_ENABLE_CRYPTO);

I think in this case ENSURE is worse because signage is hard. I would like to keep this as ret < 0 so that next person (me) doesnt have to rethink the logic of >=.

tests/unit/s2n_ktls_test.c

lrstewart · 2023-07-24T18:59:45Z

tests/unit/s2n_ktls_test.c

+        /* negotiate */
+        EXPECT_SUCCESS(s2n_negotiate_test_server_and_client(server_conn, client_conn));
+        EXPECT_EQUAL(server_conn->actual_protocol_version, S2N_TLS12);


This shouldn't be a self-talk test. You're mocking out the inputs to s2n_ktls_init_aes128_gcm_crypto_info anyway. What do you gain from doing the handshake? It does complicate the test setup, forcing you to reuse the same connections for every test case.

An kind of just by definition, a self-talk test for a private method seems like a questionable pattern.

I was trying to test interactions between the client server but yea that not necessary to test aes_128... can be much simpler by mocking the connection.

tls/s2n_ktls.c

tests/unit/s2n_ktls_test.c

tls/s2n_ktls.c

tests/features/S2N_KTLS_SUPPORTED.c

commit eafb8a2 Author: Sam Clark <[email protected]> Date: Mon Jul 31 18:05:50 2023 -0400 shell -> bash commit 12071b8 Author: Sam Clark <[email protected]> Date: Mon Jul 31 17:59:09 2023 -0400 add ubuntu quickstart back to readme commit 10bf557 Author: Sam Clark <[email protected]> Date: Mon Jul 31 17:52:06 2023 -0400 fixes commit 74adf8d Author: Sam Clark <[email protected]> Date: Mon Jul 31 16:46:19 2023 -0400 fixes commit 0548d07 Author: Sam Clark <[email protected]> Date: Mon Jul 31 16:43:08 2023 -0400 consolidate commit cbe8f2d Author: Sam Clark <[email protected]> Date: Mon Jul 31 14:55:30 2023 -0400 remove old doc sections commit f194321 Author: Sam Clark <[email protected]> Date: Mon Jul 31 12:25:28 2023 -0400 more content commit 882eb1d Author: Sam Clark <[email protected]> Date: Mon Jul 31 09:08:24 2023 -0400 fixes commit ce37d0e Author: Sam Clark <[email protected]> Date: Mon Jul 31 09:03:45 2023 -0400 fixes commit 011d15f Author: Sam Clark <[email protected]> Date: Sat Jul 29 22:59:51 2023 -0400 cmake consuming commit 7feadc1 Author: Sam Clark <[email protected]> Date: Sat Jul 29 22:27:02 2023 -0400 fixes commit 2914950 Author: Sam Clark <[email protected]> Date: Sat Jul 29 21:34:24 2023 -0400 traditional make commit 02f9841 Author: Sam Clark <[email protected]> Date: Sat Jul 29 19:45:43 2023 -0400 s2n-tls build section commit 86c4983 Author: Sam Clark <[email protected]> Date: Sat Jul 29 11:56:32 2023 -0400 Update build documentation commit ea6d02a Author: Sam Clark <[email protected]> Date: Fri Jul 28 16:49:21 2023 -0400 bindings: release 0.0.35 (aws#4122) commit 35d08ba Author: Justin Zhang <[email protected]> Date: Fri Jul 28 12:31:21 2023 -0700 refactor(bench): separate out client and server connections in benching harness (aws#4113) Enables more better control of connections for benching experiments commit 65e74ca Author: Lindsay Stewart <[email protected]> Date: Wed Jul 26 02:26:40 2023 -0700 Print error for 32bit test (aws#4107) commit b0b253e Author: toidiu <[email protected]> Date: Wed Jul 26 00:30:44 2023 -0700 ktls: set keys on socket and enable ktls (aws#4071) commit 403d5e6 Author: Lindsay Stewart <[email protected]> Date: Tue Jul 25 16:03:09 2023 -0700 Trying to use an invalid ticket should not mutate state (aws#4110) commit bce2b1a Author: James Mayclin <[email protected]> Date: Tue Jul 25 14:44:33 2023 -0700 fix: get_session behavior for TLS 1.3 (aws#4104) commit 6881358 Author: Justin Zhang <[email protected]> Date: Tue Jul 25 10:10:21 2023 -0700 feat(bench): add different certificate signature algorithms to benchmarks (aws#4080) commit aab13d5 Author: Justin Zhang <[email protected]> Date: Mon Jul 24 18:17:30 2023 -0700 feat(bench): add memory bench with valgrind/massif (aws#4081) commit 20b0174 Author: Justin Zhang <[email protected]> Date: Mon Jul 24 13:26:32 2023 -0700 feat(bench): add historical performance benchmark (aws#4083) commit 5cc827d Author: Doug Chapman <[email protected]> Date: Thu Jul 20 11:50:50 2023 -0700 nix: pin corretto version (aws#4103)

toidiu mentioned this pull request Jun 22, 2023

Tracking issue: kTLS MVP #3711

Open

32 tasks

github-actions bot added the s2n-core team label Jun 22, 2023

toidiu force-pushed the ak-ktls0_set_keys branch 3 times, most recently from 5dea779 to b1d343f Compare June 23, 2023 19:47

toidiu force-pushed the ak-ktls0_set_keys branch from b1d343f to 79c9e9a Compare July 5, 2023 19:51

toidiu force-pushed the ak-ktls0_set_keys branch from 79c9e9a to 8e80caf Compare July 13, 2023 00:00

toidiu changed the title ~~Ak ktls0 set keys~~ ktls: set keys on socket and enable ktls Jul 13, 2023

toidiu force-pushed the ak-ktls0_set_keys branch 4 times, most recently from c43e0d8 to 8d8e9e5 Compare July 13, 2023 06:54

toidiu requested a review from lrstewart July 13, 2023 06:55

toidiu marked this pull request as ready for review July 13, 2023 06:55

lrstewart reviewed Jul 13, 2023

View reviewed changes

tls/s2n_ktls.c Outdated Show resolved Hide resolved

tls/s2n_ktls.c Outdated Show resolved Hide resolved

toidiu force-pushed the ak-ktls0_set_keys branch 2 times, most recently from 3457b2c to 3a19041 Compare July 17, 2023 04:06

toidiu mentioned this pull request Jul 17, 2023

ktls: send and recv cmsg #4073

Closed

toidiu requested a review from lrstewart July 17, 2023 17:40

lrstewart reviewed Jul 17, 2023

View reviewed changes

toidiu requested a review from lrstewart July 18, 2023 18:16

toidiu force-pushed the ak-ktls0_set_keys branch from f315b74 to 77a888c Compare July 19, 2023 05:05

lrstewart reviewed Jul 19, 2023

View reviewed changes

toidiu force-pushed the ak-ktls0_set_keys branch 3 times, most recently from 2b70cf1 to c42df01 Compare July 20, 2023 22:22

jmayclin mentioned this pull request Jul 21, 2023

fix: get_session behavior for TLS 1.3 #4104

Merged

toidiu requested a review from lrstewart July 21, 2023 22:09

lrstewart reviewed Jul 24, 2023

View reviewed changes

toidiu force-pushed the ak-ktls0_set_keys branch 3 times, most recently from 8273efe to 29785b0 Compare July 24, 2023 22:22

toidiu requested a review from lrstewart July 24, 2023 22:25

lrstewart approved these changes Jul 25, 2023

View reviewed changes

tls/s2n_ktls.c Outdated Show resolved Hide resolved

tests/unit/s2n_ktls_test.c Outdated Show resolved Hide resolved

tests/unit/s2n_ktls_test.c Outdated Show resolved Hide resolved

toidiu force-pushed the ak-ktls0_set_keys branch from 3a89cc5 to 8d61991 Compare July 25, 2023 05:38

maddeleine reviewed Jul 25, 2023

View reviewed changes

tls/s2n_ktls.c Show resolved Hide resolved

tests/features/S2N_KTLS_SUPPORTED.c Show resolved Hide resolved

maddeleine approved these changes Jul 25, 2023

View reviewed changes

toidiu added 11 commits July 25, 2023 17:52

ktls: set keys on socket and enable ktls

491c7f0

feedback

9cce2f3

feedback2

ca140e2

mock out setsockopt

d18d985

feedback3

d4aff7e

test s2n_ktls_init_aes128_gcm_crypto_info

dc6a38c

more testing

5f0fc6e

instead test that we are no longer using default IO

c0261aa

feedback

a3d36e2

unused when ktls not supported

e1d868f

feedback

5c7eb6b

toidiu force-pushed the ak-ktls0_set_keys branch from 8d61991 to 5c7eb6b Compare July 26, 2023 00:52

toidiu enabled auto-merge (squash) July 26, 2023 00:52

toidiu merged commit b0b253e into aws:main Jul 26, 2023

toidiu deleted the ak-ktls0_set_keys branch July 26, 2023 17:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ktls: set keys on socket and enable ktls #4071

ktls: set keys on socket and enable ktls #4071

toidiu commented Jun 22, 2023 •

edited

Loading

lrstewart Jul 24, 2023

toidiu Jul 24, 2023

lrstewart Jul 24, 2023

toidiu Jul 24, 2023 •

edited

Loading

ktls: set keys on socket and enable ktls #4071

ktls: set keys on socket and enable ktls #4071

Conversation

toidiu commented Jun 22, 2023 • edited Loading

Description of changes:

Call-outs:

Feature probe

Testing:

lrstewart Jul 24, 2023

Choose a reason for hiding this comment

toidiu Jul 24, 2023

Choose a reason for hiding this comment

lrstewart Jul 24, 2023

Choose a reason for hiding this comment

toidiu Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

toidiu commented Jun 22, 2023 •

edited

Loading

toidiu Jul 24, 2023 •

edited

Loading