7

I hava an app in Google Play, I received a mail from Google saying that:

Your app(s) listed at the end of this email use an unsafe implementation of the interface X509TrustManager. Specifically, the implementation ignores all SSL certificate validation errors when establishing an HTTPS connection to a remote host, thereby making your app vulnerable to man-in-the-middle attacks.

To properly handle SSL certificate validation, change your code in the checkServerTrusted method of your custom X509TrustManager interface to raise either CertificateException or IllegalArgumentException whenever the certificate presented by the server does not meet your expectations.

My app uses "https", my checkServerTrusted() is the following:

 TrustManager tm = new X509TrustManager() {
        public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
        }

        public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {

        }

        public X509Certificate[] getAcceptedIssuers() {
            return null;
        }
    };

Then I modify this function:

 TrustManager tm = new X509TrustManager() {
        public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
        }

        public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
            if (chain == null) {
                throw new IllegalArgumentException("checkServerTrusted: X509Certificate array is null");
            }

            if (!(chain.length > 0)) {
                throw new IllegalArgumentException("checkServerTrusted: X509Certificate is empty");
            }

            if (!(null != authType && authType.equalsIgnoreCase("RSA"))) {

                throw new CertificateException("checkServerTrusted: AuthType is not RSA");
            }
        }

        public X509Certificate[] getAcceptedIssuers() {
            return null;
        }

the custom SSLSocketFactory:

public class MySSLSocketFactory extends SSLSocketFactory {
SSLContext sslContext = SSLContext.getInstance("TLS");

public MySSLSocketFactory(KeyStore ctx) throws NoSuchAlgorithmException, KeyManagementException, KeyStoreException, UnrecoverableKeyException {
    super(ctx);

    TrustManager tm = new X509TrustManager() {
        public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
        }

        public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
        }

        public X509Certificate[] getAcceptedIssuers() {
            return null;
        }
    };

    sslContext.init(null, new TrustManager[]{tm}, null);
}

@Override
public Socket createSocket(Socket socket, String host, int port, boolean autoClose) throws IOException, UnknownHostException {
    return sslContext.getSocketFactory().createSocket(socket, host, port, autoClose);
}

@Override
public Socket createSocket() throws IOException {
    return sslContext.getSocketFactory().createSocket();
}

}

the HttpClient function:

private static HttpClient getHttpClient(int timeout) {
    if (null == mHttpClient) {

        try {
            KeyStore trustStore = KeyStore.getInstance(KeyStore
                    .getDefaultType());
            trustStore.load(null, null);
            SSLSocketFactory sf = new MySSLSocketFactory(trustStore);
            sf.setHostnameVerifier(SSLSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER); 

            HttpParams params = new BasicHttpParams();

            HttpProtocolParams.setVersion(params, HttpVersion.HTTP_1_1);
            HttpProtocolParams.setContentCharset(params,
                    HTTP.DEFAULT_CONTENT_CHARSET);
            HttpProtocolParams.setUseExpectContinue(params, true);


            ConnManagerParams.setTimeout(params, timeout);

            HttpConnectionParams.setConnectionTimeout(params, timeout);

            HttpConnectionParams.setSoTimeout(params, timeout);


            SchemeRegistry schReg = new SchemeRegistry();
            schReg.register(new Scheme("http", PlainSocketFactory
                    .getSocketFactory(), 80));
            schReg.register(new Scheme("https", sf, 443));

            ClientConnectionManager conManager = new ThreadSafeClientConnManager(
                    params, schReg);

            mHttpClient = new DefaultHttpClient(conManager, params);
        } catch (Exception e) {
            e.printStackTrace();
            return new DefaultHttpClient();
        }
    }
    return mHttpClient;
}

But I do not know well about this,I just modify my code by what the email said,I think I have not sloved this problem.What is this warning all about? How to solve it?

zys
  • 1,306
  • 3
  • 18
  • 37
  • 3
    Why do you have this `TrustManager` in the first place? You do not need one to use SSL. – CommonsWare Feb 22 '16 at 13:40
  • Are you using any 3rd party SDK? – Idan Feb 22 '16 at 20:40
  • @CommonsWare what should i do? – zys Feb 23 '16 at 00:29
  • @Idan it is not 3rd party SDK – zys Feb 23 '16 at 00:29
  • "what should i do?" -- as Antimony indicated, ideally you get rid of it. I do not understand why your app would need one. If you do decide that you need it, implement it properly to validate the certificates and prevent MITM attacks. If you do not know how to do that, open up a fresh Stack Overflow question, where you provide your code and explain, **completely and precisely**, why you need a custom `TrustManager`, and perhaps we can help point out how you can implement that. For example, if you want to handle self-signed certificates, there are recipes for doing that. – CommonsWare Feb 23 '16 at 00:51
  • @CommonsWare My app is a servermonitor,users can add any website or server to ping it I posted my custom SSLSocketFactory and HttpClient connect funtion in the above.If I remove custom SSLSocketFactory,I use SSLSocketFactory directly? – zys Feb 23 '16 at 01:04
  • Um, you just do the HTTPS request, assuming that by "ping" you mean "make a request of some user-supplied HTTPS resource on that server". HTTPS/SSL support is built into all the major Android HTTP APIs (`HttpURLConnection`, Apache's standalone HttpClient for Android, OkHttp, Volley, etc.). You do not need to do anything with an `SSLSocketFactory`. If by "ping" you mean that you are opening a socket on the server but are not actually performing an HTTPS request... off the top of my head, I don't know how to do that. – CommonsWare Feb 23 '16 at 01:06
  • @CommonsWare If I remove custom `TrsustManager`,when visit a website that the certificate is customed by the website itself.There will be a warning that the certificate is not safe – zys Feb 23 '16 at 01:26
  • @CommonsWare Custom `TrsustManager` to allow all certificate(custom certificate and trusted certificate) – zys Feb 23 '16 at 01:27
  • @CommonsWare "Why do you have this TrustManager in the first place? You do not need one to use SSL " Can you point me to some example which doesn't use TrustManager at all to use SSL ? – Sharp Edge Jul 12 '16 at 09:53
  • 2
    @SharpEdge: Almost every Java example of using an `https` URL does not use a custom `TrustManager`. [Here is one](https://github.com/commonsguy/cw-omnibus/tree/master/Service/Downloader) that uses `HttpURLConnection` to download a PDF. [Here is one](https://github.com/commonsguy/cw-omnibus/tree/master/HTTP/Retrofit) that uses Retrofit to retrieve questions from the Stack Exchange API; [here is the Volley equivalent](https://github.com/commonsguy/cw-omnibus/tree/master/HTTP/Volley); [here is the OkHttp3 equivalent](https://github.com/commonsguy/cw-omnibus/tree/master/Internet/OkHttp3). – CommonsWare Jul 12 '16 at 12:05
  • @CommonsWare Thank you :) – Sharp Edge Jul 12 '16 at 19:08

4 Answers4

8

I found this solution ,it works well!

X509TrustManager:

public class EasyX509TrustManager
    implements X509TrustManager {

private X509TrustManager standardTrustManager = null;

/**
 * Constructor for EasyX509TrustManager.
 */
public EasyX509TrustManager(KeyStore keystore)
        throws NoSuchAlgorithmException, KeyStoreException {
    super();
    TrustManagerFactory factory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
    factory.init(keystore);
    TrustManager[] trustmanagers = factory.getTrustManagers();
    if (trustmanagers.length == 0) {
        throw new NoSuchAlgorithmException("no trust manager found");
    }
    this.standardTrustManager = (X509TrustManager) trustmanagers[0];
}

/**
 * @see X509TrustManager#checkClientTrusted(X509Certificate[], String authType)
 */
public void checkClientTrusted(X509Certificate[] certificates, String authType)
        throws CertificateException {
    standardTrustManager.checkClientTrusted(certificates, authType);
}

/**
 * @see X509TrustManager#checkServerTrusted(X509Certificate[], String authType)
 */
public void checkServerTrusted(X509Certificate[] certificates, String authType)
        throws CertificateException {
    if ((certificates != null) && (certificates.length == 1)) {
        certificates[0].checkValidity();
    } else {
        standardTrustManager.checkServerTrusted(certificates, authType);
    }
}

/**
 * @see X509TrustManager#getAcceptedIssuers()
 */
public X509Certificate[] getAcceptedIssuers() {
    return this.standardTrustManager.getAcceptedIssuers();
}

}

SSLSocketFactory:

public class EasySSLSocketFactory implements LayeredSocketFactory {

private SSLContext sslcontext = null;

private static SSLContext createEasySSLContext() throws IOException {
    try {
        SSLContext context = SSLContext.getInstance("TLS");
        context.init(null, new TrustManager[]{new EasyX509TrustManager(
                null)}, null);
        return context;
    } catch (Exception e) {
        throw new IOException(e.getMessage());
    }
}

private SSLContext getSSLContext() throws IOException {
    if (this.sslcontext == null) {
        this.sslcontext = createEasySSLContext();
    }
    return this.sslcontext;
}

/**
 * @see org.apache.http.conn.scheme.SocketFactory#connectSocket(Socket,
 * String, int, InetAddress, int,
 * HttpParams)
 */
public Socket connectSocket(Socket sock, String host, int port,
                            InetAddress localAddress, int localPort, HttpParams params)
        throws IOException, UnknownHostException, ConnectTimeoutException {
    int connTimeout = HttpConnectionParams.getConnectionTimeout(params);
    int soTimeout = HttpConnectionParams.getSoTimeout(params);

    InetSocketAddress remoteAddress = new InetSocketAddress(host, port);
    SSLSocket sslsock = (SSLSocket) ((sock != null) ? sock : createSocket());

    if ((localAddress != null) || (localPort > 0)) {
        // we need to bind explicitly
        if (localPort < 0) {
            localPort = 0; // indicates "any"
        }
        InetSocketAddress isa = new InetSocketAddress(localAddress,
                localPort);
        sslsock.bind(isa);
    }

    sslsock.connect(remoteAddress, connTimeout);
    sslsock.setSoTimeout(soTimeout);
    return sslsock;

}

/**
 * @see org.apache.http.conn.scheme.SocketFactory#createSocket()
 */
public Socket createSocket() throws IOException {
    return getSSLContext().getSocketFactory().createSocket();
}

/**
 * @see org.apache.http.conn.scheme.SocketFactory#isSecure(Socket)
 */
public boolean isSecure(Socket socket) throws IllegalArgumentException {
    return true;
}

/**
 * @see LayeredSocketFactory#createSocket(Socket,
 * String, int, boolean)
 */
public Socket createSocket(Socket socket, String host, int port,
                           boolean autoClose) throws IOException, UnknownHostException {
    return getSSLContext().getSocketFactory().createSocket(socket, host, port, autoClose);
}

// -------------------------------------------------------------------
// javadoc in org.apache.http.conn.scheme.SocketFactory says :
// Both Object.equals() and Object.hashCode() must be overridden
// for the correct operation of some connection managers
// -------------------------------------------------------------------

public boolean equals(Object obj) {
    return ((obj != null) && obj.getClass().equals(
            EasySSLSocketFactory.class));
}

public int hashCode() {
    return EasySSLSocketFactory.class.hashCode();
}

}

Then:

SchemeRegistry schReg = new SchemeRegistry();
            schReg.register(new Scheme("http", PlainSocketFactory
                    .getSocketFactory(), 80));
            schReg.register(new Scheme("https", new EasySSLSocketFactory(), 443));
Community
  • 1
  • 1
zys
  • 1,306
  • 3
  • 18
  • 37
  • 3
    Don't use this very bad code! The code allows man-in-the-middle attacks and renders the entire point of SSL null. The `checkValidity()` method only checks if the certificate is not expired and nothing else, meaning this code will happily accept ANY not expired certificate whatsoever, even if the certificate is fake, for another server and not signed by anything. – Nohus May 31 '17 at 19:54
  • @Nohus I know, but my purpose is accept all certificate – zys Jun 01 '17 at 09:36
  • 2
    That's all right if that's what you really need, but it doesn't answer the question on how to fix the security problem noticed by Google. – Nohus Jun 02 '17 at 16:02
  • does anyone know where is this CertificateException is being caught? – Artanis Dec 28 '18 at 17:54
  • 1
    @Nohus, when you say "The `checkValidity()` method only checks if the certificate is not expired and nothing else, meaning this code will happily accept ANY not expired certificate whatsoever, even if the certificate is fake, for another server and not signed by anything", what would you need to do with a delegating X509TrustManager implementation to include a check on the signature? – Gary Aug 13 '20 at 18:10
  • @Gary I don't remember now, but even when you do check the signature, you also need to check the signatures of all certificates in the signature chain, check their ordering, check that they correctly sign themselves in order, and that the root signature is trusted. The best way is to just not override the implementation, the default one works correctly. And at the end of the day, you are left wondering if you made a mistake and your app can be hacked. But I understand you may need some custom behavior. – Nohus Aug 15 '20 at 14:43
2

Your proposed modifications do not fix the security vulnerability. Your code will still accept any correctly formatted certificate, regardless of validity.

If you aren't sure how to properly verify certificates, you should just remove the custom trust manager. You don't need one unless you are doing something unusual.

Antimony
  • 37,781
  • 10
  • 100
  • 107
  • My app is a servermonitor,users can add any website or server to ping it.If I remove the custom trustmanager,I will also must remove to custom SSLSocketFactory. – zys Feb 23 '16 at 00:37
  • Perhaps you could add an interface so that users can add self-signed certs that they want to whitelist? That should be a rare occurrence anyway. – Antimony Feb 23 '16 at 01:13
  • If I remove custom TrsustManager,when visit a website that the certificate is customed by the website itself.There will be a warning that the certificate is not safe. – zys Feb 23 '16 at 01:28
0

The simplest way, is by not providing own custom TrustManager. Just use default TrustManager and it will do public key(X.509) validation and verification for you.

Awan Biru
  • 373
  • 2
  • 10
0

Make use of the default X509trustmanager's method only which are checkServerTrusted(chain, authType) and they will take care of all validation appropriately.

chetan007
  • 11
  • 4