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

feat: getter for TLS1.2 master secrets #4470

Merged
merged 7 commits into from
Mar 26, 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
32 changes: 32 additions & 0 deletions api/s2n.h
Original file line number Diff line number Diff line change
Expand Up @@ -3022,6 +3022,38 @@ S2N_API extern int s2n_connection_client_cert_used(struct s2n_connection *conn);
*/
S2N_API extern const char *s2n_connection_get_cipher(struct s2n_connection *conn);

/**
* Provides access to the TLS master secret.
*
* This is a dangerous method and should not be used unless absolutely necessary.
* Mishandling the master secret can compromise both the current connection
* and any past or future connections that use the same master secret due to
* session resumption.
*
* This method is only supported for TLS1.2, and will report an S2N_ERR_INVALID_STATE
* usage error if called for a TLS1.3 connection. TLS1.3 includes a new key schedule
* that derives independent secrets from the master secret for specific purposes,
* such as separate traffic, session ticket, and exporter secrets. Using the master
* secret directly circumvents that security feature, reducing the security of
* the protocol.
*
* If you need cryptographic material tied to the current TLS session, consider
* `s2n_connection_tls_exporter` instead. Although s2n_connection_tls_exporter
* currently only supports TLS1.3, there is also an RFC that describes exporters
* for TLS1.2: https://datatracker.ietf.org/doc/html/rfc5705 Using the master
* secret as-is or defining your own exporter is dangerous.
*
* @param conn A pointer to the connection.
* @param secret_bytes Memory to copy the master secret into. The TLS1.2 secret
* is always 48 bytes long.
* @param max_size The size of the memory available at `secret_bytes`. Must be
* at least 48 bytes.
* @returns S2N_SUCCESS on success, S2N_FAILURE otherwise. `secret_bytes`
* will be set on success.
*/
S2N_API extern int s2n_connection_get_master_secret(const struct s2n_connection *conn,
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 less scary rust bindings 🥳

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eeeeeeeeeh we still pass it a *mut pointer 🙃

I was just looking at this for our Send/Sync safety. No matter what we do with const, the bindings are still scary. You can get a *mut s2n_connection from a &Connection, because you can get a *mut s2n_connection from a &NonNull<s2n_connection>. The joys of FFI.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ya, the * mut isn't very pretty but I'm mostly just celebrating the fact that the rust connection parameter &self is way more obviously accurate for these bindings. Little wins 😄

uint8_t *secret_bytes, size_t max_size);

