-
Notifications
You must be signed in to change notification settings - Fork 0
Update README with Docker instructions #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -29,8 +29,57 @@ public GatewayServer(OpenClawConfig config, RpcRouter router) { | |||||||||||||||||
|
|
||||||||||||||||||
| @Override | ||||||||||||||||||
| public void onOpen(WebSocket conn, ClientHandshake handshake) { | ||||||||||||||||||
| logger.info("New connection from {}", conn.getRemoteSocketAddress()); | ||||||||||||||||||
| // Simple auth check could be added here | ||||||||||||||||||
| String remoteAddress = conn.getRemoteSocketAddress().toString(); | ||||||||||||||||||
| logger.info("New connection from {}", remoteAddress); | ||||||||||||||||||
|
|
||||||||||||||||||
| String expectedToken = config.getGateway().getAuthToken(); | ||||||||||||||||||
| if (expectedToken == null || expectedToken.isEmpty()) { | ||||||||||||||||||
| logger.warn("No auth token configured! Accepting connection from {}", remoteAddress); | ||||||||||||||||||
| return; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| String providedToken = extractToken(handshake); | ||||||||||||||||||
| 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; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| logger.info("Authenticated connection from {}", remoteAddress); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| private String extractToken(ClientHandshake handshake) { | ||||||||||||||||||
| // 1. Check Authorization header | ||||||||||||||||||
| String authHeader = handshake.getFieldValue("Authorization"); | ||||||||||||||||||
| if (authHeader != null && authHeader.toLowerCase().startsWith("bearer ")) { | ||||||||||||||||||
| return authHeader.substring(7).trim(); | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| // 2. Check query parameter | ||||||||||||||||||
| String descriptor = handshake.getResourceDescriptor(); | ||||||||||||||||||
| if (descriptor.contains("token=")) { | ||||||||||||||||||
| int index = descriptor.indexOf("token="); | ||||||||||||||||||
| String token = descriptor.substring(index + 6); | ||||||||||||||||||
| int end = token.indexOf('&'); | ||||||||||||||||||
| if (end != -1) { | ||||||||||||||||||
| token = token.substring(0, end); | ||||||||||||||||||
| } | ||||||||||||||||||
| return token; | ||||||||||||||||||
| } | ||||||||||||||||||
| return null; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| /** Constant-time string comparison to prevent timing attacks. */ | ||||||||||||||||||
| private boolean constantTimeEquals(String a, String b) { | ||||||||||||||||||
| if (a.length() != b.length()) { | ||||||||||||||||||
| return false; | ||||||||||||||||||
|
Comment on lines
+74
to
+76
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Early return on length mismatch defeats constant-time comparison The Root Cause and ImpactThe method is documented as "Constant-time string comparison to prevent timing attacks" (line 73), but the early return at line 75 ( 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 Impact: Partial information leakage about the auth token, reducing the effectiveness of the timing-attack protection.
Suggested change
Was this helpful? React with 👍 or 👎 to provide feedback. |
||||||||||||||||||
| } | ||||||||||||||||||
| int result = 0; | ||||||||||||||||||
| for (int i = 0; i < a.length(); i++) { | ||||||||||||||||||
| result |= a.charAt(i) ^ b.charAt(i); | ||||||||||||||||||
| } | ||||||||||||||||||
| return result == 0; | ||||||||||||||||||
| } | ||||||||||||||||||
|
|
||||||||||||||||||
| @Override | ||||||||||||||||||
|
|
||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Authentication bypass: unauthenticated clients can send RPC messages before connection close completes
When an unauthorized client connects,
onOpencallsconn.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 byonMessage, 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,onMessagecan still fire. SinceonMessageatGatewayServer.java:91-115processes any incoming RPC request without verifying the connection is authenticated, an attacker can:agent.send) before the close handshake completesThe
clientsmap (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
Was this helpful? React with 👍 or 👎 to provide feedback.