Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Cleanup libcrypto errors #4733

Merged
merged 3 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crypto/s2n_composite_cipher_aes_sha.c
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ static int s2n_composite_cipher_aes_sha_initial_hmac(struct s2n_session_key *key
* will fail. Instead of defining a possibly dangerous default or hard coding this to 0x16 error out with BoringSSL and AWS-LC(AWSLC_API_VERSION <= 17).
*/
#if defined(OPENSSL_IS_BORINGSSL) || (defined(AWSLC_API_VERSION) && (AWSLC_API_VERSION <= 17))
POSIX_BAIL(S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API);
POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
#else
uint8_t ctrl_buf[S2N_TLS12_AAD_LEN];
struct s2n_blob ctrl_blob = { 0 };
Expand Down
6 changes: 3 additions & 3 deletions crypto/s2n_kyber_evp.c
Original file line number Diff line number Diff line change
Expand Up @@ -92,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_NO_SUPPORTED_LIBCRYPTO_API);
POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
lrstewart marked this conversation as resolved.
Show resolved Hide resolved
}

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_NO_SUPPORTED_LIBCRYPTO_API);
POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
}

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_NO_SUPPORTED_LIBCRYPTO_API);
POSIX_BAIL(S2N_ERR_UNIMPLEMENTED);
}

#endif
2 changes: 1 addition & 1 deletion error/s2n_errno.c
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,6 @@ static const char *no_such_error = "Internal s2n error";
ERR_ENTRY(S2N_ERR_ARRAY_INDEX_OOB, "Array index out of bounds") \
ERR_ENTRY(S2N_ERR_FREE_STATIC_BLOB, "Cannot free a static blob") \
ERR_ENTRY(S2N_ERR_RESIZE_STATIC_BLOB, "Cannot resize a static blob") \
ERR_ENTRY(S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API, "libcrypto does not support this API") \
ERR_ENTRY(S2N_ERR_RECORD_LENGTH_TOO_LARGE, "Record length exceeds protocol version maximum") \
ERR_ENTRY(S2N_ERR_SET_DUPLICATE_VALUE, "Set already contains the provided value") \
ERR_ENTRY(S2N_ERR_ASYNC_CALLBACK_FAILED, "Callback associated with async private keys function has failed") \
Expand Down Expand Up @@ -314,6 +313,7 @@ static const char *no_such_error = "Internal s2n error";
ERR_ENTRY(S2N_ERR_TOO_MANY_CAS, "Too many certificate authorities in trust store"); \
ERR_ENTRY(S2N_ERR_BAD_HEX, "Could not parse malformed hex string"); \
ERR_ENTRY(S2N_ERR_CONFIG_NULL_BEFORE_CH_CALLBACK, "Config set to NULL before client hello callback. This should not be possible outside of tests."); \
ERR_ENTRY(S2N_ERR_API_UNSUPPORTED_BY_LIBCRYPTO, "The invoked s2n-tls API is not supported by the libcrypto") \
/* clang-format on */

