Skip to content

Commit 620418d

Browse files
committed
Protect against CSWSH: (Cross-Site WebSocket Hijacking)
1 parent 704e3af commit 620418d

File tree

4 files changed

+91
-12
lines changed

4 files changed

+91
-12
lines changed

src/gribbit/auth/Cookie.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,7 +167,9 @@ public io.netty.handler.codec.http.Cookie toNettyCookie() {
167167
nettyCookie.setDiscard(discardAtEndOfBrowserSession);
168168
nettyCookie.setHttpOnly(true); // TODO
169169
if (GribbitProperties.SSL) {
170-
nettyCookie.setSecure(true); // TODO
170+
// If SSL is enabled, force cookies to only be delivered over SSL, to prevent cookie hijacking
171+
// on public wifi networks
172+
nettyCookie.setSecure(true);
171173
}
172174
return nettyCookie;
173175
}

src/gribbit/auth/User.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,14 +398,19 @@ public void logIn(Response response) throws UnauthorizedException {
398398

399399
// Create new session token
400400
sessionTok = new Token(TokenType.SESSION, Cookie.SESSION_COOKIE_MAX_AGE_SECONDS);
401+
402+
// Create new random CSRF token every time user logs in
401403
csrfTok = CSRF.generateRandomCSRFToken();
402-
save();
404+
403405
if (sessionTokHasExpired()) {
404406
// Shouldn't happen, since we just created session tok, but just in case
405407
clearSessionTok();
406408
throw new UnauthorizedException("Couldn't create auth session");
407409
}
408410

411+
// Save tokens in database
412+
save();
413+
409414
// Save login cookies in result
410415
response.setCookie(new Cookie(Cookie.SESSION_COOKIE_NAME, "/", sessionTok.token,
411416
Cookie.SESSION_COOKIE_MAX_AGE_SECONDS));

src/gribbit/request/HttpRequestHandler.java

Lines changed: 78 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,10 @@
2525
*/
2626
package gribbit.request;
2727

28-
import static io.netty.handler.codec.http.HttpHeaderNames.CACHE_CONTROL;
2928
import static io.netty.handler.codec.http.HttpHeaderNames.ACCEPT_ENCODING;
29+
import static io.netty.handler.codec.http.HttpHeaderNames.CACHE_CONTROL;
3030
import static io.netty.handler.codec.http.HttpHeaderNames.CONNECTION;
3131
import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_ENCODING;
32-
import static io.netty.handler.codec.http.HttpHeaderNames.SERVER;
3332
import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_LENGTH;
3433
import static io.netty.handler.codec.http.HttpHeaderNames.CONTENT_TYPE;
3534
import static io.netty.handler.codec.http.HttpHeaderNames.DATE;
@@ -38,9 +37,11 @@
3837
import static io.netty.handler.codec.http.HttpHeaderNames.EXPIRES;
3938
import static io.netty.handler.codec.http.HttpHeaderNames.LAST_MODIFIED;
4039
import static io.netty.handler.codec.http.HttpHeaderNames.PRAGMA;
40+
import static io.netty.handler.codec.http.HttpHeaderNames.SERVER;
4141
import static io.netty.handler.codec.http.HttpHeaderNames.SET_COOKIE;
4242
import static io.netty.handler.codec.http.HttpHeaderValues.GZIP;
4343
import static io.netty.handler.codec.http.HttpHeaderValues.KEEP_ALIVE;
44+
import gribbit.auth.CSRF;
4445
import gribbit.auth.Cookie;
4546
import gribbit.auth.User;
4647
import gribbit.response.ErrorResponse;
@@ -109,6 +110,7 @@
109110
import java.io.RandomAccessFile;
110111
import java.net.InetAddress;
111112
import java.net.InetSocketAddress;
113+
import java.net.URI;
112114
import java.nio.charset.Charset;
113115
import java.time.Instant;
114116
import java.time.ZoneId;
@@ -826,15 +828,83 @@ public void messageReceived(ChannelHandlerContext ctx, Object msg) throws Except
826828
// Complete websocket handshake if requested
827829
// ------------------------------------------------------------------------------
828830

831+
// FIXME: Make these into class annotations
832+
String websocketPath = "/websocket";
833+
boolean isAuthenticatedWebsocket = true;
834+
829835
if (response == null && authorizedRoute == null && msg instanceof HttpRequest
830-
// TODO: Read WS routes from class annotations
831-
&& reqURI.endsWith("/websocket")) {
836+
// TODO: Read WS routes from class annotations, rather than using hardcoded "/websocket"
837+
&& reqURI.endsWith(websocketPath)) {
832838
HttpRequest httpReq = (HttpRequest) msg;
833839

834-
// Record which user was authenticated (if any) when websocket upgrade request was made.
835-
// TODO: Reject WS upgrade request for websockets that require authentication.
836-
// TODO: Also provide a means for revoking WS login.
837-
wsAuthenticatedUser = User.getLoggedInUser(request);
840+
// Protect against CSWSH: (Cross-Site WebSocket Hijacking)
841+
// http://www.christian-schneider.net/CrossSiteWebSocketHijacking.html
842+
// http://tools.ietf.org/html/rfc6455#page-7
843+
CharSequence origin = request.getOrigin();
844+
URI originUri = null;
845+
if (origin != null && origin.length() > 0) {
846+
try {
847+
// Try parsing origin URI
848+
originUri = new URI(origin.toString());
849+
} catch (Exception e) {
850+
}
851+
}
852+
// If port number is set but it is the default for the URI scheme, revert the port number
853+
// back to -1 (which means unspecified), so that it matches the server port number,
854+
// which is unspecified when serving http on port 80 and https on port 443
855+
int originPort = originUri == null ? -1 //
856+
: originUri.getPort() == 80 && "http".equals(originUri.getScheme()) ? -1 //
857+
: originUri.getPort() == 443 && "https".equals(originUri.getScheme()) ? -1 //
858+
: originUri.getPort();
859+
// Scheme, host and port all must match to forbid cross-origin requests
860+
if (originUri == null //
861+
|| !GribbitServer.uri.getScheme().equals(originUri.getScheme()) //
862+
|| !GribbitServer.uri.getHost().equals(originUri.getHost()) //
863+
|| GribbitServer.uri.getPort() != originPort) { //
864+
// Reject scripted requests to open this websocket from a different domain
865+
sendHttpErrorResponse(ctx, null, new DefaultFullHttpResponse(HttpVersion.HTTP_1_1,
866+
HttpResponseStatus.FORBIDDEN));
867+
return;
868+
}
869+
// Log.info("Origin: " + origin.toString());
870+
871+
if (isAuthenticatedWebsocket) {
872+
// For authenticated websockets, check if the user is logged in
873+
User loggedInUser = User.getLoggedInUser(request);
874+
if (loggedInUser == null) {
875+
// Not logged in, so can't connect to this websocket
876+
sendHttpErrorResponse(ctx, null, new DefaultFullHttpResponse(HttpVersion.HTTP_1_1,
877+
HttpResponseStatus.FORBIDDEN));
878+
return;
879+
}
880+
881+
// To further mitigate CSWSH attacks: check for the CSRF token in the URL parameter "_csrf";
882+
// the passed token must match the user's CSRF token. This means the websocket URL has to
883+
// be dynamically generated and inserted into the webpage that opened the websocket.
884+
// TODO: generate this URL an insert into the page somehow
885+
String csrfTok = loggedInUser.csrfTok;
886+
if (csrfTok == null || csrfTok.isEmpty() || csrfTok.equals(CSRF.CSRF_TOKEN_UNKNOWN)
887+
|| csrfTok.equals(CSRF.CSRF_TOKEN_PLACEHOLDER)) {
888+
// No valid CSRF token in User object
889+
sendHttpErrorResponse(ctx, null, new DefaultFullHttpResponse(HttpVersion.HTTP_1_1,
890+
HttpResponseStatus.FORBIDDEN));
891+
return;
892+
}
893+
String csrfParam = request.getQueryParam("_csrf");
894+
if (csrfParam == null || csrfParam.isEmpty() || !csrfParam.equals(csrfTok)) {
895+
// The CSRF URL query parameter is missing, or doesn't match the user's token
896+
sendHttpErrorResponse(ctx, null, new DefaultFullHttpResponse(HttpVersion.HTTP_1_1,
897+
HttpResponseStatus.FORBIDDEN));
898+
return;
899+
}
900+
901+
// Record which user was authenticated when the websocket upgrade request was made.
902+
// TODO: Also provide a means for revoking user's session while WS is still open,
903+
// e.g. poll the user table every few seconds to see if user's session token has
904+
// changed in the database? (Although this would mean that logging in on a new
905+
// device would log you out of all other sessions...)
906+
wsAuthenticatedUser = loggedInUser;
907+
}
838908

839909
WebSocketServerHandshakerFactory wsFactory =
840910
new WebSocketServerHandshakerFactory(GribbitServer.wsUri.toString(), null, true);

src/gribbit/request/Request.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,11 @@ public class Request {
7575
private Map<String, List<String>> queryParamToVals;
7676

7777
/**
78-
* Header for CORS.
78+
* Header for CORS, and for protecting against CSWSH. See:
7979
*
80-
* See http://en.wikipedia.org/wiki/Cross-origin_resource_sharing
80+
* http://en.wikipedia.org/wiki/Cross-origin_resource_sharing
81+
*
82+
* http://www.christian-schneider.net/CrossSiteWebSocketHijacking.html
8183
**/
8284
private CharSequence origin;
8385

0 commit comments

Comments
 (0)