Conversation
- Require 'Authorization: Bearer <token>' header or '?token=<token>' query param - Validate against configured token using constant-time comparison - Reject unauthorized connections in onOpen - Updated GatewayE2ETest to send auth headers
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (providedToken == null || !constantTimeEquals(expectedToken, providedToken)) { | ||
| logger.warn("Unauthorized connection attempt from {}", remoteAddress); | ||
| // Close with policy violation code (1008) or normal code (1000) with reason | ||
| conn.close(1008, "Unauthorized"); | ||
| return; |
There was a problem hiding this comment.
🔴 Authentication bypass: unauthenticated clients can send RPC messages before connection close completes
When an unauthorized client connects, onOpen calls conn.close(1008, "Unauthorized") which initiates a graceful WebSocket close handshake but does not immediately terminate the connection. Between the close frame being sent and the connection actually closing, the client can still send WebSocket messages that will be processed by onMessage, which has no authentication check.
Root Cause and Impact
In the java-websocket library, conn.close() is non-blocking — it sends a close frame and waits for the peer to acknowledge. During this window, onMessage can still fire. Since onMessage at GatewayServer.java:91-115 processes any incoming RPC request without verifying the connection is authenticated, an attacker can:
- Connect without a valid token
- Immediately send an RPC message (e.g.,
agent.send) before the close handshake completes - Have the message processed and receive a response
The clients map (line 21) is declared but never used to track authenticated connections, so there's no mechanism to reject messages from unauthenticated connections.
Impact: The authentication check can be bypassed entirely, allowing unauthorized access to all RPC methods.
Prompt for agents
In src/main/java/ai/openclaw/gateway/GatewayServer.java, the authentication in onOpen (lines 31-49) closes the connection but does not prevent messages from being processed during the close handshake window. To fix this:
1. Track authenticated connections using the existing `clients` ConcurrentHashMap (line 21). In onOpen, after successful authentication, add the connection to the map (e.g., `clients.put(conn.getRemoteSocketAddress().toString(), conn)`).
2. In onClose (line 86), remove the connection from the clients map.
3. In onMessage (line 91), add an authentication check at the start: if the connection is not in the clients map, log a warning and return without processing the message. For example:
if (!clients.containsValue(conn)) {
logger.warn("Message from unauthenticated connection, ignoring");
return;
}
Alternatively, use a Set<WebSocket> for O(1) lookup instead of checking containsValue on the ConcurrentHashMap.
Was this helpful? React with 👍 or 👎 to provide feedback.
| private boolean constantTimeEquals(String a, String b) { | ||
| if (a.length() != b.length()) { | ||
| return false; |
There was a problem hiding this comment.
🟡 Early return on length mismatch defeats constant-time comparison
The constantTimeEquals method returns false immediately when the two strings have different lengths (line 75), which leaks the length of the expected auth token through timing side-channels.
Root Cause and Impact
The method is documented as "Constant-time string comparison to prevent timing attacks" (line 73), but the early return at line 75 (if (a.length() != b.length()) return false;) makes the comparison time dependent on whether the attacker guessed the correct token length. An attacker can determine the exact length of the expected token by sending tokens of varying lengths and measuring response times.
Once the token length is known, the search space for brute-forcing is significantly reduced. A proper constant-time comparison should pad or otherwise handle different-length strings without an early return, for example by using MessageDigest.isEqual() which handles this correctly.
Impact: Partial information leakage about the auth token, reducing the effectiveness of the timing-attack protection.
| private boolean constantTimeEquals(String a, String b) { | |
| if (a.length() != b.length()) { | |
| return false; | |
| private boolean constantTimeEquals(String a, String b) { | |
| return java.security.MessageDigest.isEqual( | |
| a.getBytes(java.nio.charset.StandardCharsets.UTF_8), | |
| b.getBytes(java.nio.charset.StandardCharsets.UTF_8) | |
| ); |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Uh oh!
There was an error while loading. Please reload this page.