#define ERR_STR_CASE(ERR, str) \
Expand Down
4 changes: 2 additions & 2 deletions error/s2n_errno.h
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ typedef enum {
S2N_ERR_ARRAY_INDEX_OOB,
S2N_ERR_FREE_STATIC_BLOB,
S2N_ERR_RESIZE_STATIC_BLOB,
S2N_ERR_NO_SUPPORTED_LIBCRYPTO_API,
S2N_ERR_RECORD_LENGTH_TOO_LARGE,
S2N_ERR_SET_DUPLICATE_VALUE,
S2N_ERR_INVALID_PARSED_EXTENSIONS,
Expand All @@ -236,6 +235,7 @@ typedef enum {
S2N_ERR_RETRIEVE_FORK_GENERATION_NUMBER,
S2N_ERR_LIBCRYPTO_VERSION_NUMBER_MISMATCH,
S2N_ERR_LIBCRYPTO_VERSION_NAME_MISMATCH,
S2N_ERR_INTERNAL_LIBCRYPTO_ERROR,
S2N_ERR_OSSL_PROVIDER,
S2N_ERR_BAD_HEX,
S2N_ERR_TEST_ASSERTION,
Expand Down Expand Up @@ -318,7 +318,6 @@ typedef enum {
S2N_ERR_KEYING_MATERIAL_EXPIRED,
S2N_ERR_SECRET_SCHEDULE_STATE,
S2N_ERR_CERT_OWNERSHIP,
S2N_ERR_INTERNAL_LIBCRYPTO_ERROR,
S2N_ERR_HANDSHAKE_NOT_COMPLETE,
S2N_ERR_KTLS_MANAGED_IO,
S2N_ERR_KTLS_UNSUPPORTED_PLATFORM,
Expand All @@ -331,6 +330,7 @@ typedef enum {
S2N_ERR_SECURITY_POLICY_INCOMPATIBLE_CERT,
S2N_ERR_INVALID_SERIALIZED_CONNECTION,
S2N_ERR_TOO_MANY_CAS,
S2N_ERR_API_UNSUPPORTED_BY_LIBCRYPTO,
S2N_ERR_T_USAGE_END,
} s2n_error;

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/s2n_cert_authorities_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ int main(int argc, char **argv)
} else {
EXPECT_FAILURE_WITH_ERRNO(
s2n_config_set_cert_authorities_from_trust_store(config),
S2N_ERR_INTERNAL_LIBCRYPTO_ERROR);
S2N_ERR_API_UNSUPPORTED_BY_LIBCRYPTO);
EXPECT_EQUAL(config->cert_authorities.size, 0);
}
};
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/s2n_client_hello_retry_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,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_NO_SUPPORTED_LIBCRYPTO_API);
EXPECT_FAILURE_WITH_ERRNO(s2n_server_hello_retry_recv(conn), S2N_ERR_INVALID_HELLO_RETRY);

EXPECT_SUCCESS(s2n_connection_free(conn));
} else {
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/s2n_client_key_share_extension_pq_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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_NO_SUPPORTED_LIBCRYPTO_API);
POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_UNIMPLEMENTED);

const struct s2n_kem_group *kem_group = kem_group_params->kem_group;
POSIX_ENSURE_REF(kem_group);
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/s2n_kex_with_kem_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -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_NO_SUPPORTED_LIBCRYPTO_API */
* set s2n_errno to S2N_ERR_UNIMPLEMENTED */
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_NO_SUPPORTED_LIBCRYPTO_API);
POSIX_ENSURE_EQ(s2n_errno, S2N_ERR_UNIMPLEMENTED);

POSIX_GUARD(s2n_connection_free(server_conn));
s2n_errno = 0;
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/s2n_pq_kem_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ int main()
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 {
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);
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);
}
}

Expand Down
25 changes: 18 additions & 7 deletions tests/unit/s2n_server_key_share_extension_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -555,21 +555,32 @@ int main(int argc, char **argv)

