From d3fd5c7d36b9e444f87f2294863441b7e9fb4e00 Mon Sep 17 00:00:00 2001 From: Alex Weibel Date: Wed, 15 Nov 2023 12:01:24 -0800 Subject: [PATCH] Remove S2N_ERR_PQ_DISABLED --- crypto/s2n_kyber_evp.c | 9 ++-- error/s2n_errno.c | 1 - error/s2n_errno.h | 1 - tests/unit/s2n_client_hello_retry_test.c | 2 +- .../s2n_client_key_share_extension_pq_test.c | 2 +- tests/unit/s2n_kex_with_kem_test.c | 4 +- tests/unit/s2n_pq_kem_test.c | 51 ++++++------------- .../s2n_server_key_share_extension_test.c | 4 +- tls/extensions/s2n_client_key_share.c | 2 +- tls/extensions/s2n_server_key_share.c | 6 +-- tls/s2n_kex.c | 2 +- tls/s2n_server_hello_retry.c | 2 +- 12 files changed, 31 insertions(+), 55 deletions(-) diff --git a/crypto/s2n_kyber_evp.c b/crypto/s2n_kyber_evp.c index 44d11f33db4..277ffc88e39 100644 --- a/crypto/s2n_kyber_evp.c +++ b/crypto/s2n_kyber_evp.c @@ -30,7 +30,6 @@ DEFINE_POINTER_CLEANUP_FUNC(EVP_PKEY_CTX *, EVP_PKEY_CTX_free); int s2n_kyber_evp_generate_keypair(IN const struct s2n_kem *kem, OUT uint8_t *public_key, OUT uint8_t *secret_key) { - POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED); DEFER_CLEANUP(EVP_PKEY_CTX *kyber_pkey_ctx = EVP_PKEY_CTX_new_id(EVP_PKEY_KEM, NULL), EVP_PKEY_CTX_free_pointer); POSIX_GUARD_PTR(kyber_pkey_ctx); POSIX_GUARD_OSSL(EVP_PKEY_CTX_kem_set_params(kyber_pkey_ctx, kem->kem_nid), S2N_ERR_PQ_CRYPTO); @@ -53,7 +52,6 @@ int s2n_kyber_evp_generate_keypair(IN const struct s2n_kem *kem, OUT uint8_t *pu int s2n_kyber_evp_encapsulate(IN const struct s2n_kem *kem, OUT uint8_t *ciphertext, OUT uint8_t *shared_secret, IN const uint8_t *public_key) { - POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED); DEFER_CLEANUP(EVP_PKEY *kyber_pkey = EVP_PKEY_kem_new_raw_public_key(kem->kem_nid, public_key, kem->public_key_length), EVP_PKEY_free_pointer); POSIX_GUARD_PTR(kyber_pkey); @@ -74,7 +72,6 @@ int s2n_kyber_evp_encapsulate(IN const struct s2n_kem *kem, OUT uint8_t *ciphert int s2n_kyber_evp_decapsulate(IN const struct s2n_kem *kem, OUT uint8_t *shared_secret, IN const uint8_t *ciphertext, IN const uint8_t *private_key) { - POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED); DEFER_CLEANUP(EVP_PKEY *kyber_pkey = EVP_PKEY_kem_new_raw_secret_key(kem->kem_nid, private_key, kem->private_key_length), EVP_PKEY_free_pointer); POSIX_GUARD_PTR(kyber_pkey); @@ -95,19 +92,19 @@ int s2n_kyber_evp_decapsulate(IN const struct s2n_kem *kem, OUT uint8_t *shared_ int s2n_kyber_evp_generate_keypair(IN const struct s2n_kem *kem, OUT uint8_t *public_key, OUT uint8_t *secret_key) { - POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); + POSIX_BAIL(S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); } int s2n_kyber_evp_encapsulate(IN const struct s2n_kem *kem, OUT uint8_t *ciphertext, OUT uint8_t *shared_secret, IN const uint8_t *public_key) { - POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); + POSIX_BAIL(S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); } int s2n_kyber_evp_decapsulate(IN const struct s2n_kem *kem, OUT uint8_t *shared_secret, IN const uint8_t *ciphertext, IN const uint8_t *secret_key) { - POSIX_BAIL(S2N_ERR_UNIMPLEMENTED); + POSIX_BAIL(S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); } #endif diff --git a/error/s2n_errno.c b/error/s2n_errno.c index fbe52416b5a..f71c734aac2 100644 --- a/error/s2n_errno.c +++ b/error/s2n_errno.c @@ -263,7 +263,6 @@ static const char *no_such_error = "Internal s2n error"; ERR_ENTRY(S2N_ERR_INVALID_STATE, "Invalid state, this is the result of invalid use of an API. Check the API documentation for the function that raised this error for more info") \ ERR_ENTRY(S2N_ERR_UNSUPPORTED_WITH_QUIC, "Functionality not supported when running with QUIC support enabled") \ ERR_ENTRY(S2N_ERR_PQ_CRYPTO, "An error occurred in a post-quantum crypto function") \ - ERR_ENTRY(S2N_ERR_PQ_DISABLED, "Post-quantum crypto is disabled") \ ERR_ENTRY(S2N_ERR_DUPLICATE_PSK_IDENTITIES, "The list of pre-shared keys provided contains duplicate psk identities") \ ERR_ENTRY(S2N_ERR_OFFERED_PSKS_TOO_LONG, "The total pre-shared key data is too long to send over the wire") \ ERR_ENTRY(S2N_ERR_INVALID_SESSION_TICKET, "Session ticket data is not valid") \ diff --git a/error/s2n_errno.h b/error/s2n_errno.h index 1ff5070717e..c51881fb12e 100644 --- a/error/s2n_errno.h +++ b/error/s2n_errno.h @@ -222,7 +222,6 @@ typedef enum { S2N_ERR_ASYNC_CALLBACK_FAILED, S2N_ERR_ASYNC_MORE_THAN_ONE, S2N_ERR_PQ_CRYPTO, - S2N_ERR_PQ_DISABLED, S2N_ERR_INVALID_CERT_STATE, S2N_ERR_INVALID_EARLY_DATA_STATE, S2N_ERR_PKEY_CTX_INIT, diff --git a/tests/unit/s2n_client_hello_retry_test.c b/tests/unit/s2n_client_hello_retry_test.c index 5e28c45be7e..077d6e36bcd 100644 --- a/tests/unit/s2n_client_hello_retry_test.c +++ b/tests/unit/s2n_client_hello_retry_test.c @@ -189,7 +189,7 @@ int main(int argc, char **argv) conn->kex_params.server_kem_group_params.kem_group = kem_pref->tls13_kem_groups[0]; EXPECT_NULL(conn->kex_params.server_ecc_evp_params.negotiated_curve); - EXPECT_FAILURE_WITH_ERRNO(s2n_server_hello_retry_recv(conn), S2N_ERR_PQ_DISABLED); + EXPECT_FAILURE_WITH_ERRNO(s2n_server_hello_retry_recv(conn), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); EXPECT_SUCCESS(s2n_connection_free(conn)); } else { diff --git a/tests/unit/s2n_client_key_share_extension_pq_test.c b/tests/unit/s2n_client_key_share_extension_pq_test.c index 38f9da6d561..bae35f96934 100644 --- a/tests/unit/s2n_client_key_share_extension_pq_test.c +++ b/tests/unit/s2n_client_key_share_extension_pq_test.c @@ -941,7 +941,7 @@ static int s2n_generate_pq_hybrid_key_share_for_test(struct s2n_stuffer *out, st POSIX_ENSURE_REF(kem_group_params); /* This function should never be called when PQ is disabled */ - POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED); + POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); const struct s2n_kem_group *kem_group = kem_group_params->kem_group; POSIX_ENSURE_REF(kem_group); diff --git a/tests/unit/s2n_kex_with_kem_test.c b/tests/unit/s2n_kex_with_kem_test.c index 304718a6937..865c2495f73 100644 --- a/tests/unit/s2n_kex_with_kem_test.c +++ b/tests/unit/s2n_kex_with_kem_test.c @@ -128,14 +128,14 @@ static int assert_pq_disabled_checks(struct s2n_cipher_suite *cipher_suite, cons /* If PQ is disabled: * s2n_check_kem() (s2n_hybrid_ecdhe_kem.connection_supported) should indicate that the connection is not supported * s2n_configure_kem() (s2n_hybrid_ecdhe_kem.configure_connection) should return S2N_RESULT_ERROR - * set s2n_errno to S2N_ERR_PQ_DISABLED */ + * set s2n_errno to S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API */ bool connection_supported = true; POSIX_GUARD_RESULT(s2n_hybrid_ecdhe_kem.connection_supported(cipher_suite, server_conn, &connection_supported)); POSIX_ENSURE_EQ(connection_supported, false); POSIX_ENSURE_EQ(s2n_result_is_error(s2n_hybrid_ecdhe_kem.configure_connection(cipher_suite, server_conn)), true); - POSIX_ENSURE_EQ(s2n_errno, S2N_ERR_PQ_DISABLED); + POSIX_ENSURE_EQ(s2n_errno, S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); POSIX_GUARD(s2n_connection_free(server_conn)); s2n_errno = 0; diff --git a/tests/unit/s2n_pq_kem_test.c b/tests/unit/s2n_pq_kem_test.c index da838c2a7e4..f3631f801bc 100644 --- a/tests/unit/s2n_pq_kem_test.c +++ b/tests/unit/s2n_pq_kem_test.c @@ -25,17 +25,11 @@ struct s2n_kem_test_vector { const struct s2n_kem *kem; - bool (*asm_is_enabled)(); - S2N_RESULT (*enable_asm)(); - S2N_RESULT (*disable_asm)(); }; static const struct s2n_kem_test_vector test_vectors[] = { { .kem = &s2n_kyber_512_r3, - .asm_is_enabled = s2n_pq_no_asm_available, - .enable_asm = s2n_pq_noop_asm, - .disable_asm = s2n_pq_noop_asm, } }; @@ -75,36 +69,23 @@ int main() EXPECT_SUCCESS(s2n_alloc(&ciphertext, kem->ciphertext_length)); if (s2n_pq_is_enabled()) { - /* Run the tests for C and assembly implementations (where available) */ - s2n_result (*asm_toggle_funcs[])(void) = { vector.disable_asm, vector.enable_asm }; - - for (size_t j = 0; j < s2n_array_len(asm_toggle_funcs); j++) { - EXPECT_OK(asm_toggle_funcs[j]()); - - /* Test a successful round-trip: keygen->enc->dec */ - EXPECT_PQ_KEM_SUCCESS(kem->generate_keypair(kem, public_key.data, private_key.data)); - EXPECT_PQ_KEM_SUCCESS(kem->encapsulate(kem, ciphertext.data, client_shared_secret.data, public_key.data)); - EXPECT_PQ_KEM_SUCCESS(kem->decapsulate(kem, server_shared_secret.data, ciphertext.data, private_key.data)); - EXPECT_BYTEARRAY_EQUAL(server_shared_secret.data, client_shared_secret.data, kem->shared_secret_key_length); - - /* By design, if an invalid private key + ciphertext pair is provided to decapsulate(), - * the function should still succeed (return S2N_SUCCESS); however, the shared secret - * that was "decapsulated" will be a garbage random value. */ - ciphertext.data[0] ^= 1; /* Flip a bit to invalidate the ciphertext */ - - EXPECT_PQ_KEM_SUCCESS(kem->decapsulate(kem, server_shared_secret.data, ciphertext.data, private_key.data)); - EXPECT_BYTEARRAY_NOT_EQUAL(server_shared_secret.data, client_shared_secret.data, kem->shared_secret_key_length); - } + /* Test a successful round-trip: keygen->enc->dec */ + EXPECT_PQ_KEM_SUCCESS(kem->generate_keypair(kem, public_key.data, private_key.data)); + EXPECT_PQ_KEM_SUCCESS(kem->encapsulate(kem, ciphertext.data, client_shared_secret.data, public_key.data)); + EXPECT_PQ_KEM_SUCCESS(kem->decapsulate(kem, server_shared_secret.data, ciphertext.data, private_key.data)); + EXPECT_BYTEARRAY_EQUAL(server_shared_secret.data, client_shared_secret.data, kem->shared_secret_key_length); + + /* By design, if an invalid private key + ciphertext pair is provided to decapsulate(), + * the function should still succeed (return S2N_SUCCESS); however, the shared secret + * that was "decapsulated" will be a garbage random value. */ + ciphertext.data[0] ^= 1; /* Flip a bit to invalidate the ciphertext */ + + EXPECT_PQ_KEM_SUCCESS(kem->decapsulate(kem, server_shared_secret.data, ciphertext.data, private_key.data)); + EXPECT_BYTEARRAY_NOT_EQUAL(server_shared_secret.data, client_shared_secret.data, kem->shared_secret_key_length); } else { -#if !defined(S2N_LIBCRYPTO_SUPPORTS_KYBER) - EXPECT_FAILURE_WITH_ERRNO(kem->generate_keypair(kem, public_key.data, private_key.data), S2N_ERR_UNIMPLEMENTED); - EXPECT_FAILURE_WITH_ERRNO(kem->encapsulate(kem, ciphertext.data, client_shared_secret.data, public_key.data), S2N_ERR_UNIMPLEMENTED); - EXPECT_FAILURE_WITH_ERRNO(kem->decapsulate(kem, server_shared_secret.data, ciphertext.data, private_key.data), S2N_ERR_UNIMPLEMENTED); -#else - EXPECT_FAILURE_WITH_ERRNO(kem->generate_keypair(kem, public_key.data, private_key.data), S2N_ERR_PQ_DISABLED); - EXPECT_FAILURE_WITH_ERRNO(kem->encapsulate(kem, ciphertext.data, client_shared_secret.data, public_key.data), S2N_ERR_PQ_DISABLED); - EXPECT_FAILURE_WITH_ERRNO(kem->decapsulate(kem, server_shared_secret.data, ciphertext.data, private_key.data), S2N_ERR_PQ_DISABLED); -#endif + EXPECT_FAILURE_WITH_ERRNO(kem->generate_keypair(kem, public_key.data, private_key.data), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); + EXPECT_FAILURE_WITH_ERRNO(kem->encapsulate(kem, ciphertext.data, client_shared_secret.data, public_key.data), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); + EXPECT_FAILURE_WITH_ERRNO(kem->decapsulate(kem, server_shared_secret.data, ciphertext.data, private_key.data), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); } } diff --git a/tests/unit/s2n_server_key_share_extension_test.c b/tests/unit/s2n_server_key_share_extension_test.c index ab3802623ff..44569f0f81e 100644 --- a/tests/unit/s2n_server_key_share_extension_test.c +++ b/tests/unit/s2n_server_key_share_extension_test.c @@ -567,7 +567,7 @@ int main(int argc, char **argv) EXPECT_SUCCESS(s2n_stuffer_init(&iana_stuffer, &iana_blob)); EXPECT_SUCCESS(s2n_stuffer_write_uint16(&iana_stuffer, test_security_policy.kem_preferences->tls13_kem_groups[0]->iana_id)); - EXPECT_FAILURE_WITH_ERRNO(s2n_server_key_share_extension.recv(client_conn, &iana_stuffer), S2N_ERR_PQ_DISABLED); + EXPECT_FAILURE_WITH_ERRNO(s2n_server_key_share_extension.recv(client_conn, &iana_stuffer), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); EXPECT_SUCCESS(s2n_connection_free(client_conn)); } @@ -782,7 +782,7 @@ int main(int argc, char **argv) EXPECT_NOT_NULL(conn = s2n_connection_new(S2N_SERVER)); if (!s2n_pq_is_enabled()) { - EXPECT_FAILURE_WITH_ERRNO(s2n_server_key_share_send_check_pq_hybrid(conn), S2N_ERR_PQ_DISABLED); + EXPECT_FAILURE_WITH_ERRNO(s2n_server_key_share_send_check_pq_hybrid(conn), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); } if (s2n_pq_is_enabled()) { diff --git a/tls/extensions/s2n_client_key_share.c b/tls/extensions/s2n_client_key_share.c index a1f3a3b4e52..1226ee15d4f 100644 --- a/tls/extensions/s2n_client_key_share.c +++ b/tls/extensions/s2n_client_key_share.c @@ -106,7 +106,7 @@ static int s2n_generate_pq_hybrid_key_share(struct s2n_stuffer *out, struct s2n_ POSIX_ENSURE_REF(kem_group_params); /* This function should never be called when PQ is disabled */ - POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED); + POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); const struct s2n_kem_group *kem_group = kem_group_params->kem_group; POSIX_ENSURE_REF(kem_group); diff --git a/tls/extensions/s2n_server_key_share.c b/tls/extensions/s2n_server_key_share.c index d5c63d5515b..819b852116f 100644 --- a/tls/extensions/s2n_server_key_share.c +++ b/tls/extensions/s2n_server_key_share.c @@ -39,7 +39,7 @@ static int s2n_server_key_share_generate_pq_hybrid(struct s2n_connection *conn, POSIX_ENSURE_REF(out); POSIX_ENSURE_REF(conn); - POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED); + POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); struct s2n_kem_group_params *server_kem_group_params = &conn->kex_params.server_kem_group_params; struct s2n_kem_params *client_kem_params = &conn->kex_params.client_kem_group_params.kem_params; @@ -74,7 +74,7 @@ int s2n_server_key_share_send_check_pq_hybrid(struct s2n_connection *conn) { POSIX_ENSURE_REF(conn); - POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED); + POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); POSIX_ENSURE_REF(conn->kex_params.server_kem_group_params.kem_group); POSIX_ENSURE_REF(conn->kex_params.server_kem_group_params.kem_params.kem); @@ -165,7 +165,7 @@ static int s2n_server_key_share_recv_pq_hybrid(struct s2n_connection *conn, uint /* If PQ is disabled, the client should not have sent any PQ IDs * in the supported_groups list of the initial ClientHello */ - POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED); + POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); const struct s2n_kem_preferences *kem_pref = NULL; POSIX_GUARD(s2n_connection_get_kem_preferences(conn, &kem_pref)); diff --git a/tls/s2n_kex.c b/tls/s2n_kex.c index 880fd9a13e1..19cac757856 100644 --- a/tls/s2n_kex.c +++ b/tls/s2n_kex.c @@ -113,7 +113,7 @@ static S2N_RESULT s2n_configure_kem(const struct s2n_cipher_suite *cipher_suite, RESULT_ENSURE_REF(cipher_suite); RESULT_ENSURE_REF(conn); - RESULT_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED); + RESULT_ENSURE(s2n_pq_is_enabled(), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); const struct s2n_kem_preferences *kem_preferences = NULL; RESULT_GUARD_POSIX(s2n_connection_get_kem_preferences(conn, &kem_preferences)); diff --git a/tls/s2n_server_hello_retry.c b/tls/s2n_server_hello_retry.c index dac22e159e4..54d8dcb022d 100644 --- a/tls/s2n_server_hello_retry.c +++ b/tls/s2n_server_hello_retry.c @@ -102,7 +102,7 @@ int s2n_server_hello_retry_recv(struct s2n_connection *conn) if (kem_group != NULL) { /* If PQ is disabled, the client should not have sent any PQ IDs * in the supported_groups list of the initial ClientHello */ - POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_PQ_DISABLED); + POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API); new_key_share_requested = (kem_group != conn->kex_params.client_kem_group_params.kem_group); }