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: Increase received signature scheme limit #4544

Merged
merged 3 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from 2 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 error/s2n_errno.c
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ static const char *no_such_error = "Internal s2n error";
ERR_ENTRY(S2N_ERR_CLIENT_MODE, "Operation not allowed in client mode") \
ERR_ENTRY(S2N_ERR_CLIENT_MODE_DISABLED, "client connections not allowed") \
ERR_ENTRY(S2N_ERR_TOO_MANY_CERTIFICATES, "only 1 certificate is supported in client mode") \
ERR_ENTRY(S2N_ERR_TOO_MANY_SIGNATURE_SCHEMES, "Max supported length of SignatureAlgorithms/SignatureSchemes list is 32") \
ERR_ENTRY(S2N_ERR_TOO_MANY_SIGNATURE_SCHEMES, "Max supported length of SignatureAlgorithms/SignatureSchemes list is 128") \
ERR_ENTRY(S2N_ERR_CLIENT_AUTH_NOT_SUPPORTED_IN_FIPS_MODE, "Client Auth is not supported when in FIPS mode") \
ERR_ENTRY(S2N_ERR_INVALID_BASE64, "invalid base64 encountered") \
ERR_ENTRY(S2N_ERR_INVALID_HEX, "invalid HEX encountered") \
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/s2n_client_signature_algorithms_extension_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_client_signature_algorithms_extension.recv(server_conn, &io));
EXPECT_EQUAL(s2n_stuffer_data_available(&io), 0);

EXPECT_TRUE(server_conn->handshake_params.client_sig_hash_algs.len > 0);
EXPECT_TRUE(server_conn->handshake_params.peer_sig_scheme_list.len > 0);

s2n_stuffer_free(&io);
s2n_connection_free(client_conn);
Expand All @@ -91,7 +91,7 @@ int main(int argc, char **argv)

/* If a valid algorithm is offered among unknown algorithms, the valid one should be chosen */
EXPECT_SUCCESS(s2n_client_signature_algorithms_extension.recv(conn, &signature_algorithms_extension));
EXPECT_EQUAL(conn->handshake_params.client_sig_hash_algs.len, sig_hash_algs.len);
EXPECT_EQUAL(conn->handshake_params.peer_sig_scheme_list.len, sig_hash_algs.len);
EXPECT_OK(s2n_signature_algorithm_select(conn));
EXPECT_EQUAL(conn->handshake_params.server_cert_sig_scheme->iana_value, TLS_SIGNATURE_SCHEME_RSA_PKCS1_SHA384);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_server_signature_algorithms_extension.recv(client_conn, &io));
EXPECT_EQUAL(s2n_stuffer_data_available(&io), 0);

EXPECT_TRUE(client_conn->handshake_params.server_sig_hash_algs.len > 0);
EXPECT_TRUE(client_conn->handshake_params.peer_sig_scheme_list.len > 0);

s2n_stuffer_free(&io);
EXPECT_SUCCESS(s2n_connection_free(server_conn));
Expand Down
66 changes: 45 additions & 21 deletions tests/unit/s2n_signature_algorithms_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ int main(int argc, char **argv)
struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
test_schemes, s2n_array_len(test_schemes)));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.server_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
test_schemes, s2n_array_len(test_schemes)));

/* Test: ECDSA */
Expand Down Expand Up @@ -301,7 +301,7 @@ int main(int argc, char **argv)
struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
test_schemes, s2n_array_len(test_schemes)));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
test_schemes, s2n_array_len(test_schemes)));

/* Test: ECDSA */
Expand Down Expand Up @@ -348,7 +348,7 @@ int main(int argc, char **argv)

struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context, &expected, 1));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.server_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
&expected, 1));

EXPECT_OK(s2n_signature_algorithm_select(conn));
Expand All @@ -367,7 +367,7 @@ int main(int argc, char **argv)

struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context, &expected, 1));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.server_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
&expected, 1));

EXPECT_OK(s2n_signature_algorithm_select(conn));
Expand Down Expand Up @@ -398,7 +398,7 @@ int main(int argc, char **argv)
{
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
order, s2n_array_len(order)));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
reversed_order, s2n_array_len(reversed_order)));

EXPECT_OK(s2n_signature_algorithm_select(conn));
Expand All @@ -409,7 +409,7 @@ int main(int argc, char **argv)
{
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
reversed_order, s2n_array_len(reversed_order)));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
order, s2n_array_len(order)));

EXPECT_OK(s2n_signature_algorithm_select(conn));
Expand All @@ -422,7 +422,7 @@ int main(int argc, char **argv)
{
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
order, s2n_array_len(order)));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
order, s2n_array_len(order)));

EXPECT_OK(s2n_signature_algorithm_select(conn));
Expand All @@ -444,7 +444,7 @@ int main(int argc, char **argv)
struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
&invalid, 1));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
&invalid, 1));

/* Fails for TLS1.3 */
Expand All @@ -471,7 +471,7 @@ int main(int argc, char **argv)
struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
&invalid, 1));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
&invalid, 1));

