XSS protection in signin / signout.

XSRF protection for register / unregister.
Support only v3 and upwards.
Rev to version 4.
This commit is contained in:
burke.davey
2010-07-19 00:15:37 +00:00
parent 0c3c62c7d0
commit b1c50187d4
5 changed files with 73 additions and 80 deletions

View File

@@ -18,6 +18,7 @@ package com.google.android.chrometophone.server;
import java.io.IOException;
import java.net.URLEncoder;
import java.util.logging.Logger;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
@@ -28,61 +29,46 @@ import com.google.appengine.api.users.UserServiceFactory;
@SuppressWarnings("serial")
public class AuthServlet extends HttpServlet {
private static final Logger log =
Logger.getLogger(SendServlet.class.getName());
private static final String ERROR_STATUS = "ERROR";
@Override
public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
if (req.getRequestURI().startsWith("/signin")) {
doSignIn(req, resp);
} else if (req.getRequestURI().startsWith("/signout")) {
doSignOut(req, resp);
}
}
resp.setContentType("text/html");
boolean signIn = req.getRequestURI().startsWith("/signin");
private void doSignIn(HttpServletRequest req, HttpServletResponse resp) throws IOException {
// Get the extension return URL
String extRet = req.getParameter("extret");
if (extRet == null) {
resp.setContentType("text/plain");
resp.setStatus(400);
resp.getWriter().println(ERROR_STATUS + " (extret parameter missing)");
return;
}
// If login is complete, redirect to the extension page. Otherwise, send user to login,
// setting the continue page back to this servlet (since UserService does not understand
// chrome-extension:// URLs
// If login/logout is complete, redirect to the extension page. Otherwise, send user to
// login/logout, setting the continue page back to this servlet (since UserService does
// not understand chrome-extension:// URLs)
if (req.getParameter("completed") != null) {
// Server-side redirects don't work for chrome-extension:// URLs so we do a client-
// side redirect instead
resp.getWriter().println("<meta http-equiv=\"refresh\" content=\"0;url=" + extRet + "\">");
// Sanitize the extRet URL for XSS protection
String regEx = "chrome-extension://[a-z]+" +
(signIn ? "/signed_in\\.html" : "/signed_out\\.html");
if (extRet.matches(regEx)) {
resp.getWriter().println("<meta http-equiv=\"refresh\" content=\"0;url=" + extRet + "\">");
} else {
resp.setStatus(400);
resp.getWriter().println(ERROR_STATUS + " (invalid redirect)");
log.warning("Invalid redirect " + extRet);
}
} else {
String followOnURL = req.getRequestURI() + "?completed=true&extret=" +
URLEncoder.encode(extRet, "UTF-8");
UserService userService = UserServiceFactory.getUserService();
resp.sendRedirect(userService.createLoginURL(followOnURL));
}
}
private void doSignOut(HttpServletRequest req, HttpServletResponse resp) throws IOException {
// Get the extension return URL
String extRet = req.getParameter("extret");
if (extRet == null) {
resp.setContentType("text/plain");
resp.getWriter().println(ERROR_STATUS + " (extret parameter missing)");
return;
}
// If logout is complete, redirect to the extension page. Otherwise, send user to login,
// setting the continue page back to this servlet (since UserService does not understand
// chrome-extension:// URLs
if (req.getParameter("completed") != null) {
// Server-side redirects don't work for chrome-extension:// URLs so we do a client-
// side redirect instead
resp.getWriter().println("<meta http-equiv=\"refresh\" content=\"0;url=" + extRet + "\">");
} else {
String followOnURL = req.getRequestURI() + "?completed=true&extret=" +
URLEncoder.encode(extRet, "UTF-8");
UserService userService = UserServiceFactory.getUserService();
resp.sendRedirect(userService.createLogoutURL(followOnURL));
resp.sendRedirect(signIn ? userService.createLoginURL(followOnURL) :
userService.createLogoutURL(followOnURL));
}
}
}

View File

@@ -77,6 +77,7 @@ public class RegisterServlet extends HttpServlet {
/**
* @deprecated will be removed in next rel.
*/
@Deprecated
@Override
public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
doPost(req, resp);
@@ -86,6 +87,14 @@ public class RegisterServlet extends HttpServlet {
public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException {
resp.setContentType("text/plain");
// Basic XSRF protection
if (req.getHeader("X-Same-Domain") == null) {
// TODO: Enable at consumer launch
//resp.setStatus(400);
//resp.getWriter().println(ERROR_STATUS + " (Missing X-Same-Domain header)");
//return;
}
String deviceRegistrationID = req.getParameter("devregid");
if (deviceRegistrationID == null) {
resp.setStatus(400);

View File

@@ -17,7 +17,6 @@
package com.google.android.chrometophone.server;
import java.io.IOException;
import java.net.URLEncoder;
import java.util.logging.Logger;
import javax.jdo.JDOObjectNotFoundException;
@@ -30,8 +29,6 @@ import com.google.android.c2dm.server.C2DMessaging;
import com.google.appengine.api.datastore.Key;
import com.google.appengine.api.datastore.KeyFactory;
import com.google.appengine.api.users.User;
import com.google.appengine.api.users.UserService;
import com.google.appengine.api.users.UserServiceFactory;
@SuppressWarnings("serial")
public class SendServlet extends HttpServlet {
@@ -51,19 +48,25 @@ public class SendServlet extends HttpServlet {
public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
resp.setContentType("text/plain");
// Basic XSRF protection
if (req.getHeader("X-Extension") == null) {
resp.setStatus(400);
resp.getWriter().println(ERROR_STATUS + " - Please upgrade your extension");
log.warning("Missing X-Extension header");
resp.setStatus(400);
return;
}
// Check API version
String apiVersionString = req.getParameter("ver");
if (apiVersionString == null) apiVersionString = "1";
int apiVersion = Integer.parseInt(apiVersionString);
log.info("Extension version: " + apiVersion);
if (apiVersion < 3) {
resp.setStatus(400);
resp.getWriter().println(ERROR_STATUS +
" (Please remove old Chrome extension and install latest)");
return;
}
// Basic XSRF protection (TODO: remove X-Extension in a future release for consistency)
if (req.getHeader("X-Same-Domain") == null && req.getHeader("X-Extension") == null) {
resp.setStatus(400);
resp.getWriter().println(ERROR_STATUS + " (Missing header)");
log.warning("Missing header");
return;
}
String sel = req.getParameter("sel");
if (sel == null) sel = ""; // optional
@@ -78,23 +81,14 @@ public class SendServlet extends HttpServlet {
User user = RegisterServlet.checkUser(req, resp, false);
if (user != null) {
doSendToPhone(url, title, sel, user.getEmail(), apiVersion, resp);
doSendToPhone(url, title, sel, user.getEmail(), resp);
} else {
if (apiVersion >= 2) { // TODO: Make this default code path on launch
resp.getWriter().println(LOGIN_REQUIRED_STATUS);
} else { // TODO: DEPRECATED code path. Delete on launch
String followOnURL = req.getRequestURI() + "?title=" +
URLEncoder.encode(title, "UTF-8") +
"&url=" + URLEncoder.encode(url, "UTF-8") +
"&sel=" + URLEncoder.encode(sel, "UTF-8");
UserService userService = UserServiceFactory.getUserService();
resp.sendRedirect(userService.createLoginURL(followOnURL));
}
resp.getWriter().println(LOGIN_REQUIRED_STATUS);
}
}
private boolean doSendToPhone(String url, String title, String sel,
String userAccount, int apiVersion, HttpServletResponse resp) throws IOException {
String userAccount, HttpServletResponse resp) throws IOException {
// Get device info
DeviceInfo deviceInfo = null;
// Shared PMF
@@ -106,12 +100,7 @@ public class SendServlet extends HttpServlet {
deviceInfo = pm.getObjectById(DeviceInfo.class, key);
} catch (JDOObjectNotFoundException e) {
log.warning("Device not registered");
if (apiVersion >= 3) { // TODO: Make this default code path on launch
resp.getWriter().println(DEVICE_NOT_REGISTERED_STATUS);
} else { // TODO: DEPRECATED code path. Delete on launch
resp.setStatus(400);
resp.getWriter().println(ERROR_STATUS + " (Device not registered)");
}
resp.getWriter().println(DEVICE_NOT_REGISTERED_STATUS);
return false;
}
} finally {

View File

@@ -42,6 +42,7 @@ public class UnregisterServlet extends HttpServlet {
/**
* @deprecated Will be removed in next rel cycle.
*/
@Deprecated
@Override
public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
doPost(req, resp);
@@ -51,6 +52,14 @@ public class UnregisterServlet extends HttpServlet {
public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException {
resp.setContentType("text/plain");
// Basic XSRF protection
if (req.getHeader("X-Same-Domain") == null) {
// TODO: Enable at consumer launch
//resp.setStatus(400);
//resp.getWriter().println(ERROR_STATUS + " (Missing X-Same-Domain header)");
//return;
}
String deviceRegistrationID = req.getParameter("devregid");
if (deviceRegistrationID == null) {
resp.setStatus(400);

View File

@@ -16,7 +16,7 @@
-->
<appengine-web-app xmlns="http://appengine.google.com/ns/1.0">
<application>chrometophone</application>
<version>3</version>
<version>4</version>
<system-properties>
<property name="java.util.logging.config.file" value="WEB-INF/logging.properties"/>
</system-properties>