Merge pull request square#1244 from square/jwilson_1227_fix_alpn_leak

Always call ALPN.remove() after handshake.

okhttp/src/main/java/com/squareup/okhttp/CertificatePinner.java

@@ -19,6 +19,7 @@
 import java.security.cert.Certificate;
 import java.security.cert.X509Certificate;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -118,32 +119,39 @@ private CertificatePinner(Builder builder) {
    * @throws SSLPeerUnverifiedException if {@code peerCertificates} don't match
    *     the certificates pinned for {@code hostname}.
    */
-  public void check(String hostname, Certificate... peerCertificates)
+  public void check(String hostname, List<Certificate> peerCertificates)
       throws SSLPeerUnverifiedException {
     List<ByteString> pins = hostnameToPins.get(hostname);
     if (pins == null) return;
 
-    for (Certificate c : peerCertificates) {
-      X509Certificate x509Certificate = (X509Certificate) c;
+    for (int i = 0, size = peerCertificates.size(); i < size; i++) {
+      X509Certificate x509Certificate = (X509Certificate) peerCertificates.get(i);
       if (pins.contains(sha1(x509Certificate))) return; // Success!
     }
 
     // If we couldn't find a matching pin, format a nice exception.
     StringBuilder message = new StringBuilder()
         .append("Certificate pinning failure!")
         .append("\n  Peer certificate chain:");
-    for (Certificate c : peerCertificates) {
-      X509Certificate x509Certificate = (X509Certificate) c;
+    for (int i = 0, size = peerCertificates.size(); i < size; i++) {
+      X509Certificate x509Certificate = (X509Certificate) peerCertificates.get(i);
       message.append("\n    ").append(pin(x509Certificate))
           .append(": ").append(x509Certificate.getSubjectDN().getName());
     }
     message.append("\n  Pinned certificates for ").append(hostname).append(":");
-    for (ByteString pin : pins) {
+    for (int i = 0, size = pins.size(); i < size; i++) {
+      ByteString pin = pins.get(i);
       message.append("\n    sha1/").append(pin.base64());
     }
     throw new SSLPeerUnverifiedException(message.toString());
   }
 
+  /** @deprecated replaced with {@link #check(String, List)}. */
+  public void check(String hostname, Certificate... peerCertificates)
+      throws SSLPeerUnverifiedException {
+    check(hostname, Arrays.asList(peerCertificates));
+  }
+
   /**
    * Returns the SHA-1 of {@code certificate}'s public key. This uses the
    * mechanism Moxie Marlinspike describes in <a

okhttp/src/main/java/com/squareup/okhttp/Connection.java

@@ -236,8 +236,20 @@ private void upgradeToTls(Request tunnelRequest, int readTimeout, int writeTimeo
     // Configure the socket's ciphers, TLS versions, and extensions.
     route.connectionSpec.apply(sslSocket, route);
 
-    // Force handshake. This can throw!
-    sslSocket.startHandshake();
+    try {
+      // Force handshake. This can throw!
+      sslSocket.startHandshake();
+
+      String maybeProtocol;
+      if (route.connectionSpec.supportsTlsExtensions()
+          && (maybeProtocol = platform.getSelectedProtocol(sslSocket)) != null) {
+        protocol = Protocol.get(maybeProtocol); // Throws IOE on unknown.
+      }
+    } finally {
+      platform.afterHandshake(sslSocket);
+    }
+
+    handshake = Handshake.get(sslSocket.getSession());
 
     // Verify that the socket's certificates are acceptable for the target host.
     if (!route.address.hostnameVerifier.verify(route.address.uriHost, sslSocket.getSession())) {
@@ -249,16 +261,7 @@ private void upgradeToTls(Request tunnelRequest, int readTimeout, int writeTimeo
     }
 
     // Check that the certificate pinner is satisfied by the certificates presented.
-    route.address.certificatePinner.check(route.address.uriHost,
-        sslSocket.getSession().getPeerCertificates());
-
-    handshake = Handshake.get(sslSocket.getSession());
-
-    String maybeProtocol;
-    if (route.connectionSpec.supportsTlsExtensions()
-        && (maybeProtocol = platform.getSelectedProtocol(sslSocket)) != null) {
-      protocol = Protocol.get(maybeProtocol); // Throws IOE on unknown.
-    }
+    route.address.certificatePinner.check(route.address.uriHost, handshake.peerCertificates());
 
     if (protocol == Protocol.SPDY_3 || protocol == Protocol.HTTP_2) {
       sslSocket.setSoTimeout(0); // SPDY timeouts are set per-stream.

okhttp/src/main/java/com/squareup/okhttp/internal/Platform.java

@@ -83,6 +83,13 @@ public void configureTlsExtensions(SSLSocket sslSocket, String hostname,
       List<Protocol> protocols) {
   }
 
+  /**
+   * Called after the TLS handshake to release resources allocated by {@link
+   * #configureTlsExtensions}.
+   */
+  public void afterHandshake(SSLSocket sslSocket) {
+  }
+
   /** Returns the negotiated protocol, or null if no protocol was negotiated. */
   public String getSelectedProtocol(SSLSocket socket) {
     return null;
@@ -128,8 +135,9 @@ private static Platform findPlatform() {
       Class<?> serverProviderClass = Class.forName(negoClassName + "$ServerProvider");
       Method putMethod = negoClass.getMethod("put", SSLSocket.class, providerClass);
       Method getMethod = negoClass.getMethod("get", SSLSocket.class);
+      Method removeMethod = negoClass.getMethod("remove", SSLSocket.class);
       return new JdkWithJettyBootPlatform(
-          putMethod, getMethod, clientProviderClass, serverProviderClass);
+          putMethod, getMethod, removeMethod, clientProviderClass, serverProviderClass);
     } catch (ClassNotFoundException ignored) {
     } catch (NoSuchMethodException ignored) { // The ALPN version isn't what we expect.
     }
@@ -239,15 +247,17 @@ private Android(Method trafficStatsTagSocket, Method trafficStatsUntagSocket) {
    * OpenJDK 7+ with {@code org.mortbay.jetty.alpn/alpn-boot} in the boot class path.
    */
   private static class JdkWithJettyBootPlatform extends Platform {
-    private final Method getMethod;
     private final Method putMethod;
+    private final Method getMethod;
+    private final Method removeMethod;
     private final Class<?> clientProviderClass;
     private final Class<?> serverProviderClass;
 
-    public JdkWithJettyBootPlatform(Method putMethod, Method getMethod,
+    public JdkWithJettyBootPlatform(Method putMethod, Method getMethod, Method removeMethod,
         Class<?> clientProviderClass, Class<?> serverProviderClass) {
       this.putMethod = putMethod;
       this.getMethod = getMethod;
+      this.removeMethod = removeMethod;
       this.clientProviderClass = clientProviderClass;
       this.serverProviderClass = serverProviderClass;
     }
@@ -271,6 +281,16 @@ public JdkWithJettyBootPlatform(Method putMethod, Method getMethod,
       }
     }
 
+    @Override public void afterHandshake(SSLSocket sslSocket) {
+      try {
+        removeMethod.invoke(null, sslSocket);
+      } catch (IllegalAccessException ignored) {
+        throw new AssertionError();
+      } catch (InvocationTargetException ignored) {
+        throw new AssertionError();
+      }
+    }
+
     @Override public String getSelectedProtocol(SSLSocket socket) {
       try {
         JettyNegoProvider provider =