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.

Sign up for GitHub

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

Allow TLS PSK without server certificate #2083

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Allow TLS PSK without server certificate #2083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 7 additions & 5 deletions ssl/handshake_server.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,11 +793,13 @@ static enum ssl_hs_wait_t do_select_certificate(SSL_HANDSHAKE *hs) {
}
}

// Load |hs->local_pubkey| from the cert prematurely. The certificate could be
// subject to change once we negotiate signature algorithms later. If it
// changes to another leaf certificate the server and client has support for,
// we reload it.
if (!ssl_handshake_load_local_pubkey(hs)) {
// Load |hs->local_pubkey| from the cert (if present) prematurely. The
// certificate could be subject to change once we negotiate signature
// algorithms later. If it changes to another leaf certificate the server and
// client has support for, we reload it. The public key may only be absent if
// PSK is enabled on the server, as indicated by presense of a callback.
if (!ssl_handshake_load_local_pubkey(hs) &&
!(hs->local_pubkey == nullptr && hs->config->psk_server_callback)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the check for hs->local_pubkey == nullptr not redundant 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.

Sort of. You're right that the call to ssl_handshake_load_local_pubkey will check hs->local_pubkey's nullity, but that's an implementation detail. I check it again here 1.) for clarity 2.) to preserve this logic if ssl_handshake_load_local_pubkey's implementation is changed in the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

... if ssl_handshake_load_local_pubkey's implementation is changed in the future.

NP: Even if the implementation changed, I don't think it matters whether hs->local_pubkey is null or not. Looking at the condition in an equivalent form makes it more clear to me:

  if (!ssl_handshake_load_local_pubkey(hs) &&
      (hs->local_pubkey != nullptr ||
      hs->config->psk_server_callback == nullptr)) {
```

return ssl_hs_error;
}

Expand Down
1 change: 1 addition & 0 deletions ssl/ssl_privkey.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ static bool ssl_public_key_rsa_pss_check(EVP_PKEY *pubkey, uint16_t sigalg) {

static bool tls12_pkey_supports_cipher_auth(SSL_HANDSHAKE *hs,
const EVP_PKEY *key) {
GUARD_PTR(key);
SSL *const ssl = hs->ssl;
// We may have a private key that supports the signature algorithm, but we
// need to verify that the negotiated cipher allows it. This behavior is only
Expand Down
17 changes: 0 additions & 17 deletions tests/ci/integration/python_patch/main/aws-lc-cpython.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,6 @@ index 0e50d09..f4b7b3c 100644
def test_dh_params(self):
# Check we can get a connection with ephemeral Diffie-Hellman
client_context, server_context, hostname = testing_context()
@@ -4364,14 +4366,14 @@ def test_session_handling(self):
def test_psk(self):
psk = bytes.fromhex('deadbeef')

- client_context = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT)
+ client_context, server_context, _ = testing_context()
+
client_context.check_hostname = False
client_context.verify_mode = ssl.CERT_NONE
client_context.maximum_version = ssl.TLSVersion.TLSv1_2
client_context.set_ciphers('PSK')
client_context.set_psk_client_callback(lambda hint: (None, psk))

- server_context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER)
server_context.maximum_version = ssl.TLSVersion.TLSv1_2
server_context.set_ciphers('PSK')
server_context.set_psk_server_callback(lambda identity: psk)
@@ -4443,14 +4445,14 @@ def server_callback(identity):
self.assertEqual(identity, client_identity)
return psk
Expand Down
Loading