Skip to content

Commit

Permalink
fix: Increase received signature scheme limit (#4544)
Browse files Browse the repository at this point in the history
  • Loading branch information
@goatgoose
goatgoose authored May 7, 2024
1 parent eb168f2 commit 8aa419e
Show file tree
Hide file tree
Showing 12 changed files with 64 additions and 44 deletions.
2 changes: 1 addition & 1 deletion error/s2n_errno.c
View file
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
View file
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
2 changes: 1 addition & 1 deletion tests/unit/s2n_server_signature_algorithms_extension_test.c
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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);
}
12 changes: 7 additions & 5 deletions tls/s2n_handshake.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,15 @@ 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.
*
* In the case of server connections, this list contains the client's supported signature
* schemes offered in the ClientHello. In the case of client connections, this list contains
* the server's supported signature schemes offered in the CertificateRequest.
*/
struct s2n_sig_scheme_list peer_sig_scheme_list;
/* 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
View file
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
View file
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
View file
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

0 comments on commit 8aa419e

Please sign in to comment.