Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

getPinnedHttpsUrlConnection seems to leak sockets #9

Open
ashoykh opened this issue Aug 14, 2013 · 0 comments
Open

getPinnedHttpsUrlConnection seems to leak sockets #9

ashoykh opened this issue Aug 14, 2013 · 0 comments

Comments

@ashoykh
Copy link

ashoykh commented Aug 14, 2013

My code uses getPinnedHttpsUrlConnection to create url connections to my server. After it's done reading a response, it closes the UrlConnection. The UrlConnection documentation says that it will not free the socket that it opens, and in fact that is what I see - if I do 'adb -e shell ls -l /proc/[processid]/fd | grep socket' shows a new socket opened (I'm assuming it's in CLOSED_WAIT state). This isn't a problem yet. However, each time that I call getPinnedHttpsUrlConnection, a new socket is opened and left in this state. Eventually, after calling getPinnedHttpsUrlConnection enough times, the android system reaps my process for having too many open file descriptors.

I suspect the issue here is that we are creating a new SSLSocketFactory for each call to getPinnedHttpsUrlConnection. In fact, I verify that caching the SSLSocketFactory keeps the number of sockets open to a constant.

I suggest modifying the PinningHelper class in the following way:

public PinningHelper {
  private final static Map<List<String>,SSLSocketFactory> sSSLSocketFactories = new HashMap<List<String>, SSLSocketFactory>();
...stuff...
  public static HttpsURLConnection getPinnedHttpsURLConnection(Context context, String[] pins, URL url)
      throws IOException
  {
    if (!url.getProtocol().equals("https")) {
      throw new IllegalArgumentException("Attempt to construct pinned non-https connection!");
    }

    SSLSocketFactory pinnedSslSocketFactory = getPinnedSslSocketFactory(context, pins, 0);

    HttpsURLConnection urlConnection = (HttpsURLConnection)url.openConnection();
    urlConnection.setSSLSocketFactory(pinnedSslSocketFactory);

    return urlConnection;
  }

  public static SSLSocketFactory getPinnedSslSocketFactory(Context context, String[] pins, long enforceUntilTimestampMillis) {
    List<String> pinsList = Arrays.asList(pins);
    synchronized(sSSLSocketFactories) {
      if (!sSSLSocketFactories.containsKey(pinsList)) {
        sSSLSocketFactories.put(pinsList, createPinnedSslSocketFactory(context, pins, 0));
      }
      return sSSLSocketFactories.get(pinsList);
    }
  }

  private static SSLSocketFactory createPinnedSslSocketFactory(Context context, String[] pins, long enforceUntilTimestampMillis) {
    TrustManager[] trustManagers = new TrustManager[1];
    trustManagers[0]             = new PinningTrustManager(SystemKeyStore.getInstance(context), pins, enforceUntilTimestampMillis);

    try {
      SSLContext sslContext = SSLContext.getInstance("TLS");
      sslContext.init(null, trustManagers, null);

      return sslContext.getSocketFactory();
    } catch (NoSuchAlgorithmException nsae) {
      throw new AssertionError(nsae);
    } catch (KeyManagementException e) {
      throw new AssertionError(e);
    }
  }
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant