From 3428ff46d0ee0bfd5096c82dc1bd15af1abe96d1 Mon Sep 17 00:00:00 2001 From: Arni Dagur Date: Thu, 16 Jul 2026 17:42:26 +0100 Subject: [PATCH] fix: fail reads clearly when the peer sends a TLS 1.3 KeyUpdate The kernel cannot switch traffic keys, so after a peer rekeys every read fails to decrypt with an opaque `EBADMSG`. Detect the KeyUpdate handshake message and fail the read with an explicit error instead. NewSessionTicket is still ignored. This matches Facebook's kTLS implementation: https://github.com/facebookincubator/fizz/blob/7fb7075a65ec9e669a6851669502d549e12f9e0f/fizz/experimental/ktls/AsyncKTLSSocket.cpp#L257-L280 We should implement `KeyUpdate` in the `ktls` crate at some point. There is already ongoing work (https://github.com/rustls/ktls/pull/62). Until that is complete, it's best to return an explicit error. --- ktls/src/ktls_stream.rs | 30 ++++++++++++--- ktls/tests/integration_test.rs | 70 ++++++++++++++++++++++++++-------- 2 files changed, 79 insertions(+), 21 deletions(-) diff --git a/ktls/src/ktls_stream.rs b/ktls/src/ktls_stream.rs index da45bc2..2aac21a 100644 --- a/ktls/src/ktls_stream.rs +++ b/ktls/src/ktls_stream.rs @@ -225,12 +225,30 @@ where return task::Poll::Ready(Ok(())); } TlsGetRecordType::Handshake => { - // TODO: this is where we receive TLS 1.3 resumption tickets, - // should those be stored anywhere? I'm not even sure what - // format they have at this point - tracing::trace!( - "ignoring handshake message (probably a resumption ticket)" - ); + const MSG_NEW_SESSION_TICKET: u8 = 4; + const MSG_KEY_UPDATE: u8 = 24; + let message_type = r.iovs().next().and_then(|iov| iov.first()); + match message_type { + Some(&MSG_KEY_UPDATE) => { + // KeyUpdate is fatal: This crate cannot switch traffic + // keys, so every read after the peer rekeys would fail + // to decrypt. Recent kernels (6.14+) support rekeying, + // but it's not implemented here. + return task::Poll::Ready(Err(io::Error::new( + io::ErrorKind::Unsupported, + "peer sent a TLS 1.3 KeyUpdate, which is currently unsupported by the ktls crate", + ))); + } + Some(&MSG_NEW_SESSION_TICKET) => { + // NewSessionTicket is safe to ignore: A ticket only + // matters if you want to resume later. Dropping it + // doesn't affect the current connection. + tracing::trace!("ignoring NewSessionTicket"); + } + other => { + tracing::trace!(?other, "ignoring unexpected handshake message"); + } + } } TlsGetRecordType::ApplicationData => { unreachable!( diff --git a/ktls/tests/integration_test.rs b/ktls/tests/integration_test.rs index d39a0bf..2ccaaf7 100644 --- a/ktls/tests/integration_test.rs +++ b/ktls/tests/integration_test.rs @@ -640,6 +640,36 @@ async fn read_returns_eof_when_close_notify_reply_would_block() { jh.await.unwrap(); } +/// The kernel cannot switch traffic keys, so a peer-initiated TLS 1.3 +/// KeyUpdate must fail reads with a clear error instead of the opaque +/// decrypt failures every later read would produce. +#[tokio::test] +async fn key_update_fails_reads_with_clear_error() { + let cipher_suite = KtlsCipherSuite { + version: KtlsVersion::TLS13, + typ: KtlsCipherType::AesGcm128, + }; + + let (mut server, mut client) = ktls_server_rustls_client(cipher_suite).await; + + // 1. Sanity round trip before the rekey. + client.write_all(b"hello").await.unwrap(); + client.flush().await.unwrap(); + let mut buf = [0u8; 5]; + server.read_exact(&mut buf).await.unwrap(); + assert_eq!(&buf, b"hello"); + + // 2. The client rekeys, then writes with the new keys. + client.get_mut().1.refresh_traffic_keys().unwrap(); + client.write_all(b"rekeyed").await.unwrap(); + client.flush().await.unwrap(); + + // 3. The server's next read must report the KeyUpdate clearly. + let err = server.read(&mut buf).await.unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::Unsupported, "{err}"); + assert!(err.to_string().contains("KeyUpdate"), "{err}"); +} + #[tokio::test] async fn missing_close_notify_is_unexpected_eof() { let cipher_suite = KtlsCipherSuite { @@ -647,6 +677,30 @@ async fn missing_close_notify_is_unexpected_eof() { typ: KtlsCipherType::AesGcm128, }; + let (mut server, mut client) = ktls_server_rustls_client(cipher_suite).await; + + // 1. Sanity round trip. + client.write_all(b"hello").await.unwrap(); + client.flush().await.unwrap(); + let mut buf = [0u8; 5]; + server.read_exact(&mut buf).await.unwrap(); + assert_eq!(&buf, b"hello"); + + // 2. The client sends a bare TCP FIN, bypassing the TLS shutdown. + client.get_mut().0.shutdown().await.unwrap(); + + // 3. The server must report truncation, not end-of-stream. + let err = server.read(&mut buf).await.unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::UnexpectedEof, "{err}"); +} + +/// Handshake + kTLS config for one ktls server and one plain rustls client. +async fn ktls_server_rustls_client( + cipher_suite: KtlsCipherSuite, +) -> ( + ktls::KtlsStream, + tokio_rustls::client::TlsStream, +) { let ckey = generate_simple_self_signed(vec!["localhost".to_string()]).unwrap(); let mut server_config = @@ -685,19 +739,5 @@ async fn missing_close_notify_is_unexpected_eof() { .await .unwrap() }; - let (mut server, mut client) = tokio::join!(server, client); - - // 1. Sanity round trip. - client.write_all(b"hello").await.unwrap(); - client.flush().await.unwrap(); - let mut buf = [0u8; 5]; - server.read_exact(&mut buf).await.unwrap(); - assert_eq!(&buf, b"hello"); - - // 2. The client sends a bare TCP FIN, bypassing the TLS shutdown. - client.get_mut().0.shutdown().await.unwrap(); - - // 3. The server must report truncation, not end-of-stream. - let err = server.read(&mut buf).await.unwrap_err(); - assert_eq!(err.kind(), io::ErrorKind::UnexpectedEof, "{err}"); + tokio::join!(server, client) }