/* Fails for TLS1.2 */
Expand Down Expand Up @@ -505,7 +505,7 @@ int main(int argc, char **argv)
struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
&invalid, 1));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
&invalid, 1));

/* Fails with SHA1 */
Expand Down Expand Up @@ -536,7 +536,7 @@ int main(int argc, char **argv)
struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
&invalid, 1));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
&invalid, 1));

/* Fails for pkcs1 */
Expand All @@ -563,7 +563,7 @@ int main(int argc, char **argv)
struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
&scheme, 1));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.server_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
&scheme, 1));

/* Fails for default config with no certs */
Expand Down Expand Up @@ -594,7 +594,7 @@ int main(int argc, char **argv)
struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
&scheme, 1));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.server_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
&scheme, 1));

/* Fails for default config with no certs */
Expand Down Expand Up @@ -634,7 +634,7 @@ int main(int argc, char **argv)
/* Fails with wrong curve (256) */
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
&ecdsa256, 1));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
&ecdsa256, 1));
EXPECT_ERROR_WITH_ERRNO(
s2n_signature_algorithm_select(conn),
Expand All @@ -643,7 +643,7 @@ int main(int argc, char **argv)
/* Succeeds with right curve (384) */
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
&ecdsa384, 1));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
&ecdsa384, 1));
EXPECT_OK(s2n_signature_algorithm_select(conn));
};
Expand Down Expand Up @@ -673,7 +673,7 @@ int main(int argc, char **argv)
struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
schemes, s2n_array_len(schemes)));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.server_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
schemes, s2n_array_len(schemes)));

EXPECT_OK(s2n_signature_algorithm_select(conn));
Expand Down Expand Up @@ -702,7 +702,7 @@ int main(int argc, char **argv)
struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
local_schemes, s2n_array_len(local_schemes)));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
peer_schemes, s2n_array_len(peer_schemes)));

EXPECT_OK(s2n_signature_algorithm_select(conn));
Expand Down Expand Up @@ -852,7 +852,7 @@ int main(int argc, char **argv)
struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
local_schemes, s2n_array_len(local_schemes)));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
peer_schemes, s2n_array_len(peer_schemes)));

/* ECDSA */
Expand Down Expand Up @@ -893,7 +893,7 @@ int main(int argc, char **argv)
struct s2n_local_sig_schemes_context local_context = { 0 };
EXPECT_OK(s2n_test_set_local_sig_schemes(conn, &local_context,
local_schemes, s2n_array_len(local_schemes)));
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
peer_schemes, s2n_array_len(peer_schemes)));

EXPECT_OK(s2n_signature_algorithm_select(conn));
Expand Down Expand Up @@ -1056,6 +1056,30 @@ int main(int argc, char **argv)
};
};

/* Test: Ensure that the maximum number of permitted signature schemes can be received. */
const uint16_t max_sig_schemes = TLS_SIGNATURE_SCHEME_LIST_MAX_LEN;
for (uint16_t count = max_sig_schemes - 1; count <= max_sig_schemes + 1; count++) {
DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(conn);

DEFER_CLEANUP(struct s2n_stuffer input = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&input, 0));

uint16_t sig_scheme_list_size = count * TLS_SIGNATURE_SCHEME_LEN;
EXPECT_SUCCESS(s2n_stuffer_write_uint16(&input, sig_scheme_list_size));
for (size_t i = 0; i < count; i++) {
EXPECT_SUCCESS(s2n_stuffer_write_uint16(&input, s2n_rsa_pkcs1_sha256.iana_value));
}

int ret = s2n_recv_supported_sig_scheme_list(&input, &conn->handshake_params.peer_sig_scheme_list);
if (count <= max_sig_schemes) {
EXPECT_SUCCESS(ret);
} else {
EXPECT_FAILURE_WITH_ERRNO(ret, S2N_ERR_TOO_MANY_SIGNATURE_SCHEMES);
}
}

/* Test: send and receive default signature preferences */
for (size_t i = S2N_TLS10; i < S2N_TLS13; i++) {
DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
Expand Down Expand Up @@ -1162,7 +1186,7 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));

const struct s2n_signature_scheme *schemes[] = { &s2n_rsa_pss_rsae_sha256 };
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.server_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
schemes, s2n_array_len(schemes)));

if (s2n_is_rsa_pss_signing_supported()) {
Expand All @@ -1187,7 +1211,7 @@ int main(int argc, char **argv)

/* Invalid (PKCS1 not allowed by TLS1.3) */
const struct s2n_signature_scheme *peer_schemes[] = { &s2n_rsa_pkcs1_sha224 };
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.client_sig_hash_algs,
EXPECT_OK(s2n_test_set_peer_sig_schemes(&conn->handshake_params.peer_sig_scheme_list,
peer_schemes, s2n_array_len(peer_schemes)));

/* Both PKCS1 and PSS supported */
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/s2n_tls13_cert_request_extensions_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@ int main(int argc, char **argv)
EXPECT_NOT_NULL(conn = s2n_connection_new(S2N_CLIENT));
conn->actual_protocol_version = S2N_TLS13;

EXPECT_EQUAL(conn->handshake_params.server_sig_hash_algs.len, 0);
EXPECT_EQUAL(conn->handshake_params.peer_sig_scheme_list.len, 0);
EXPECT_SUCCESS(s2n_tls13_cert_req_send(conn));
EXPECT_SUCCESS(s2n_tls13_cert_req_recv(conn));
EXPECT_NOT_EQUAL(conn->handshake_params.server_sig_hash_algs.len, 0);
EXPECT_NOT_EQUAL(conn->handshake_params.peer_sig_scheme_list.len, 0);

EXPECT_SUCCESS(s2n_connection_free(conn));
}
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/s2n_tls13_cert_request_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ int main(int argc, char **argv)
EXPECT_TRUE(s2n_stuffer_data_available(&client_conn->handshake.io) > 0);
EXPECT_SUCCESS(s2n_tls13_cert_req_recv(client_conn));

EXPECT_TRUE(client_conn->handshake_params.server_sig_hash_algs.len > 0);
EXPECT_TRUE(client_conn->handshake_params.peer_sig_scheme_list.len > 0);

EXPECT_SUCCESS(s2n_connection_free(client_conn));
EXPECT_SUCCESS(s2n_connection_free(server_conn));
Expand Down
2 changes: 1 addition & 1 deletion tls/extensions/s2n_client_signature_algorithms.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,5 @@ static int s2n_client_signature_algorithms_send(struct s2n_connection *conn, str

static int s2n_client_signature_algorithms_recv(struct s2n_connection *conn, struct s2n_stuffer *extension)
{
return s2n_recv_supported_sig_scheme_list(extension, &conn->handshake_params.client_sig_hash_algs);
return s2n_recv_supported_sig_scheme_list(extension, &conn->handshake_params.peer_sig_scheme_list);
}
2 changes: 1 addition & 1 deletion tls/extensions/s2n_server_signature_algorithms.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,5 +44,5 @@ static int s2n_signature_algorithms_send(struct s2n_connection *conn, struct s2n

static int s2n_signature_algorithms_recv(struct s2n_connection *conn, struct s2n_stuffer *extension)
{
return s2n_recv_supported_sig_scheme_list(extension, &conn->handshake_params.server_sig_hash_algs);
return s2n_recv_supported_sig_scheme_list(extension, &conn->handshake_params.peer_sig_scheme_list);
}
7 changes: 2 additions & 5 deletions tls/s2n_handshake.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,10 @@ struct s2n_handshake_parameters {
struct s2n_blob client_cert_chain;
s2n_pkey_type client_cert_pkey_type;

/* Signature/hash algorithm pairs offered by the client in the signature_algorithms extension */
struct s2n_sig_scheme_list client_sig_hash_algs;
/* Signature/hash algorithm pairs offered by the peer. */
struct s2n_sig_scheme_list peer_sig_scheme_list;
goatgoose marked this conversation as resolved.
Show resolved Hide resolved
/* Signature scheme chosen by the server */
const struct s2n_signature_scheme *server_cert_sig_scheme;

/* Signature/hash algorithm pairs offered by the server in the certificate request */
struct s2n_sig_scheme_list server_sig_hash_algs;
/* Signature scheme chosen by the client */
const struct s2n_signature_scheme *client_cert_sig_scheme;

Expand Down
2 changes: 1 addition & 1 deletion tls/s2n_server_cert_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ int s2n_cert_req_recv(struct s2n_connection *conn)
POSIX_GUARD(s2n_recv_client_cert_preferences(in, &cert_type));

if (conn->actual_protocol_version == S2N_TLS12) {
POSIX_GUARD(s2n_recv_supported_sig_scheme_list(in, &conn->handshake_params.server_sig_hash_algs));
POSIX_GUARD(s2n_recv_supported_sig_scheme_list(in, &conn->handshake_params.peer_sig_scheme_list));
}

uint16_t cert_authorities_len = 0;
Expand Down
8 changes: 1 addition & 7 deletions tls/s2n_signature_algorithms.c
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,7 @@ static S2N_RESULT s2n_signature_algorithms_validate_supported_by_peer(
{
RESULT_ENSURE_REF(conn);

const struct s2n_sig_scheme_list *peer_list = NULL;
if (conn->mode == S2N_CLIENT) {
peer_list = &conn->handshake_params.server_sig_hash_algs;
} else {
peer_list = &conn->handshake_params.client_sig_hash_algs;
}

const struct s2n_sig_scheme_list *peer_list = &conn->handshake_params.peer_sig_scheme_list;
for (size_t i = 0; i < peer_list->len; i++) {
if (peer_list->iana_list[i] == iana) {
return S2N_RESULT_OK;
Expand Down
2 changes: 1 addition & 1 deletion tls/s2n_tls_parameters.h
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,7 @@
#define TLS_SIGNATURE_SCHEME_RSA_PSS_PSS_SHA512 0x080B

#define TLS_SIGNATURE_SCHEME_LEN 2
#define TLS_SIGNATURE_SCHEME_LIST_MAX_LEN 64
#define TLS_SIGNATURE_SCHEME_LIST_MAX_LEN 128

/* The TLS record types we support */
#define SSLv2_CLIENT_HELLO 1
Expand Down
Loading