7095980: Ensure HttpURLConnection (and supporting APIs) don't expose HttpOnly cookies
Reviewed-by: michaelm
--- a/jdk/src/share/classes/java/net/HttpCookie.java Thu Dec 15 19:52:13 2011 -0800
+++ b/jdk/src/share/classes/java/net/HttpCookie.java Fri Dec 16 16:09:41 2011 +0000
@@ -72,6 +72,10 @@
private boolean httpOnly; // HttpOnly ... i.e. not accessible to scripts
private int version = 1; // Version=1 ... RFC 2965 style
+ // The original header this cookie was consructed from, if it was
+ // constructed by parsing a header, otherwise null.
+ private final String header;
+
// Hold the creation time (in seconds) of the http cookie for later
// expiration calculation
private final long whenCreated;
@@ -128,6 +132,10 @@
* @see #setVersion
*/
public HttpCookie(String name, String value) {
+ this(name, value, null /*header*/);
+ }
+
+ private HttpCookie(String name, String value, String header) {
name = name.trim();
if (name.length() == 0 || !isToken(name) || isReserved(name)) {
throw new IllegalArgumentException("Illegal cookie name");
@@ -140,6 +148,7 @@
whenCreated = System.currentTimeMillis();
portlist = null;
+ this.header = header;
}
/**
@@ -163,6 +172,15 @@
* if the header string is {@code null}
*/
public static List<HttpCookie> parse(String header) {
+ return parse(header, false);
+ }
+
+ // Private version of parse() that will store the original header used to
+ // create the cookie, in the cookie itself. This can be useful for filtering
+ // Set-Cookie[2] headers, using the internal parsing logic defined in this
+ // class.
+ private static List<HttpCookie> parse(String header, boolean retainHeader) {
+
int version = guessCookieVersion(header);
// if header start with set-cookie or set-cookie2, strip it off
@@ -178,7 +196,7 @@
// so the parse logic is slightly different
if (version == 0) {
// Netscape draft cookie
- HttpCookie cookie = parseInternal(header);
+ HttpCookie cookie = parseInternal(header, retainHeader);
cookie.setVersion(0);
cookies.add(cookie);
} else {
@@ -187,7 +205,7 @@
// it'll separate them with comma
List<String> cookieStrings = splitMultiCookies(header);
for (String cookieStr : cookieStrings) {
- HttpCookie cookie = parseInternal(cookieStr);
+ HttpCookie cookie = parseInternal(cookieStr, retainHeader);
cookie.setVersion(1);
cookies.add(cookie);
}
@@ -804,7 +822,8 @@
* @throws IllegalArgumentException
* if header string violates the cookie specification
*/
- private static HttpCookie parseInternal(String header)
+ private static HttpCookie parseInternal(String header,
+ boolean retainHeader)
{
HttpCookie cookie = null;
String namevaluePair = null;
@@ -819,7 +838,13 @@
if (index != -1) {
String name = namevaluePair.substring(0, index).trim();
String value = namevaluePair.substring(index + 1).trim();
- cookie = new HttpCookie(name, stripOffSurroundingQuote(value));
+ if (retainHeader)
+ cookie = new HttpCookie(name,
+ stripOffSurroundingQuote(value),
+ header);
+ else
+ cookie = new HttpCookie(name,
+ stripOffSurroundingQuote(value));
} else {
// no "=" in name-value pair; it's an error
throw new IllegalArgumentException("Invalid cookie name-value pair");
@@ -972,6 +997,28 @@
}
}
+ static {
+ sun.misc.SharedSecrets.setJavaNetHttpCookieAccess(
+ new sun.misc.JavaNetHttpCookieAccess() {
+ public List<HttpCookie> parse(String header) {
+ return HttpCookie.parse(header, true);
+ }
+
+ public String header(HttpCookie cookie) {
+ return cookie.header;
+ }
+ }
+ );
+ }
+
+ /*
+ * Returns the original header this cookie was consructed from, if it was
+ * constructed by parsing a header, otherwise null.
+ */
+ private String header() {
+ return header;
+ }
+
/*
* Constructs a string representation of this cookie. The string format is
* as Netscape spec, but without leading "Cookie:" token.
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/src/share/classes/sun/misc/JavaNetHttpCookieAccess.java Fri Dec 16 16:09:41 2011 +0000
@@ -0,0 +1,44 @@
+/*
+ * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation. Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+package sun.misc;
+
+import java.net.HttpCookie;
+import java.util.List;
+
+public interface JavaNetHttpCookieAccess {
+ /*
+ * Constructs cookies from Set-Cookie or Set-Cookie2 header string,
+ * retaining the original header String in the cookie itself.
+ */
+ public List<HttpCookie> parse(String header);
+
+ /*
+ * Returns the original header this cookie was consructed from, if it was
+ * constructed by parsing a header, otherwise null.
+ */
+ public String header(HttpCookie cookie);
+}
+
--- a/jdk/src/share/classes/sun/misc/SharedSecrets.java Thu Dec 15 19:52:13 2011 -0800
+++ b/jdk/src/share/classes/sun/misc/SharedSecrets.java Fri Dec 16 16:09:41 2011 +0000
@@ -47,6 +47,7 @@
private static JavaLangAccess javaLangAccess;
private static JavaIOAccess javaIOAccess;
private static JavaNetAccess javaNetAccess;
+ private static JavaNetHttpCookieAccess javaNetHttpCookieAccess;
private static JavaNioAccess javaNioAccess;
private static JavaIOFileDescriptorAccess javaIOFileDescriptorAccess;
private static JavaSecurityProtectionDomainAccess javaSecurityProtectionDomainAccess;
@@ -81,6 +82,16 @@
return javaNetAccess;
}
+ public static void setJavaNetHttpCookieAccess(JavaNetHttpCookieAccess a) {
+ javaNetHttpCookieAccess = a;
+ }
+
+ public static JavaNetHttpCookieAccess getJavaNetHttpCookieAccess() {
+ if (javaNetHttpCookieAccess == null)
+ unsafe.ensureClassInitialized(java.net.HttpCookie.class);
+ return javaNetHttpCookieAccess;
+ }
+
public static void setJavaNioAccess(JavaNioAccess jna) {
javaNioAccess = jna;
}
--- a/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java Thu Dec 15 19:52:13 2011 -0800
+++ b/jdk/src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java Fri Dec 16 16:09:41 2011 +0000
@@ -32,6 +32,7 @@
import java.net.HttpRetryException;
import java.net.PasswordAuthentication;
import java.net.Authenticator;
+import java.net.HttpCookie;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.net.SocketTimeoutException;
@@ -46,6 +47,8 @@
import java.net.CacheRequest;
import java.net.Authenticator.RequestorType;
import java.io.*;
+import java.util.ArrayList;
+import java.util.Collections;
import java.util.Date;
import java.util.Map;
import java.util.List;
@@ -2580,6 +2583,80 @@
return false;
}
+ // constant strings represent set-cookie header names
+ private final static String SET_COOKIE = "set-cookie";
+ private final static String SET_COOKIE2 = "set-cookie2";
+
+ /**
+ * Returns a filtered version of the given headers value.
+ *
+ * Note: The implementation currently only filters out HttpOnly cookies
+ * from Set-Cookie and Set-Cookie2 headers.
+ */
+ private String filterHeaderField(String name, String value) {
+ if (value == null)
+ return null;
+
+ if (SET_COOKIE.equalsIgnoreCase(name) ||
+ SET_COOKIE2.equalsIgnoreCase(name)) {
+ // Filtering only if there is a cookie handler. [Assumption: the
+ // cookie handler will store/retrieve the HttpOnly cookies]
+ if (cookieHandler == null)
+ return value;
+
+ sun.misc.JavaNetHttpCookieAccess access =
+ sun.misc.SharedSecrets.getJavaNetHttpCookieAccess();
+ StringBuilder retValue = new StringBuilder();
+ List<HttpCookie> cookies = access.parse(value);
+ boolean multipleCookies = false;
+ for (HttpCookie cookie : cookies) {
+ // skip HttpOnly cookies
+ if (cookie.isHttpOnly())
+ continue;
+ if (multipleCookies)
+ retValue.append(','); // RFC 2965, comma separated
+ retValue.append(access.header(cookie));
+ multipleCookies = true;
+ }
+
+ return retValue.length() == 0 ? null : retValue.toString();
+ }
+
+ return value;
+ }
+
+ // Cache the filtered response headers so that they don't need
+ // to be generated for every getHeaderFields() call.
+ private Map<String, List<String>> filteredHeaders; // null
+
+ private Map<String, List<String>> getFilteredHeaderFields() {
+ if (filteredHeaders != null)
+ return filteredHeaders;
+
+ filteredHeaders = new HashMap<>();
+ Map<String, List<String>> headers;
+
+ if (cachedHeaders != null)
+ headers = cachedHeaders.getHeaders();
+ else
+ headers = responses.getHeaders();
+
+ for (Map.Entry<String, List<String>> e: headers.entrySet()) {
+ String key = e.getKey();
+ List<String> values = e.getValue(), filteredVals = new ArrayList<>();
+ for (String value : values) {
+ String fVal = filterHeaderField(key, value);
+ if (fVal != null)
+ filteredVals.add(fVal);
+ }
+ if (!filteredVals.isEmpty())
+ filteredHeaders.put(key,
+ Collections.unmodifiableList(filteredVals));
+ }
+
+ return filteredHeaders;
+ }
+
/**
* Gets a header field by name. Returns null if not known.
* @param name the name of the header field
@@ -2591,10 +2668,10 @@
} catch (IOException e) {}
if (cachedHeaders != null) {
- return cachedHeaders.findValue(name);
+ return filterHeaderField(name, cachedHeaders.findValue(name));
}
- return responses.findValue(name);
+ return filterHeaderField(name, responses.findValue(name));
}
/**
@@ -2613,11 +2690,7 @@
getInputStream();
} catch (IOException e) {}
- if (cachedHeaders != null) {
- return cachedHeaders.getHeaders();
- }
-
- return responses.getHeaders();
+ return getFilteredHeaderFields();
}
/**
@@ -2631,9 +2704,10 @@
} catch (IOException e) {}
if (cachedHeaders != null) {
- return cachedHeaders.getValue(n);
+ return filterHeaderField(cachedHeaders.getKey(n),
+ cachedHeaders.getValue(n));
}
- return responses.getValue(n);
+ return filterHeaderField(responses.getKey(n), responses.getValue(n));
}
/**
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/jdk/test/sun/net/www/protocol/http/HttpOnly.java Fri Dec 16 16:09:41 2011 +0000
@@ -0,0 +1,242 @@
+/*
+ * Copyright (c) 2011, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+/**
+ * @test
+ * @bug 7095980
+ * @summary Ensure HttpURLConnection (and supporting APIs) don't expose
+ * HttpOnly cookies
+ */
+
+import java.io.IOException;
+import java.net.CookieHandler;
+import java.net.CookieManager;
+import java.net.CookiePolicy;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.URI;
+import java.net.HttpURLConnection;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import com.sun.net.httpserver.Headers;
+import com.sun.net.httpserver.HttpExchange;
+import com.sun.net.httpserver.HttpHandler;
+import com.sun.net.httpserver.HttpServer;
+
+/*
+ * 1) start the HTTP server
+ * 2) populate cookie store with HttpOnly cookies
+ * 3) make HTTP request that should contain HttpOnly cookies
+ * 4) check HttpOnly cookies received by server
+ * 5) server reply with Set-Cookie containing HttpOnly cookie
+ * 6) check HttpOnly cookies are not accessible from Http client
+ */
+
+public class HttpOnly {
+
+ static final String URI_PATH = "/xxyyzz/";
+ static final int SESSION_ID = 12345;
+
+ void test(String[] args) throws Exception {
+ HttpServer server = startHttpServer();
+ CookieHandler previousHandler = CookieHandler.getDefault();
+ try {
+ InetSocketAddress address = server.getAddress();
+ URI uri = new URI("http://" + InetAddress.getLocalHost().getHostAddress()
+ + ":" + address.getPort() + URI_PATH);
+ populateCookieStore(uri);
+ doClient(uri);
+ } finally {
+ CookieHandler.setDefault(previousHandler);
+ server.stop(0);
+ }
+ }
+
+ void populateCookieStore(URI uri)
+ throws IOException {
+
+ CookieManager cm = new CookieManager(null, CookiePolicy.ACCEPT_ALL);
+ CookieHandler.setDefault(cm);
+ Map<String,List<String>> header = new HashMap<>();
+ List<String> values = new ArrayList<>();
+ values.add("JSESSIONID=" + SESSION_ID + "; version=1; Path="
+ + URI_PATH +"; HttpOnly");
+ values.add("CUSTOMER=WILE_E_COYOTE; version=1; Path=" + URI_PATH);
+ header.put("Set-Cookie", values);
+ cm.put(uri, header);
+ }
+
+ void doClient(URI uri) throws Exception {
+ HttpURLConnection uc = (HttpURLConnection) uri.toURL().openConnection();
+ int resp = uc.getResponseCode();
+ check(resp == 200,
+ "Unexpected response code. Expected 200, got " + resp);
+
+ // TEST 1: check getRequestProperty doesn't return the HttpOnly cookie
+ // In fact, that it doesn't return any automatically set cookies.
+ String cookie = uc.getRequestProperty("Cookie");
+ check(cookie == null,
+ "Cookie header returned from getRequestProperty, value " + cookie);
+
+ // TEST 2: check getRequestProperties doesn't return the HttpOnly cookie.
+ // In fact, that it doesn't return any automatically set cookies.
+ Map<String,List<String>> reqHeaders = uc.getRequestProperties();
+ Set<Map.Entry<String,List<String>>> entries = reqHeaders.entrySet();
+ for (Map.Entry<String,List<String>> entry : entries) {
+ String header = entry.getKey();
+ check(!"Cookie".equalsIgnoreCase(header),
+ "Cookie header returned from getRequestProperties, value " +
+ entry.getValue());
+ }
+
+ // TEST 3: check getHeaderField doesn't return Set-Cookie with HttpOnly
+ String setCookie = uc.getHeaderField("Set-Cookie");
+ if (setCookie != null) {
+ debug("Set-Cookie:" + setCookie);
+ check(!setCookie.toLowerCase().contains("httponly"),
+ "getHeaderField returned Set-Cookie header with HttpOnly, " +
+ "value = " + setCookie);
+ }
+
+ // TEST 3.5: check getHeaderField doesn't return Set-Cookie2 with HttpOnly
+ String setCookie2 = uc.getHeaderField("Set-Cookie2");
+ if (setCookie2 != null) {
+ debug("Set-Cookie2:" + setCookie2);
+ check(!setCookie2.toLowerCase().contains("httponly"),
+ "getHeaderField returned Set-Cookie2 header with HttpOnly, " +
+ "value = " + setCookie2);
+ }
+
+ // TEST 4: check getHeaderFields doesn't return Set-Cookie
+ // or Set-Cookie2 headers with HttpOnly
+ Map<String,List<String>> respHeaders = uc.getHeaderFields();
+ Set<Map.Entry<String,List<String>>> respEntries = respHeaders.entrySet();
+ for (Map.Entry<String,List<String>> entry : respEntries) {
+ String header = entry.getKey();
+ if ("Set-Cookie".equalsIgnoreCase(header)) {
+ List<String> setCookieValues = entry.getValue();
+ debug("Set-Cookie:" + setCookieValues);
+ for (String value : setCookieValues)
+ check(!value.toLowerCase().contains("httponly"),
+ "getHeaderFields returned Set-Cookie header with HttpOnly, "
+ + "value = " + value);
+ }
+ if ("Set-Cookie2".equalsIgnoreCase(header)) {
+ List<String> setCookieValues = entry.getValue();
+ debug("Set-Cookie2:" + setCookieValues);
+ for (String value : setCookieValues)
+ check(!value.toLowerCase().contains("httponly"),
+ "getHeaderFields returned Set-Cookie2 header with HttpOnly, "
+ + "value = " + value);
+ }
+ }
+
+ // Now add some user set cookies into the mix.
+ uc = (HttpURLConnection) uri.toURL().openConnection();
+ uc.addRequestProperty("Cookie", "CUSTOMER_ID=CHEGAR;");
+ resp = uc.getResponseCode();
+ check(resp == 200,
+ "Unexpected response code. Expected 200, got " + resp);
+
+ // TEST 5: check getRequestProperty doesn't return the HttpOnly cookie
+ cookie = uc.getRequestProperty("Cookie");
+ check(!cookie.toLowerCase().contains("httponly"),
+ "HttpOnly cookie returned from getRequestProperty, value " + cookie);
+
+ // TEST 6: check getRequestProperties doesn't return the HttpOnly cookie.
+ reqHeaders = uc.getRequestProperties();
+ entries = reqHeaders.entrySet();
+ for (Map.Entry<String,List<String>> entry : entries) {
+ String header = entry.getKey();
+ if ("Cookie".equalsIgnoreCase(header)) {
+ for (String val : entry.getValue())
+ check(!val.toLowerCase().contains("httponly"),
+ "HttpOnly cookie returned from getRequestProperties," +
+ " value " + val);
+ }
+ }
+ }
+
+ // HTTP Server
+ HttpServer startHttpServer() throws IOException {
+ HttpServer httpServer = HttpServer.create(new InetSocketAddress(0), 0);
+ httpServer.createContext(URI_PATH, new SimpleHandler());
+ httpServer.start();
+ return httpServer;
+ }
+
+ class SimpleHandler implements HttpHandler {
+ @Override
+ public void handle(HttpExchange t) throws IOException {
+ Headers reqHeaders = t.getRequestHeaders();
+
+ // some small sanity check
+ List<String> cookies = reqHeaders.get("Cookie");
+ for (String cookie : cookies) {
+ if (!cookie.contains("JSESSIONID")
+ || !cookie.contains("WILE_E_COYOTE"))
+ t.sendResponseHeaders(400, -1);
+ }
+
+ // return some cookies so we can check getHeaderField(s)
+ Headers respHeaders = t.getResponseHeaders();
+ List<String> values = new ArrayList<>();
+ values.add("ID=JOEBLOGGS; version=1; Path=" + URI_PATH);
+ values.add("NEW_JSESSIONID=" + (SESSION_ID+1) + "; version=1; Path="
+ + URI_PATH +"; HttpOnly");
+ values.add("NEW_CUSTOMER=WILE_E_COYOTE2; version=1; Path=" + URI_PATH);
+ respHeaders.put("Set-Cookie", values);
+ values = new ArrayList<>();
+ values.add("COOKIE2_CUSTOMER=WILE_E_COYOTE2; version=1; Path="
+ + URI_PATH);
+ respHeaders.put("Set-Cookie2", values);
+ values.add("COOKIE2_JSESSIONID=" + (SESSION_ID+100)
+ + "; version=1; Path=" + URI_PATH +"; HttpOnly");
+ respHeaders.put("Set-Cookie2", values);
+
+ t.sendResponseHeaders(200, -1);
+ t.close();
+ }
+ }
+
+ volatile int passed = 0, failed = 0;
+ boolean debug = false;
+ void pass() {passed++;}
+ void fail() {failed++;}
+ void fail(String msg) {System.err.println(msg); fail();}
+ void unexpected(Throwable t) {failed++; t.printStackTrace();}
+ void debug(String message) { if (debug) System.out.println(message); }
+ void check(boolean cond, String failMessage) {if (cond) pass(); else fail(failMessage);}
+ public static void main(String[] args) throws Throwable {
+ Class<?> k = new Object(){}.getClass().getEnclosingClass();
+ try {k.getMethod("instanceMain",String[].class)
+ .invoke( k.newInstance(), (Object) args);}
+ catch (Throwable e) {throw e.getCause();}}
+ public void instanceMain(String[] args) throws Throwable {
+ try {test(args);} catch (Throwable t) {unexpected(t);}
+ System.out.printf("%nPassed = %d, failed = %d%n%n", passed, failed);
+ if (failed > 0) throw new AssertionError("Some tests failed");}
+}
+