Skip to content

Commit

Permalink
Actually add support for SSL_get_server/peer_tmp_key (aws#1945)
Browse files Browse the repository at this point in the history
Ruby has a dependency on `SSL_get_server_tmp_key` which we've
exposed as a no-op in the past. Ruby has a couple of tests that expect
the function to actually return a value however, which means we'll have
to actually support this. The relevant information about the server key
was saved in a `tmp` structure in `SSL3_STATE` (originally
`ssl3_state_st` in OpenSSL). This `tmp` structure has been removed by
upstream, with its contents moved either to `SSL_HANDSHAKE` or the
state machine. `peer_key` seems to contain the relevant information we
want and it's been moved to `SSL_HANDSHAKE` in this case. The issue
is any information in `SSL_HANDSHAKE` is shed immediately after the
connection has been established and the contents of `peer_key` is shed
along with it. We may have to revert parts of a4c8ff0 to move the field
back into SSL3_STATE so we can access the field.
Laster versions of OpenSSL have changed this to an alias to
`SSL_get_peer_tmp_key` which means you can retrieve the client’s key if
you’re the server and vice versa. This causes
`SSL_get_server_tmp_key` to have very confusing behavior where you
actually retrieve the client's key when you're a server and Ruby already 
has tests somewhat dependent on the `SSL_get_peer_tmp_key` behavior.

### Testing:
Test that tries retrieving a X25519 or EC_KEY based on the connection
for each TLS version.

By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache 2.0 license and the ISC license.
  • Loading branch information
samuel40791765 authored Nov 5, 2024
1 parent 25709ca commit 8f1aae9
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 22 deletions.
16 changes: 11 additions & 5 deletions include/openssl/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2703,6 +2703,17 @@ OPENSSL_EXPORT uint16_t SSL_get_group_id(const SSL *ssl);
// the given TLS group ID, or NULL if the group is unknown.
OPENSSL_EXPORT const char *SSL_get_group_name(uint16_t group_id);

// SSL_get_peer_tmp_key sets |*out_key| to the temporary key provided by the
// peer that was during the key exchange. If |ssl| is the server, the client's
// temporary key is returned; if |ssl| is the client, the server's temporary key
// is returned. It returns 1 on success and 0 if otherwise.
OPENSSL_EXPORT int SSL_get_peer_tmp_key(SSL *ssl, EVP_PKEY **out_key);

// SSL_get_server_tmp_key is a backwards compatible alias to
// |SSL_get_peer_tmp_key| in OpenSSL. Note that this means the client's
// temporary key is being set to |*out_key| instead, if |ssl| is the server.
OPENSSL_EXPORT int SSL_get_server_tmp_key(SSL *ssl, EVP_PKEY **out_key);

// *** EXPERIMENTAL — DO NOT USE WITHOUT CHECKING ***
//
// |SSL_to_bytes| and |SSL_from_bytes| are developed to support SSL transfer
Expand Down Expand Up @@ -5828,11 +5839,6 @@ DEFINE_STACK_OF(SSL_COMP)
// AWS-LC does not support the use of FFDH cipher suites in libssl. The
// following functions are only provided as no-ops for easier compatibility.

// SSL_get_server_tmp_key returns zero. This was deprecated as part of the
// removal of |EVP_PKEY_DH|.
OPENSSL_EXPORT OPENSSL_DEPRECATED int SSL_get_server_tmp_key(
SSL *ssl, EVP_PKEY **out_key);

// SSL_CTX_set_tmp_dh returns 1.
//
// TODO (CryptoAlg-2398): Add |OPENSSL_DEPRECATED|. nginx defines -Werror and
Expand Down
5 changes: 4 additions & 1 deletion ssl/extensions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2312,6 +2312,7 @@ static bool ext_key_share_add_clienthello(const SSL_HANDSHAKE *hs, CBB *out,

bool ssl_ext_key_share_parse_serverhello(SSL_HANDSHAKE *hs,
Array<uint8_t> *out_secret,
Array<uint8_t> *out_peer_key,
uint8_t *out_alert, CBS *contents) {
CBS peer_key;
uint16_t group_id;
Expand All @@ -2333,7 +2334,9 @@ bool ssl_ext_key_share_parse_serverhello(SSL_HANDSHAKE *hs,
key_share = hs->key_shares[1].get();
}

if (!key_share->Finish(out_secret, out_alert, peer_key)) {
if (!key_share->Finish(out_secret, out_alert, peer_key) ||
// Save peer's public key for observation with |SSL_get_peer_tmp_key|.
!out_peer_key->CopyFrom(peer_key)) {
*out_alert = SSL_AD_INTERNAL_ERROR;
return false;
}
Expand Down
9 changes: 5 additions & 4 deletions ssl/handshake_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1163,7 +1163,7 @@ static enum ssl_hs_wait_t do_read_server_key_exchange(SSL_HANDSHAKE *hs) {

// Save the group and peer public key for later.
hs->new_session->group_id = group_id;
if (!hs->peer_key.CopyFrom(point)) {
if (!ssl->s3->peer_key.CopyFrom(point)) {
return ssl_hs_error;
}
} else if (!(alg_k & SSL_kPSK)) {
Expand Down Expand Up @@ -1508,16 +1508,17 @@ static enum ssl_hs_wait_t do_send_client_key_exchange(SSL_HANDSHAKE *hs) {
bssl::UniquePtr<SSLKeyShare> key_share =
SSLKeyShare::Create(hs->new_session->group_id);
uint8_t alert = SSL_AD_DECODE_ERROR;
if (!key_share || !key_share->Accept(&child, &pms, &alert, hs->peer_key)) {
if (!key_share ||
!key_share->Accept(&child, &pms, &alert, ssl->s3->peer_key)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
}
if (!CBB_flush(&body)) {
return ssl_hs_error;
}

// The peer key can now be discarded.
hs->peer_key.Reset();
// The peer key could be discarded, but we preserve it since OpenSSL
// allows the user to observe it with |SSL_get_peer_tmp_key|.
} else if (alg_k & SSL_kPSK) {
// For plain PSK, other_secret is a block of 0s with the same length as
// the pre-shared key.
Expand Down
4 changes: 3 additions & 1 deletion ssl/handshake_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1532,7 +1532,9 @@ static enum ssl_hs_wait_t do_read_client_key_exchange(SSL_HANDSHAKE *hs) {

// Compute the premaster.
uint8_t alert = SSL_AD_DECODE_ERROR;
if (!hs->key_shares[0]->Finish(&premaster_secret, &alert, peer_key)) {
if (!hs->key_shares[0]->Finish(&premaster_secret, &alert, peer_key) ||
// Save peer's public key for observation with |SSL_get_peer_tmp_key|.
!ssl->s3->peer_key.CopyFrom(peer_key)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
}
Expand Down
14 changes: 11 additions & 3 deletions ssl/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -1275,6 +1275,11 @@ Span<const uint16_t> PQGroups();
// false.
bool ssl_nid_to_group_id(uint16_t *out_group_id, int nid);

// ssl_nid_to_group_id looks up the group corresponding to |group_id|. On
// success, it sets |*out_nid| to the group's nid and returns true. Otherwise,
// it returns false.
bool ssl_group_id_to_nid(uint16_t *out_nid, int group_id);

// ssl_name_to_group_id looks up the group corresponding to the |name| string of
// length |len|. On success, it sets |*out_group_id| to the group ID and returns
// true. Otherwise, it returns false.
Expand Down Expand Up @@ -2061,9 +2066,6 @@ struct SSL_HANDSHAKE {
// supports with delegated credentials.
Array<uint16_t> peer_delegated_credential_sigalgs;

// peer_key is the peer's ECDH key for a TLS 1.2 client.
Array<uint8_t> peer_key;

// extension_permutation is the permutation to apply to ClientHello
// extensions. It maps indices into the |kExtensions| table into other
// indices.
Expand Down Expand Up @@ -2336,6 +2338,7 @@ bool ssl_setup_key_shares(SSL_HANDSHAKE *hs, uint16_t override_group_id);

bool ssl_ext_key_share_parse_serverhello(SSL_HANDSHAKE *hs,
Array<uint8_t> *out_secret,
Array<uint8_t> *out_peer_key,
uint8_t *out_alert, CBS *contents);
bool ssl_ext_key_share_parse_clienthello(SSL_HANDSHAKE *hs, bool *out_found,
Span<const uint8_t> *out_peer_key,
Expand Down Expand Up @@ -3033,6 +3036,11 @@ struct SSL3_STATE {
// one.
UniquePtr<SSL_HANDSHAKE> hs;

// peer_key is the peer's ECDH key for both TLS 1.2/1.3. This is only used
// for observing with |SSL_get_peer_tmp_key| and is not serialized as part of
// the SSL Transfer feature.
Array<uint8_t> peer_key;

uint8_t write_traffic_secret[SSL_MAX_MD_SIZE] = {0};
uint8_t read_traffic_secret[SSL_MAX_MD_SIZE] = {0};
uint8_t exporter_secret[SSL_MAX_MD_SIZE] = {0};
Expand Down
11 changes: 11 additions & 0 deletions ssl/ssl_key_share.cc
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,17 @@ bool ssl_nid_to_group_id(uint16_t *out_group_id, int nid) {
return false;
}

bool ssl_group_id_to_nid(uint16_t *out_nid, int group_id) {
GUARD_PTR(out_nid);
for (const auto &group : kNamedGroups) {
if (group.group_id == group_id) {
*out_nid = group.nid;
return true;
}
}
return false;
}

bool ssl_name_to_group_id(uint16_t *out_group_id, const char *name, size_t len) {
for (const auto &group : kNamedGroups) {
if (len == strlen(group.name) &&
Expand Down
46 changes: 45 additions & 1 deletion ssl/ssl_lib.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2668,7 +2668,51 @@ const COMP_METHOD *SSL_get_current_compression(SSL *ssl) { return NULL; }

const COMP_METHOD *SSL_get_current_expansion(SSL *ssl) { return NULL; }

int SSL_get_server_tmp_key(SSL *ssl, EVP_PKEY **out_key) { return 0; }
int SSL_get_peer_tmp_key(SSL *ssl, EVP_PKEY **out_key) {
GUARD_PTR(ssl);
GUARD_PTR(ssl->s3);
GUARD_PTR(out_key);

SSL_SESSION *session = SSL_get_session(ssl);
uint16_t nid;
if (!session || !ssl_group_id_to_nid(&nid, session->group_id)) {
return 0;
}
bssl::UniquePtr<EVP_PKEY> ret(EVP_PKEY_new());
if (!ret) {
return 0;
}

// Assign key type based on the session's key exchange |nid|.
if (nid == EVP_PKEY_X25519) {
if (!EVP_PKEY_set_type(ret.get(), EVP_PKEY_X25519)) {
return 0;
}
} else {
EC_KEY *key = EC_KEY_new_by_curve_name(nid);
if (!key) {
// We only support ECDHE for temporary keys, so fail if an unrecognized
// key exchange is used.
OPENSSL_PUT_ERROR(SSL, SSL_R_UNKNOWN_KEY_EXCHANGE_TYPE);
return 0;
}
if (!EVP_PKEY_assign_EC_KEY(ret.get(), key)) {
return 0;
}
}

if (!EVP_PKEY_set1_tls_encodedpoint(ret.get(), ssl->s3->peer_key.data(),
ssl->s3->peer_key.size())) {
return 0;
}
EVP_PKEY_up_ref(ret.get());
*out_key = ret.get();
return 1;
}

int SSL_get_server_tmp_key(SSL *ssl, EVP_PKEY **out_key) {
return SSL_get_peer_tmp_key(ssl, out_key);
}

void SSL_CTX_set_quiet_shutdown(SSL_CTX *ctx, int mode) {
ctx->quiet_shutdown = (mode != 0);
Expand Down
32 changes: 32 additions & 0 deletions ssl/ssl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10165,6 +10165,38 @@ TEST_P(SSLVersionTest, TicketSessionIDsMatch) {
EXPECT_EQ(Bytes(SessionIDOf(client.get())), Bytes(SessionIDOf(server.get())));
}

TEST_P(SSLVersionTest, PeerTmpKey) {
if (getVersionParam().transfer_ssl) {
// The peer's temporary key is not within the boundary of the SSL transfer
// feature.
GTEST_SKIP();
}

// Default should be using X5519 as the key exchange.
ASSERT_TRUE(Connect());
for (SSL *ssl : {client_.get(), server_.get()}) {
SCOPED_TRACE(SSL_is_server(ssl) ? "server" : "client");
EVP_PKEY *key = nullptr;
EXPECT_TRUE(SSL_get_peer_tmp_key(ssl, &key));
EXPECT_EQ(EVP_PKEY_id(key), EVP_PKEY_X25519);
bssl::UniquePtr<EVP_PKEY> pkey(key);
}

// Check that EC Groups for the key exchange also work.
ASSERT_TRUE(SSL_CTX_set1_groups_list(server_ctx_.get(), "P-384"));
ASSERT_TRUE(Connect());
for (SSL *ssl : {client_.get(), server_.get()}) {
SCOPED_TRACE(SSL_is_server(ssl) ? "server" : "client");
EVP_PKEY *key = nullptr;
EXPECT_TRUE(SSL_get_peer_tmp_key(ssl, &key));
EXPECT_EQ(EVP_PKEY_id(key), EVP_PKEY_EC);
EC_KEY *ec_key = EVP_PKEY_get0_EC_KEY(key);
EXPECT_TRUE(ec_key);
EXPECT_EQ(EC_KEY_get0_group(ec_key), EC_group_p384());
bssl::UniquePtr<EVP_PKEY> pkey(key);
}
}

static void WriteHelloRequest(SSL *server) {
// This function assumes TLS 1.2 with ChaCha20-Poly1305.
ASSERT_EQ(SSL_version(server), TLS1_2_VERSION);
Expand Down
10 changes: 4 additions & 6 deletions ssl/tls13_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -420,8 +420,7 @@ static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) {
uint16_t version;
if (!supported_versions.present ||
!CBS_get_u16(&supported_versions.data, &version) ||
CBS_len(&supported_versions.data) != 0 ||
version != ssl->version) {
CBS_len(&supported_versions.data) != 0 || version != ssl->version) {
OPENSSL_PUT_ERROR(SSL, SSL_R_SECOND_SERVERHELLO_VERSION_MISMATCH);
ssl_send_alert(ssl, SSL3_AL_FATAL, SSL_AD_ILLEGAL_PARAMETER);
return ssl_hs_error;
Expand Down Expand Up @@ -497,15 +496,14 @@ static enum ssl_hs_wait_t do_read_server_hello(SSL_HANDSHAKE *hs) {
// Resolve ECDHE and incorporate it into the secret.
Array<uint8_t> dhe_secret;
alert = SSL_AD_DECODE_ERROR;
if (!ssl_ext_key_share_parse_serverhello(hs, &dhe_secret, &alert,
&key_share.data)) {
if (!ssl_ext_key_share_parse_serverhello(hs, &dhe_secret, &ssl->s3->peer_key,
&alert, &key_share.data)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return ssl_hs_error;
}

if (!tls13_advance_key_schedule(hs, dhe_secret) ||
!ssl_hash_message(hs, msg) ||
!tls13_derive_handshake_secrets(hs)) {
!ssl_hash_message(hs, msg) || !tls13_derive_handshake_secrets(hs)) {
return ssl_hs_error;
}

Expand Down
4 changes: 3 additions & 1 deletion ssl/tls13_server.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ static bool resolve_ecdhe_secret(SSL_HANDSHAKE *hs,
if (!key_share || //
!CBB_init(public_key.get(), 32) ||
!key_share->Accept(public_key.get(), &secret, &alert, peer_key) ||
!CBBFinishArray(public_key.get(), &hs->ecdh_public_key)) {
!CBBFinishArray(public_key.get(), &hs->ecdh_public_key) ||
// Save peer's public key for observation with |SSL_get_peer_tmp_key|.
!ssl->s3->peer_key.CopyFrom(peer_key)) {
ssl_send_alert(ssl, SSL3_AL_FATAL, alert);
return false;
}
Expand Down

0 comments on commit 8f1aae9

Please sign in to comment.