/**
* Provides access to the TLS-Exporter functionality.
*
Expand Down
14 changes: 14 additions & 0 deletions bindings/rust/s2n-tls/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,6 +892,20 @@ impl Connection {
}
}
}

pub fn master_secret(&self) -> Result<Vec<u8>, Error> {
// TLS1.2 master secrets are always 48 bytes
let mut secret = vec![0; 48];
unsafe {
s2n_connection_get_master_secret(
self.connection.as_ptr(),
secret.as_mut_ptr(),
secret.len(),
)
.into_result()?;
}
Ok(secret)
}
}

struct Context {
Expand Down
33 changes: 33 additions & 0 deletions bindings/rust/s2n-tls/src/testing/s2n_tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,7 @@ mod tests {
use crate::{
callbacks::{ClientHelloCallback, ConnectionFuture},
enums::ClientAuthType,
error::ErrorType,
testing::{client_hello::*, s2n_tls::*, *},
};
use alloc::sync::Arc;
Expand Down Expand Up @@ -892,4 +893,36 @@ mod tests {

Ok(())
}

#[test]
fn master_secret_success() -> Result<(), Error> {
let policy = security::Policy::from_version("test_all_tls12")?;
let config = config_builder(&policy)?.build()?;
let pair = poll_tls_pair(tls_pair(config));
let server = pair.server.0.connection;
let client = pair.client.0.connection;

let server_secret = server.master_secret()?;
let client_secret = client.master_secret()?;
assert_eq!(server_secret, client_secret);

Ok(())
}

#[test]
fn master_secret_failure() -> Result<(), Error> {
// TLS1.3 does not support getting the master secret
let config = config_builder(&security::DEFAULT_TLS13)?.build()?;
let pair = poll_tls_pair(tls_pair(config));
let server = pair.server.0.connection;
let client = pair.client.0.connection;

let server_error = server.master_secret().unwrap_err();
assert_eq!(server_error.kind(), ErrorType::UsageError);

let client_error = client.master_secret().unwrap_err();
assert_eq!(client_error.kind(), ErrorType::UsageError);

Ok(())
}
}
2 changes: 0 additions & 2 deletions tests/unit/s2n_client_extensions_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ static uint8_t sct_list[] = {
0xff, 0xff, 0xff, 0xff, 0xff
};

message_type_t s2n_conn_get_current_message_type(struct s2n_connection *conn);

/* Helper function to allow us to easily repeat the PQ extension test for many scenarios.
* If the KEM negotiation is expected to fail (because of e.g. a client/server extension
* mismatch), pass in expected_kem_id = -1. The tests should always EXPECT_SUCCESS when
Expand Down
158 changes: 158 additions & 0 deletions tests/unit/s2n_crypto_test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

#include "tls/s2n_crypto.h"

#include "s2n_test.h"
#include "testlib/s2n_testlib.h"

int main()
{
BEGIN_TEST();

DEFER_CLEANUP(struct s2n_cert_chain_and_key *ecdsa_chain_and_key = NULL,
s2n_cert_chain_and_key_ptr_free);
EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&ecdsa_chain_and_key,
S2N_DEFAULT_ECDSA_TEST_CERT_CHAIN, S2N_DEFAULT_ECDSA_TEST_PRIVATE_KEY));

/* Test s2n_connection_get_master_secret */
{
const uint8_t test_secret[] = {
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x10,
0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x19, 0x20,
0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x10,
0xF1, 0xF2, 0xF3, 0xF4, 0xF5, 0xF6, 0xF7, 0xF8, 0xF9, 0xFF,
0x88, 0x87, 0x86, 0x85, 0x84, 0x83, 0x82, 0x81
};

/* s2n_connection_get_master_secret takes a constant connection, so our
* tests can share the same connection.
*/
DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_OK(s2n_skip_handshake(conn));
EXPECT_MEMCPY_SUCCESS(conn->secrets.version.tls12.master_secret,
test_secret, sizeof(test_secret));

/* Test safety checks */
{
uint8_t output[S2N_TLS_SECRET_LEN] = { 0 };
EXPECT_FAILURE_WITH_ERRNO(
s2n_connection_get_master_secret(conn, NULL, 0),
S2N_ERR_NULL);
EXPECT_FAILURE_WITH_ERRNO(
s2n_connection_get_master_secret(NULL, output, 0),
S2N_ERR_NULL);
};

/* Test: successfully get master secret */
{
uint8_t output[S2N_TLS_SECRET_LEN] = { 0 };
EXPECT_SUCCESS(s2n_connection_get_master_secret(conn, output, sizeof(output)));
EXPECT_BYTEARRAY_EQUAL(test_secret, output, sizeof(output));
};

/* Test: TLS1.3 not supported */
{
uint8_t output[S2N_TLS_SECRET_LEN] = { 0 };

conn->actual_protocol_version = S2N_TLS13;
EXPECT_FAILURE_WITH_ERRNO(
s2n_connection_get_master_secret(conn, output, sizeof(output)),
S2N_ERR_INVALID_STATE);

conn->actual_protocol_version = S2N_TLS12;
EXPECT_SUCCESS(s2n_connection_get_master_secret(conn, output, sizeof(output)));
EXPECT_BYTEARRAY_EQUAL(test_secret, output, sizeof(output));
};

/* Test: at least S2N_TLS_SECRET_LEN of output required */
{
uint8_t output[S2N_TLS_SECRET_LEN] = { 0 };

/* Fail if insufficient memory */
EXPECT_FAILURE_WITH_ERRNO(
s2n_connection_get_master_secret(conn, output, 0),
S2N_ERR_INSUFFICIENT_MEM_SIZE);
EXPECT_FAILURE_WITH_ERRNO(
s2n_connection_get_master_secret(conn, output, 1),
S2N_ERR_INSUFFICIENT_MEM_SIZE);
EXPECT_FAILURE_WITH_ERRNO(
s2n_connection_get_master_secret(conn, output, S2N_TLS_SECRET_LEN - 1),
S2N_ERR_INSUFFICIENT_MEM_SIZE);

/* Succeed if exactly S2N_TLS_SECRET_LEN bytes */
EXPECT_SUCCESS(s2n_connection_get_master_secret(conn, output, S2N_TLS_SECRET_LEN));
EXPECT_BYTEARRAY_EQUAL(test_secret, output, sizeof(output));

/* Succeed if more than S2N_TLS_SECRET_LEN bytes */
EXPECT_SUCCESS(s2n_connection_get_master_secret(conn, output, S2N_TLS_SECRET_LEN + 1));
EXPECT_BYTEARRAY_EQUAL(test_secret, output, sizeof(output));
};

/* Test: handshake must be complete */
{
uint8_t output[S2N_TLS_SECRET_LEN] = { 0 };

conn->handshake.message_number = 0;
EXPECT_FAILURE_WITH_ERRNO(
s2n_connection_get_master_secret(conn, output, sizeof(output)),
S2N_ERR_HANDSHAKE_NOT_COMPLETE);

EXPECT_OK(s2n_skip_handshake(conn));
EXPECT_SUCCESS(s2n_connection_get_master_secret(conn, output, sizeof(output)));
EXPECT_BYTEARRAY_EQUAL(test_secret, output, sizeof(output));
};

/* Test: self-talk */
{
DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(config);
EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, ecdsa_chain_and_key));
EXPECT_SUCCESS(s2n_config_set_unsafe_for_testing(config));
EXPECT_SUCCESS(s2n_config_set_cipher_preferences(config, "test_all_tls12"));

