Skip to content

tls: reject invalid ServerKeyExchange signature in TLS client - #5248

Open
musthaf-bughunter wants to merge 1 commit into
secdev:masterfrom
musthaf-bughunter:tls-client-ske-signature
Open

musthaf-bughunter wants to merge 1 commit into
secdev:masterfrom
musthaf-bughunter:tls-client-ske-signature

Conversation

@musthaf-bughunter

Copy link
Copy Markdown

Description

TLSServerKeyExchange.post_dissection only logs a failed (EC)DHE parameter signature check (handshake.py#L1105-L1109), and TLSClientAutomaton.HANDLED_SERVERKEYEXCHANGE does nothing with it (automaton_cli.py#L475-L477). With verify_server=True / cafile, a TLS <= 1.2 client checks the forwarded certificate chain and hostname, then accepts key exchange parameters signed by any key, so a MitM that only has the server certificate can complete the handshake.

Repro (srv.pem issued by ca.pem for scapy.test, evil_key.pem an unrelated RSA key):

srv = TLSServerAutomaton(mycert="srv.pem", mykey="evil_key.pem")
# in another thread: srv.run()
cli = TLSClientAutomaton(client_hello=TLSClientHello(version=0x0303, ciphers=0xc02f),
                         server_name="scapy.test", cafile="ca.pem",
                         data=[b"hello", b"quit"])
cli.run()
# INFO: TLS: invalid ServerKeyExchange signature [...]
# > TLS handshake completed!   -> data is sent

The result is now stored in the session (server_kx_sig_valid) and the client aborts with a decrypt_error alert, the same way as the TLS 1.3 CertificateVerify check (server_cert_verify_valid). Anonymous key exchanges are not affected. A test using a server key that doesn't match its certificate is added to tlsclientserver.uts.

TLSServerKeyExchange.post_dissection only logged a failed signature
check, and TLSClientAutomaton continued the handshake. Store the result
in the session and abort with a decrypt_error alert, as is already done
for the TLS 1.3 CertificateVerify.

AI-Assisted: yes (Claude Opus 5.5)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant