jchambers / pushy

A Java library for sending APNs (iOS/macOS/Safari) push notifications
https://pushy-apns.org/
MIT License
1.81k stars 450 forks source link

Add the ability to provide a custom TrustManager while creating SSLContext #36

Closed ssrini closed 10 years ago

ssrini commented 10 years ago

Currently SSLHandlerUtil provides the default Trust Managers. While this works with servers such as JBoss and SUN/Oracle JVM, we have had security exceptions (SSL trust) when using Pushy within other corporate server environments such as SAP NetWeaver and SAP JVM.

The only solution is to replace the SSLHandlerUtil class and provide a custom Trust Manager implementation that trusts the APNS certificates.

Enhancement request: Rather than having to patch Pushy for this, it would be better if a mechanism to provide a custom TrustManager is provided via ApnsEnvironment

jchambers commented 10 years ago

@ssrini I'm afraid I don't have ready access to the SAP stuff you're using, so it's difficult for me to test this on my own. Could you please provide an example of how you're constructing a TrustManager that works in your environment? I'm mostly curious whether we need to go straight to the custom TrustManager or if it makes sense to go with a custom TrustManagerFactory.

ssrini commented 10 years ago

@jchambers Below the entire code snippet of the static method that we reimplemented.

protected static SslHandler createSslHandler(final KeyStore keyStore, final char[] keyStorePassword) throws KeyManagementException, NoSuchAlgorithmException, KeyStoreException, UnrecoverableKeyException {
    String algorithm = Security.getProperty("ssl.KeyManagerFactory.algorithm");

    if (algorithm == null) {
        algorithm = DEFAULT_ALGORITHM;
    }

 // Create a trust manager that does not validate certificate chains
    TrustManager[] trustAllCerts = new TrustManager[]{new X509TrustManager() {

        public java.security.cert.X509Certificate[] getAcceptedIssuers() {
            return null;
        }

        public void checkClientTrusted(X509Certificate[] certs, String authType) {
        }

        public void checkServerTrusted(X509Certificate[] certs, String authType) {
        }
      }
    };

    final TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(algorithm);
    trustManagerFactory.init((KeyStore) null);

    final KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(algorithm);
    keyManagerFactory.init(keyStore, keyStorePassword);

    final SSLContext sslContext = SSLContext.getInstance(PROTOCOL);
    sslContext.init(keyManagerFactory.getKeyManagers(), trustAllCerts, null);

    final SSLEngine sslEngine = sslContext.createSSLEngine();
    sslEngine.setUseClientMode(true);

    return new SslHandler(sslEngine);
}
jchambers commented 10 years ago

Pardon the delay; I think The Right Thing To Do™ here is to let callers provide their own SSLContext via the PushManagerFactory (introduced in #38). Before we do that, though, we ought to just require TLS in all environments and update the mock servers accordingly (see #16). That is, I think, the next thing I'll be working on.

jchambers commented 10 years ago

@ssrini The changes in #46 will allow you to specify a custom SSLContext when constructing a PushManager, which should give you the flexibility you need to work around your servlet container's defaults.

That said, I'd like to take this opportunity to strongly discourage you from using a trust-all TrustManager in production. Instead, I'd encourage you to create a new keystore using keytool and import Apple's certificate as a trusted certificate. When you create your custom SSLContext, you can use your custom keystore (with Apple's certificate added as a trusted certificate) when creating the TrustManagerFactory and everything should work gracefully. Take a look at SSLUtil in #46 for an example of how we're doing something very similar for our own unit tests.

ssrini commented 10 years ago

@jchambers I will check and let you know. I like what you suggest on the keystore. Unfortunately many times customers do not allow us to install or tweak anything on their systems and there we need to go with the "unsafe" option of trusting all. Where feasible we will do what you suggest.