remoting: Harden security key auth handlers and signaling transport [chromium/src : main]
0 views
Skip to first unread message
Yuwei Huang (Gerrit)
Jun 19, 2026, 3:28:40 AM (10 days ago) Jun 19
to chromium...@chromium.org, chromotin...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Yuwei Huang added 2 comments![]( "Open in Gerrit")
File remoting/host/security_key/security_key_auth_handler_mojo.cc
Line 148, Patchset 3 (Latest):
if (!send_message_callback_) {Yuwei Huang . unresolved
Please fix this WARNING reported by autoreview issue finding: It's slightly more efficient and logical to move this null-check before storing the callback and starting the timer, so we avoid that work if we're just going to drop the request.
```cpp
if (!send_message_callback_) {
LOG(ERROR) << "send_message_callback_ is null, dropping request.";
CloseSecurityKeyRequestConnection(connection_id);
return;
}
// Reset the timer to give the client a chance to send the response.
connection.disconnect_timer.Start(FROM_HERE, kSecurityKeyRequestTimeout,
GetCloseConnectionClosure(connection_id));
connection.on_security_key_request_callback = std::move(callback);
```
File remoting/host/security_key/security_key_auth_handler_mojo_unittest.cc
Line 291, Patchset 3 (Latest):
auth_handler_->SetSendMessageCallback(base::NullCallback());Yuwei Huang . unresolved
Please fix this WARNING reported by autoreview issue finding: Please add `#include "base/functional/callback_helpers.h"` for `base::NullCallback()`.
Related details
Attention set is empty
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yuwei Huang (Gerrit)
Jun 19, 2026, 3:39:49 AM (10 days ago) Jun 19
to Joe Downing, chromium...@chromium.org, chromotin...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Joe Downing
Yuwei Huang voted and added 3 comments![]( "Open in Gerrit")
Votes added by Yuwei Huang
| Auto-Submit | +1 |
| Commit-Queue | +1 |
3 comments
Patchset-level comments
File remoting/host/security_key/security_key_auth_handler_mojo.cc
Please fix this WARNING reported by autoreview issue finding: It's slightly more efficient and logical to move this null-check before storing the callback and starting the timer, so we avoid that work if we're just going to drop the request.
```cpp
if (!send_message_callback_) {
LOG(ERROR) << "send_message_callback_ is null, dropping request.";
CloseSecurityKeyRequestConnection(connection_id);
return;
}// Reset the timer to give the client a chance to send the response.
connection.disconnect_timer.Start(FROM_HERE, kSecurityKeyRequestTimeout,
GetCloseConnectionClosure(connection_id));
connection.on_security_key_request_callback = std::move(callback);
```
Yuwei Huang
Done
File remoting/host/security_key/security_key_auth_handler_mojo_unittest.cc
Line 291, Patchset 3:
auth_handler_->SetSendMessageCallback(base::NullCallback());Yuwei Huang . resolved
Please fix this WARNING reported by autoreview issue finding: Please add `#include "base/functional/callback_helpers.h"` for `base::NullCallback()`.
Yuwei Huang
Done
Related details
Attention is currently required from:
- Joe Downing
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Joe Downing (Gerrit)
Jun 19, 2026, 12:21:11 PM (10 days ago) Jun 19
to Yuwei Huang, Chromium LUCI CQ, chromium...@chromium.org, chromotin...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Yuwei Huang
Joe Downing voted![]( "Open in Gerrit")
Attention is currently required from:
- Yuwei Huang
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Jun 19, 2026, 12:26:33 PM (10 days ago) Jun 19
to Yuwei Huang, Joe Downing, chromium...@chromium.org, chromotin...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
remoting: Harden security key auth handlers and signaling transport
MAGI imposes these changes on the refactoring CL, so I figure I should
do it in a parent CL instead.
This CL adds robust null-safety and input-size validation to the
security key host-side components to prevent crashes and resource
exhaustion:
1. Mojo/Posix Auth Handlers: Removes the entry-point DCHECK and adds
runtime null checks for the send_message_callback_ to prevent host
crashes if a client disconnects during an active request.
2. Legacy Signaling: Enforces a strict 512KB pre-parsing limit on raw
JSON messages and a 64KB size limit on parsed payload byte lists to
prevent DoS/OOM on the JSON reader.
3. Unit Tests: Adds dedicated Mojo and Posix null-safety test cases.
TAG=agy CONV=a74bbced-b6c1-454e-adea-b8a5ad41a1ef
Bug: 517007701
Change-Id: Ia30e81908ba50e23295b9fc221df81d55e57b025
Auto-Submit: Yuwei Huang <yuw...@chromium.org>
Commit-Queue: Joe Downing <joe...@chromium.org>
Reviewed-by: Joe Downing <joe...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1649713}
Files:
- M remoting/host/security_key/security_key_auth_handler_mojo.cc
- M remoting/host/security_key/security_key_auth_handler_mojo_unittest.cc
- M remoting/host/security_key/security_key_auth_handler_posix.cc
- M remoting/host/security_key/security_key_auth_handler_posix_unittest.cc
- M remoting/host/security_key/security_key_extension_session.cc
Change size: M
Delta: 5 files changed, 100 insertions(+), 9 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Joe Downing
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages