Safely accepting a known SSL certificate with a different host name

Question

My app communicates with a server over an internal network through HTTPS. The SSL certificate on this server is listed for the host as its external host name. I want to accept this certificate, but I don't want to accept ANY certificate as many workarounds I've seen seem to do. I would also like to have it so that later on, in the strong possibility that his app is accessing a different server with a correct SSL certificate, that I don't have to change it. I have it working as I would like it seems, but I am very unsure of myself when it comes to implementing my own methods to override a library used for secure communications, and I am not very strong in my knowledge of SSL communications.

I would basically like input on security loopholes and its effectiveness at doing what I need.

Here is the code I have:

HttpClient httpclient = new DefaultHttpClient();
SSLSocketFactory ssf = SSLSocketFactory.getSocketFactory();
ssf.setHostnameVerifier( new X509HostnameVerifier() {

   @Override
   public void verify( String host, String[] cns, String[] subjectAlts )
         throws SSLException
   {
   }

   @Override
   public void verify( String host, X509Certificate cert ) throws SSLException
   {               
   }

   @Override
   public void verify( String host, SSLSocket ssl ) throws IOException
   {                 
      boolean foundHost = false;
      for( javax.security.cert.X509Certificate cert : ssl.getSession().getPeerCertificateChain() )
      {
         String[] tokens = cert.getSubjectDN().getName().split( "," );
         for( String token : tokens )
         {
            String[] keyVal = token.split( "=" );

            if( keyVal.length > 1 && keyVal[0].equals( "CN" ) )
            {
               foundHost = keyVal[1].equals( context.getString( R.string.CertHostName ) );

               if(!foundHost)
               {
                foundHost = keyVal[1].equals( host );
               }
            }

            if(foundHost)
            {
               break;
            }      
         }
         if(foundHost)
         {
            break;
         }
      }

      if(!foundHost)
      {
         throw new IOException( "Mismatched host in SSL certificate" );
      }
   }

   @Override
   public boolean verify( String host, SSLSession session )
   {
      return false;
   }
} );
httpclient.getConnectionManager().getSchemeRegistry().unregister( "https" );
httpclient.getConnectionManager().getSchemeRegistry().register( new Scheme( "https", ssf, 443 ) );

Basically, I found out which method to override through some log statements to see which one was being used in the anonymous inner X509HostnameVerifier class, and hopefully handled it appropriately. Input would be appreciated.

Answer 1

There are a few items inhere I would suggest can be improved (assuming the security aspects of the code are valid - i.e. legal disclaimer about 'safe' and SSL means speak to someone who's more of a security expert than me).

First, I assume you are using the apache-commons based HttpClient code.... this is fine, but it would have been useful to show the import statements ;-).

Regardless. There are three things I would change.

First, whenever you do an anonymous class implementation and it has more than 1 method that needs to be overridden, then I recommend that you create a full class implementation for it. In this case, your code should have a (perhaps nested) class that implements the

private static final class AlternateHostnameVerifier implements X509HostnameVerifier {
    ....
}

then your code becomes:

HttpClient httpclient = new DefaultHttpClient();
SSLSocketFactory ssf = SSLSocketFactory.getSocketFactory();
ssf.setHostnameVerifier(new AlternateHostnameVerifier());
httpclient.getConnectionManager().getSchemeRegistry().unregister( "https" );
httpclient.getConnectionManager().getSchemeRegistry().register( new Scheme( "https", ssf, 443 ) );

This is a much better way to show the critical logic of your program, and still make it readable.

Next up, in your new AlternateHostnameVerifier class, I would recommend you implement all the interface methods fully. Leaving two of the methods as stub methods is only a short-term solution. In the future the apache-commons implementation may change and call one of the other verify methods, and then you have a sudden, unexplained bug. Specifically throwing a UnsupportedOperationException is a much better alternative than leaving it blank.

Finally, I have a preference for methods that can to simply 'return' when they succeed rather than trying to break out of loops and keep track of variables outside of the loops.

In this case, you have foundHost which tracks the state. you can eliminate this variable if you just 'return' each time you set it to true, and otherwise always throw the exception.

So, you can rewrite the code as:

@Override
public void verify( String host, SSLSocket ssl ) throws IOException {                 
  for( javax.security.cert.X509Certificate cert : ssl.getSession().getPeerCertificateChain() ) {
     String[] tokens = cert.getSubjectDN().getName().split( "," );
     for( String token : tokens ) {
        String[] keyVal = token.split( "=" , 2);

        if( keyVal.length > 1 && keyVal[0].equals( "CN" ) ) {
           if (keyVal[1].equals( host ) || keyVal[1].equals( context.getString( R.string.CertHostName ) )) {
               // our CN matches the Android's certificate host.
               return;
           }
        }
     }
  }
  throw new IOException( "Mismatched host in SSL certificate" );
}

Answer 2

Instead of implementing your X509HostnameVerifier from scratch, extend org.apache.http.conn.ssl.AbstractVerifier, which does most of that work for you. All you have to do is override .verify(String host, String[] cns, String[] subjectAlts, boolean stringWithSubDomains().