advancedtls: adding AdvancedTlsX509TrustManager and AdvancedTlsX509KeyManager

core/src/main/java/io/grpc/util/AdvancedTlsX509KeyManager.java

@@ -27,6 +27,7 @@
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
 import java.security.spec.InvalidKeySpecException;
+import java.util.Arrays;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
@@ -35,11 +36,11 @@
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.X509ExtendedKeyManager;
 
-@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8024")
 /**
  * AdvancedTlsX509KeyManager is an {@code X509ExtendedKeyManager} that allows users to configure
  * advanced TLS features, such as private key and certificate chain reloading, etc.
  */
+@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8024")
 public final class AdvancedTlsX509KeyManager extends X509ExtendedKeyManager {
   private static final Logger log = Logger.getLogger(AdvancedTlsX509KeyManager.class.getName());
 
@@ -57,12 +58,12 @@ public PrivateKey getPrivateKey(String alias) {
 
   @Override
   public X509Certificate[] getCertificateChain(String alias) {
-    return this.certs;
+    return Arrays.copyOf(this.certs, this.certs.length);
   }
 
   @Override
   public String[] getClientAliases(String keyType, Principal[] issuers) {
-    return new String[0];
+    return new String[] {"default"};
   }
 
   @Override
@@ -77,7 +78,7 @@ public String chooseEngineClientAlias(String[] keyType, Principal[] issuers, SSL
 
   @Override
   public String[] getServerAliases(String keyType, Principal[] issuers) {
-    return new String[0];
+    return new String[] {"default"};
   }
 
   @Override
@@ -93,38 +94,36 @@ public String chooseEngineServerAlias(String keyType, Principal[] issuers,
 
   /**
    * Updates the current cached private key and cert chains.
-   * Right now, callers should make sure |key| matches the public key on the leaf certificate of
-   * |certs|.
-   * TODO(ZhenLian): explore possibilities to do a crypto check here.
    *
    * @param key  the private key that is going to be used
    * @param certs  the certificate chain that is going to be used
    */
   public void updateIdentityCredentials(PrivateKey key, X509Certificate[] certs) {
     // Same question as the trust manager: what to do if copy is not feasible here?
+    // Right now, callers should make sure {@code key} matches the public key on the leaf
+    // certificate of {@code certs}.
+    // TODO(ZhenLian): explore possibilities to do a crypto check here.
     this.key = key;
-    this.certs = certs;
+    this.certs = Arrays.copyOf(certs, certs.length);
   }
 
   /**
-   * starts a {@code ScheduledExecutorService} to read private key and certificate chains from the
-   * local file paths periodically, and update the cached identity credentials if they are both
+   * Schedules a {@code ScheduledExecutorService} to read private key and certificate chains from
+   * the local file paths periodically, and update the cached identity credentials if they are both
    * updated.
    *
    * @param keyFile  the file on disk holding the private key
    * @param certFile  the file on disk holding the certificate chain
-   * @param initialDelay the time to delay first read-and-update execution
-   * @param delay the period between successive read-and-update executions
+   * @param period the period between successive read-and-update executions
    * @param unit the time unit of the initialDelay and period parameters
-   * @param executor the execute service we use to start the read-and-update thread
-   * @return an object that caller should close when the scheduled execution is not needed
+   * @param executor the execute service we use to read and update the credentials
+   * @return an object that caller should close when the file refreshes are not needed
    */
   public Closeable updateIdentityCredentialsFromFile(File keyFile, File certFile,
-      long initialDelay, long delay, TimeUnit unit, ScheduledExecutorService executor) {
+      long period, TimeUnit unit, ScheduledExecutorService executor) {
     final ScheduledFuture<?> future =
         executor.scheduleWithFixedDelay(
-            new LoadFilePathExecution(keyFile, certFile),
-            initialDelay, delay, unit);
+            new LoadFilePathExecution(keyFile, certFile), 0, period, unit);
     return new Closeable() {
       @Override public void close() {
         future.cancel(false);
@@ -174,8 +173,8 @@ public UpdateResult(boolean success, long keyTime, long certTime) {
   }
 
   /**
-   * reads the private key and certificates specified in the path locations. Updates |key| and
-   * |cert| if both of their modified time changed since last read.
+   * Reads the private key and certificates specified in the path locations. Updates {@code key} and
+   * {@code cert} if both of their modified time changed since last read.
    *
    * @param keyFile  the file on disk holding the private key
    * @param certFile  the file on disk holding the certificate chain
@@ -189,15 +188,21 @@ private UpdateResult readAndUpdate(File keyFile, File certFile, long oldKeyTime,
     long newCertTime = certFile.lastModified();
     // We only update when both the key and the certs are updated.
     if (newKeyTime != oldKeyTime && newCertTime != oldCertTime) {
-      PrivateKey key = CertificateUtils.getPrivateKey(new FileInputStream(keyFile));
-      X509Certificate[] certs = CertificateUtils.getX509Certificates(new FileInputStream(certFile));
+      FileInputStream keyInputStream = new FileInputStream(keyFile);
+      FileInputStream certInputStream = new FileInputStream(certFile);
+      PrivateKey key = CertificateUtils.getPrivateKey(keyInputStream);
+      X509Certificate[] certs = CertificateUtils.getX509Certificates(certInputStream);
+      keyInputStream.close();
+      certInputStream.close();
       updateIdentityCredentials(key, certs);
       return new UpdateResult(true, newKeyTime, newCertTime);
     }
     return new UpdateResult(false, -1, -1);
   }
 
-  // Mainly used to avoid throwing IO Exceptions in java.io.Closeable.
+  /**
+   * Mainly used to avoid throwing IO Exceptions in java.io.Closeable.
+   */
   public interface Closeable extends java.io.Closeable {
     @Override public void close();
   }

core/src/main/java/io/grpc/util/AdvancedTlsX509TrustManager.java

@@ -19,14 +19,14 @@
 import io.grpc.ExperimentalApi;
 import java.io.File;
 import java.io.FileInputStream;
-import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.net.Socket;
 import java.security.KeyStore;
 import java.security.KeyStoreException;
 import java.security.NoSuchAlgorithmException;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
+import java.util.Arrays;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
@@ -39,29 +39,27 @@
 import javax.net.ssl.X509ExtendedTrustManager;
 import org.codehaus.mojo.animal_sniffer.IgnoreJRERequirement;
 
-@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8024")
 /**
- * For Android users: this class is only supported in API level 24 and above.
  * AdvancedTlsX509TrustManager is an {@code X509ExtendedTrustManager} that allows users to configure
  * advanced TLS features, such as root certificate reloading, peer cert custom verification, etc.
+ * For Android users: this class is only supported in API level 24 and above.
  */
+@ExperimentalApi("https://github.com/grpc/grpc-java/issues/8024")
 @IgnoreJRERequirement
 public final class AdvancedTlsX509TrustManager extends X509ExtendedTrustManager {
   private static final Logger log = Logger.getLogger(AdvancedTlsX509TrustManager.class.getName());
 
-  private Verification verification;
-  private PeerVerifier peerVerifier;
-  private SslSocketPeerVerifier sslSocketPeerVerifier;
-  private SslEnginePeerVerifier sslEnginePeerVerifier;
+  private final Verification verification;
+  private final PeerVerifier peerVerifier;
+  private final SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier;
   // The trust certs we will use to verify peer certificate chain.
   private volatile X509Certificate[] trustCerts;
 
   private AdvancedTlsX509TrustManager(Verification verification,  PeerVerifier peerVerifier,
-      SslSocketPeerVerifier socketPeerVerifier, SslEnginePeerVerifier enginePeerVerifier) {
+      SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier) {
     this.verification = verification;
     this.peerVerifier = peerVerifier;
-    this.sslSocketPeerVerifier = socketPeerVerifier;
-    this.sslEnginePeerVerifier = enginePeerVerifier;
+    this.socketAndEnginePeerVerifier = socketAndEnginePeerVerifier;
   }
 
   @Override
@@ -121,7 +119,7 @@ public void updateTrustCredentials(X509Certificate[] trustCerts) {
     // Question: in design, we will do an copy of trustCerts, but that seems not easy, since
     // X509Certificate is an abstract class without copy constructor.
     // Can we document in the API that trustCerts shouldn't be modified once set?
-    this.trustCerts = trustCerts;
+    this.trustCerts = Arrays.copyOf(trustCerts, trustCerts.length);
   }
 
   private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine,
@@ -198,37 +196,33 @@ private void checkTrusted(X509Certificate[] chain, String authType, SSLEngine ss
       }
     }
     // Perform the additional peer cert check.
-    if (sslEngine != null && sslEnginePeerVerifier != null) {
-      sslEnginePeerVerifier.verifyPeerCertificate(chain, authType, sslEngine);
-    }
-    if (socket != null && sslSocketPeerVerifier != null) {
-      sslSocketPeerVerifier.verifyPeerCertificate(chain, authType, socket);
+    if (sslEngine != null && socket != null && socketAndEnginePeerVerifier != null) {
+      socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, socket);
+      socketAndEnginePeerVerifier.verifyPeerCertificate(chain, authType, sslEngine);
     }
     if (peerVerifier != null) {
       peerVerifier.verifyPeerCertificate(chain, authType);
     }
-    if (sslEnginePeerVerifier == null && sslSocketPeerVerifier == null && peerVerifier == null) {
+    if (socketAndEnginePeerVerifier == null && peerVerifier == null) {
       log.log(Level.INFO, "No peer verifier is specified");
     }
   }
 
   /**
-   * starts a {@code ScheduledExecutorService} to read trust certificates from a local file path
+   * Schedules a {@code ScheduledExecutorService} to read trust certificates from a local file path
    * periodically, and update the cached trust certs if there is an update.
    *
    * @param trustCertFile  the file on disk holding the trust certificates
-   * @param initialDelay the time to delay first read-and-update execution
-   * @param delay the period between successive read-and-update executions
+   * @param period the period between successive read-and-update executions
    * @param unit the time unit of the initialDelay and period parameters
-   * @param executor the execute service we use to start the read-and-update thread
-   * @return an object that caller should close when the scheduled execution is not needed
+   * @param executor the execute service we use to read and update the credentials
+   * @return an object that caller should close when the file refreshes are not needed
    */
-  public Closeable updateTrustCredentialsFromFile(File trustCertFile, long initialDelay,
-      long delay, TimeUnit unit, ScheduledExecutorService executor) {
+  public Closeable updateTrustCredentialsFromFile(File trustCertFile, long period, TimeUnit unit,
+      ScheduledExecutorService executor) {
     final ScheduledFuture<?> future =
         executor.scheduleWithFixedDelay(
-            new LoadFilePathExecution(trustCertFile),
-            initialDelay, delay, unit);
+            new LoadFilePathExecution(trustCertFile), 0, period, unit);
     return new Closeable() {
       @Override public void close() {
         future.cancel(false);
@@ -252,7 +246,7 @@ public void run() {
         if (newUpdateTime != 0) {
           this.currentTime = newUpdateTime;
         }
-      } catch (FileNotFoundException | CertificateException e) {
+      } catch (CertificateException | IOException e) {
         log.log(Level.SEVERE, "readAndUpdate() execution thread failed", e);
 
       }
@@ -268,11 +262,12 @@ public void run() {
    * @return 0 if failed or the modified time is not changed, otherwise the new modified time
    */
   private long readAndUpdate(File trustCertFile, long oldTime)
-      throws FileNotFoundException, CertificateException {
+      throws CertificateException, IOException {
     long newTime = trustCertFile.lastModified();
     if (newTime != oldTime) {
-      X509Certificate[] certificates = CertificateUtils.getX509Certificates(
-          new FileInputStream(trustCertFile));
+      FileInputStream inputStream = new FileInputStream(trustCertFile);
+      X509Certificate[] certificates = CertificateUtils.getX509Certificates(inputStream);
+      inputStream.close();
       updateTrustCredentials(certificates);
       return newTime;
     }
@@ -328,9 +323,9 @@ void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType)
   }
 
   // Additional custom peer verification check.
-  // It will be used when checkClientTrusted/checkServerTrusted is called with the {@code Socket}
-  // parameter.
-  public interface SslSocketPeerVerifier {
+  // It will be used when checkClientTrusted/checkServerTrusted is called with the {@code Socket} or
+  // the {@code SSLEngine} parameter.
+  public interface SslSocketAndEnginePeerVerifier {
     /**
      * Verifies the peer certificate chain. For more information, please refer to
      * {@code X509ExtendedTrustManager}.
@@ -342,12 +337,7 @@ public interface SslSocketPeerVerifier {
      */
     void verifyPeerCertificate(X509Certificate[] peerCertChain,  String authType, Socket socket)
         throws CertificateException;
-  }
 
-  // Additional custom peer verification check.
-  // It will be used when checkClientTrusted/checkServerTrusted is called with the {@code SSLEngine}
-  // parameter.
-  public interface SslEnginePeerVerifier {
     /**
      * Verifies the peer certificate chain. For more information, please refer to
      * {@code X509ExtendedTrustManager}.
@@ -363,36 +353,32 @@ void verifyPeerCertificate(X509Certificate[] peerCertChain, String authType, SSL
 
   public static final class Builder {
 
-    private Verification verification;
+    private Verification verification = Verification.CertificateAndHostNameVerification;
     private PeerVerifier peerVerifier;
-    private SslSocketPeerVerifier socketPeerVerifier;
-    private SslEnginePeerVerifier enginePeerVerifier;
+    private SslSocketAndEnginePeerVerifier socketAndEnginePeerVerifier;
+
+    private Builder() {}
 
     public Builder setVerification(Verification verification) {
       this.verification = verification;
       return this;
     }
 
-    public Builder setPeerVerifier(PeerVerifier peerVerifier) {
-      this.peerVerifier = peerVerifier;
-      return this;
-    }
-
-    public Builder setSslSocketPeerVerifier(SslSocketPeerVerifier peerVerifier) {
-      this.socketPeerVerifier = peerVerifier;
+    public Builder setPeerVerifier(PeerVerifier verifier) {
+      this.peerVerifier = verifier;
       return this;
     }
 
-    public Builder setSslEnginePeerVerifier(SslEnginePeerVerifier peerVerifier) {
-      this.enginePeerVerifier = peerVerifier;
+    public Builder setSslSocketAndEnginePeerVerifier(SslSocketAndEnginePeerVerifier verifier) {
+      this.socketAndEnginePeerVerifier = verifier;
       return this;
     }
 
     // Question: shall we do some sanity checks here, to make sure all three verifiers are set, to
     // be more secure?
     public AdvancedTlsX509TrustManager build() {
       return new AdvancedTlsX509TrustManager(this.verification, this.peerVerifier,
-          this.socketPeerVerifier, this.enginePeerVerifier);
+          this.socketAndEnginePeerVerifier);
     }
   }
 }