7095980: Ensure HttpURLConnection (and supporting APIs) don't expose HttpOnly cookies

Reviewed-by: michaelm

src/share/classes/java/net/HttpCookie.java

@@ -72,6 +72,10 @@ public final class HttpCookie implements Cloneable {
     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 @@ public final class HttpCookie implements Cloneable {
      * @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 @@ public final class HttpCookie implements Cloneable {
 
         whenCreated = System.currentTimeMillis();
         portlist = null;
+        this.header = header;
     }
 
     /**
@@ -163,6 +172,15 @@ public final class HttpCookie implements Cloneable {
      *          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 @@ public final class HttpCookie implements Cloneable {
         // 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 @@ public final class HttpCookie implements Cloneable {
             // 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 @@ public final class HttpCookie implements Cloneable {
      * @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 @@ public final class HttpCookie implements Cloneable {
             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 @@ public final class HttpCookie implements Cloneable {
         }
     }
 
+    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.

src/share/classes/sun/misc/JavaNetHttpCookieAccess.java

+/*
+ * 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);
+}
+

src/share/classes/sun/misc/SharedSecrets.java

@@ -47,6 +47,7 @@ public class SharedSecrets {
     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 @@ public class SharedSecrets {
         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;
     }

src/share/classes/sun/net/www/protocol/http/HttpURLConnection.java

@@ -32,6 +32,7 @@ import java.net.ProtocolException;
 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.SecureCacheResponse;
 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 @@ public class HttpURLConnection extends java.net.HttpURLConnection {
         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 @@ public class HttpURLConnection extends java.net.HttpURLConnection {
         } 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 @@ public class HttpURLConnection extends java.net.HttpURLConnection {
             getInputStream();
         } catch (IOException e) {}
 
-        if (cachedHeaders != null) {
-            return cachedHeaders.getHeaders();
-        }
-
-        return responses.getHeaders();
+        return getFilteredHeaderFields();
     }
 
     /**
@@ -2631,9 +2704,10 @@ public class HttpURLConnection extends java.net.HttpURLConnection {
         } 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));
     }
 
     /**

test/sun/net/www/protocol/http/HttpOnly.java

+/*
+ * 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");}
+}
+