Repository navigation
Conversation
|
Tested on hardware (Pixel 9 Pro, Google Play services): with this change, successful get-assertion and make-credential ceremonies end on the phone's success screen instead of "Something went wrong". I added a test: the connection loop is closed while a getAssertion is still pending, and the only frame after the request is one encrypted Shutdown. One observation for whoever uses |
AlfioEmanueleFresta
left a comment
There was a problem hiding this comment.
Thank you so much @p-c-h-b!
This looks good to me.
I had started working on #276 which added shutdown & lingering. Your implementation of shutdown in this PR is better IMO as it waits 2s like Chrome, so I'll land this and rebase #276 on top of your changes later.
Sorry for the delay reviewing/merging your PR, as I was AFK abroad.
`CableChannel::close()` was a TODO, and dropping the channel aborts the connection task, so the authenticator never received the caBLE Shutdown control message: it saw the tunnel drop instead. Google Play services then ends even a successful hybrid ceremony on a "Something went wrong" screen. Close the session the way Chromium's `FidoTunnelDevice` does (it encrypts and sends a one-byte `kShutdown` message, then waits for the peer to close): - `close()` signals the connection task once the tunnel is connected, then waits up to 3 s for it to finish; before the tunnel is up there is nothing to shut down and it returns at once. - The connection task sends Shutdown (type byte 0, padded and encrypted through the same path as CTAP frames), then waits up to 2 s for the peer to close its side. - Inbound, a bare Shutdown (the type byte with no payload, as Chromium and phones send it) is accepted instead of failing as `InvalidFraming`. Tests cover the bare-Shutdown parse, the Shutdown frame on the wire (decrypts to exactly `[0x00]`), and CTAP frames through the shared path.
Drives the connection loop with a scripted peer: post-handshake message, a pending getAssertion, then close. The only frame after the request is an encrypted Shutdown, and the task ends cleanly once the peer closes its side.
a90f2ff to
c3614ee
Compare
Problem
CableChannel::close()is a TODO:and
Drop for CableChannelaborts the connection task. So after a hybrid (caBLE v2) ceremony the authenticator never receives the Shutdown control message; it sees the tunnel drop instead. Google Play services on a Pixel 9 Pro (Android 16) then ends successful ceremonies on a "Something went wrong" screen. The client and the relying party both succeed. Logcat at the end of a successful get-assertion:Found while integrating libwebauthn 0.10.0 as the hybrid client of a desktop app.
Change
This follows what Chromium's
FidoTunnelDevicedoes: on close it encrypts and sends a one-bytekShutdownmessage, then waits for the peer to close.close()signals the connection task through a new oneshot, once the tunnel is connected, then waits up to 3 s for the task to finish. Before the tunnel is up there is nothing to shut down, and it returns immediately. Chromium waits up to three minutes for the peer; a short bound keepsclose()from stalling a caller.0) through the same pad/frame/encrypt path as CTAP requests, which is factored out ofconnection_sendassend_tunnel_message. It then waits up to 2 s for the peer to close its side.CableTunnelMessage::from_slicerejected every empty payload, so a bare Shutdown from the peer (the type byte alone, as Chromium and phones send it) failed asInvalidFraming. Shutdown may now be empty; the other types still require a payload.Dropis unchanged (it still aborts): a caller that wants a clean ending callsclose().Tests
In
transport::cable::protocol::tests:send_tunnel_messagedecrypts, on the other side of a Noise pair, to exactly[0x00];cargo test -p libwebauthn --lib transport::cable(41 passed),cargo clippy -p libwebauthn --all-targets -- -D warningsandcargo fmt --all -- --checkare clean on this branch.On hardware, the same patch applied to 0.10.0 is in use against Google Play services; the phone is expected to end on its success screen instead of "Something went wrong".