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

test: remove load system certs functionality for s2n_default_tls13_config #4897

Merged
merged 4 commits into from
Nov 19, 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
15 changes: 6 additions & 9 deletions tests/fuzz/s2n_certificate_extensions_parse_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,6 @@ static uint8_t verify_host_accept_everything(const char *host_name, size_t host_
/* This test is for TLS versions 1.3 and up only */
static const uint8_t TLS_VERSIONS[] = {S2N_TLS13};

int s2n_fuzz_init(int *argc, char **argv[])
{
/* Initialize the trust store */
toidiu marked this conversation as resolved.
Show resolved Hide resolved
POSIX_GUARD_RESULT(s2n_config_testing_defaults_init_tls13_certs());
POSIX_GUARD(s2n_enable_tls13_in_test());
return S2N_SUCCESS;
}

int s2n_fuzz_test(const uint8_t *buf, size_t len)
{
/* We need at least one byte of input to set parameters */
Expand All @@ -67,8 +59,13 @@ int s2n_fuzz_test(const uint8_t *buf, size_t len)
POSIX_GUARD(s2n_stuffer_alloc(&fuzz_stuffer, len));
POSIX_GUARD(s2n_stuffer_write_bytes(&fuzz_stuffer, buf, len));

DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(config);
POSIX_GUARD(s2n_config_set_cipher_preferences(config, "20240503"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yay for simplicity! The old function definitely seems a bit tricky.

It seems like there are two things the old function was doing, which were presumably important for some reason

  1. enabling TLS 1.3
  2. loading system certs

Can we call those out (+ why) in a comment here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. enabling TLS 1.3

There is a comment at the top of this file that we need tls13.

  1. loading system certs

Ran locally and this line fails if we remove cert loading so its implicitly documented.


struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT);
POSIX_ENSURE_REF(client_conn);
POSIX_GUARD(s2n_connection_set_config(client_conn, config));

/* Pull a byte off the libfuzzer input and use it to set parameters */
uint8_t randval = 0;
Expand Down Expand Up @@ -115,4 +112,4 @@ int s2n_fuzz_test(const uint8_t *buf, size_t len)
return S2N_SUCCESS;
}

S2N_FUZZ_TARGET(s2n_fuzz_init, s2n_fuzz_test, NULL)
S2N_FUZZ_TARGET(NULL, s2n_fuzz_test, NULL)
6 changes: 0 additions & 6 deletions tls/s2n_config.c
Original file line number Diff line number Diff line change
Expand Up @@ -251,12 +251,6 @@ int s2n_config_defaults_init(void)
return S2N_SUCCESS;
}

S2N_RESULT s2n_config_testing_defaults_init_tls13_certs(void)
{
RESULT_GUARD_POSIX(s2n_config_load_system_certs(&s2n_default_tls13_config));
return S2N_RESULT_OK;
}

void s2n_wipe_static_configs(void)
{
s2n_config_cleanup(&s2n_default_fips_config);
Expand Down
1 change: 0 additions & 1 deletion tls/s2n_config.h
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,6 @@ struct s2n_config {
S2N_CLEANUP_RESULT s2n_config_ptr_free(struct s2n_config **config);

int s2n_config_defaults_init(void);
S2N_RESULT s2n_config_testing_defaults_init_tls13_certs(void);
struct s2n_config *s2n_fetch_default_config(void);
int s2n_config_set_unsafe_for_testing(struct s2n_config *config);

Expand Down
Loading