/* If PQ is disabled, the client will not have sent PQ IDs/keyshares in the ClientHello;
* if the server responded with a PQ keyshare, we should error. */
if (!s2n_pq_is_enabled()) {
struct s2n_connection *client_conn = NULL;
EXPECT_NOT_NULL(client_conn = s2n_connection_new(S2N_CLIENT));
{
DEFER_CLEANUP(struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(client_conn);
client_conn->security_policy_override = &test_security_policy;

/* The connection is set up to be receiving a HelloRetryRequest in order to exit
* s2n_server_key_share_extension.recv earlier for testing the success case.
*/
client_conn->handshake.state_machine = S2N_STATE_MACHINE_TLS13;
EXPECT_SUCCESS(s2n_set_connection_hello_retry_flags(client_conn));
EXPECT_TRUE(s2n_is_hello_retry_message(client_conn));
lrstewart marked this conversation as resolved.
Show resolved Hide resolved

uint8_t iana_buffer[2];
struct s2n_blob iana_blob = { 0 };
struct s2n_stuffer iana_stuffer = { 0 };
EXPECT_SUCCESS(s2n_blob_init(&iana_blob, iana_buffer, 2));
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_NO_SUPPORTED_LIBCRYPTO_API);

EXPECT_SUCCESS(s2n_connection_free(client_conn));
int ret = s2n_server_key_share_extension.recv(client_conn, &iana_stuffer);
if (s2n_pq_is_enabled()) {
EXPECT_SUCCESS(ret);
} else {
EXPECT_FAILURE_WITH_ERRNO(ret, S2N_ERR_ECDHE_UNSUPPORTED_CURVE);
}
}

/* Test s2n_server_key_share_extension.recv with KAT pq key shares */
Expand Down Expand Up @@ -782,7 +793,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_NO_SUPPORTED_LIBCRYPTO_API);
EXPECT_FAILURE_WITH_ERRNO(s2n_server_key_share_send_check_pq_hybrid(conn), S2N_ERR_UNIMPLEMENTED);
}

if (s2n_pq_is_enabled()) {
Expand Down
2 changes: 1 addition & 1 deletion tls/extensions/s2n_cert_authorities.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ static S2N_RESULT s2n_cert_authorities_set_from_trust_store(struct s2n_config *c
RESULT_GUARD_POSIX(s2n_stuffer_extract_blob(&output, &config->cert_authorities));
return S2N_RESULT_OK;
#else
RESULT_BAIL(S2N_ERR_INTERNAL_LIBCRYPTO_ERROR);
RESULT_BAIL(S2N_ERR_API_UNSUPPORTED_BY_LIBCRYPTO);
#endif
}

Expand Down
2 changes: 1 addition & 1 deletion tls/extensions/s2n_client_key_share.c
Original file line number Diff line number Diff line change
Expand Up @@ -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_NO_SUPPORTED_LIBCRYPTO_API);
POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_UNIMPLEMENTED);

const struct s2n_kem_group *kem_group = kem_group_params->kem_group;
POSIX_ENSURE_REF(kem_group);
Expand Down
6 changes: 3 additions & 3 deletions tls/extensions/s2n_server_key_share.c
Original file line number Diff line number Diff line change
Expand Up @@ -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_NO_SUPPORTED_LIBCRYPTO_API);
POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_UNIMPLEMENTED);

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;
Expand Down Expand Up @@ -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_NO_SUPPORTED_LIBCRYPTO_API);
POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_UNIMPLEMENTED);

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);
Expand Down Expand Up @@ -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_NO_SUPPORTED_LIBCRYPTO_API);
POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_ECDHE_UNSUPPORTED_CURVE);

const struct s2n_kem_preferences *kem_pref = NULL;
POSIX_GUARD(s2n_connection_get_kem_preferences(conn, &kem_pref));
Expand Down
2 changes: 1 addition & 1 deletion tls/s2n_kex.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,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_NO_SUPPORTED_LIBCRYPTO_API);
RESULT_ENSURE(s2n_pq_is_enabled(), S2N_ERR_UNIMPLEMENTED);

const struct s2n_kem_preferences *kem_preferences = NULL;
RESULT_GUARD_POSIX(s2n_connection_get_kem_preferences(conn, &kem_preferences));
Expand Down
2 changes: 1 addition & 1 deletion tls/s2n_server_hello_retry.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ int s2n_server_hello_retry_recv(struct s2n_connection *conn)
if (server_preferred_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_NO_SUPPORTED_LIBCRYPTO_API);
POSIX_ENSURE(s2n_pq_is_enabled(), S2N_ERR_INVALID_HELLO_RETRY);
new_key_share_requested = (server_preferred_kem_group != client_preferred_kem_group);
}

Expand Down
Loading