DEFER_CLEANUP(struct s2n_connection *client = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(client);
EXPECT_SUCCESS(s2n_connection_set_config(client, config));

DEFER_CLEANUP(struct s2n_connection *server = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(server);
EXPECT_SUCCESS(s2n_connection_set_config(server, config));

struct s2n_test_io_pair io_pair = { 0 };
EXPECT_SUCCESS(s2n_io_pair_init_non_blocking(&io_pair));
EXPECT_SUCCESS(s2n_connections_set_io_pair(client, server, &io_pair));
EXPECT_SUCCESS(s2n_negotiate_test_server_and_client(server, client));

uint8_t server_output[S2N_TLS_SECRET_LEN] = { 0 };
EXPECT_SUCCESS(s2n_connection_get_master_secret(server,
server_output, sizeof(server_output)));
EXPECT_BYTEARRAY_EQUAL(server->secrets.version.tls12.master_secret,
server_output, sizeof(server_output));
EXPECT_BYTEARRAY_EQUAL(client->secrets.version.tls12.master_secret,
server_output, sizeof(server_output));

uint8_t client_output[S2N_TLS_SECRET_LEN] = { 0 };
EXPECT_SUCCESS(s2n_connection_get_master_secret(client,
client_output, sizeof(client_output)));
EXPECT_BYTEARRAY_EQUAL(server_output, client_output, sizeof(client_output));
};
};

END_TEST();
}
2 changes: 0 additions & 2 deletions tests/unit/s2n_fragmentation_coalescing_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,6 @@ uint8_t fatal_alert[] = { /* Fatal: unexpected message */
0x02, 0x0a
};

message_type_t s2n_conn_get_current_message_type(struct s2n_connection *conn);

void fragmented_message(int write_fd)
{
int written = 0;
Expand Down
2 changes: 0 additions & 2 deletions tests/unit/s2n_malformed_handshake_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,6 @@ static uint8_t certificate_too_large[] = {
0x00, 0x00, 0x10
};

message_type_t s2n_conn_get_current_message_type(struct s2n_connection *conn);

void send_messages(int write_fd, uint8_t *server_hello, uint32_t server_hello_len, uint8_t *server_cert, uint32_t server_cert_len)
{
uint8_t record_header[5] = { TLS_HANDSHAKE, 0x03, 0x03, (server_hello_len >> 8), server_hello_len & 0xff };
Expand Down
16 changes: 16 additions & 0 deletions tls/s2n_crypto.c
Original file line number Diff line number Diff line change
Expand Up @@ -119,3 +119,19 @@ S2N_RESULT s2n_crypto_parameters_switch(struct s2n_connection *conn)

return S2N_RESULT_OK;
}

int s2n_connection_get_master_secret(const struct s2n_connection *conn,
uint8_t *secret_bytes, size_t max_size)
{
POSIX_ENSURE_REF(conn);
POSIX_ENSURE_REF(secret_bytes);
POSIX_ENSURE(max_size >= S2N_TLS_SECRET_LEN, S2N_ERR_INSUFFICIENT_MEM_SIZE);
POSIX_ENSURE(conn->actual_protocol_version < S2N_TLS13, S2N_ERR_INVALID_STATE);
lrstewart marked this conversation as resolved.
Show resolved Hide resolved
/* Technically the master secret is available earlier, but after the handshake
* is the simplest rule and matches our TLS1.3 exporter behavior. */
POSIX_ENSURE(is_handshake_complete(conn), S2N_ERR_HANDSHAKE_NOT_COMPLETE);
/* Final sanity check: TLS1.2 doesn't use the extract_secret_type field */
POSIX_ENSURE_EQ(conn->secrets.extract_secret_type, S2N_NONE_SECRET);
POSIX_CHECKED_MEMCPY(secret_bytes, conn->secrets.version.tls12.master_secret, S2N_TLS_SECRET_LEN);
return S2N_SUCCESS;
}
2 changes: 1 addition & 1 deletion tls/s2n_handshake.h
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ struct s2n_handshake {
};

/* Only used in our test cases. */
message_type_t s2n_conn_get_current_message_type(struct s2n_connection *conn);
message_type_t s2n_conn_get_current_message_type(const struct s2n_connection *conn);

/* s2n_handshake */
int s2n_handshake_require_all_hashes(struct s2n_handshake *handshake);
Expand Down
2 changes: 1 addition & 1 deletion tls/s2n_handshake_io.c
Original file line number Diff line number Diff line change
Expand Up @@ -841,7 +841,7 @@ static const char *tls13_handshake_type_names[] = {
#define CONNECTION_IS_WRITER(conn) (ACTIVE_STATE(conn).writer == CONNECTION_WRITER(conn))

/* Only used in our test cases. */
message_type_t s2n_conn_get_current_message_type(struct s2n_connection *conn)
message_type_t s2n_conn_get_current_message_type(const struct s2n_connection *conn)
{
return ACTIVE_MESSAGE(conn);
}
Expand Down
Loading