Add support for multiple certificate files (#1153) (#1169)

* Add support for multiple certificate files

This update brings support for specifying multiple certificate files via `TrustStrategy#trustCustomCertificateSignedBy(File...)`. Also, it deprecates `TrustStrategy#certFiles()` that is superseded by `TrustStrategy#certFiles()`.

In addition, it adds support for the following Testkit feature flags:
- `Feature:API:SSLConfig`
- `Detail:DefaultSecurityConfigValueEquality`

* Updated certFiles management

driver/clirr-ignored-differences.xml

@@ -55,4 +55,11 @@
         <method>java.lang.Iterable values()</method>
     </difference>
 
+    <difference>
+        <className>org/neo4j/driver/Config$TrustStrategy</className>
+        <differenceType>7005</differenceType>
+        <method>org.neo4j.driver.Config$TrustStrategy trustCustomCertificateSignedBy(java.io.File)</method>
+        <to>org.neo4j.driver.Config$TrustStrategy trustCustomCertificateSignedBy(java.io.File[])</to>
+    </difference>
+
 </differences>

driver/src/main/java/org/neo4j/driver/Config.java

@@ -21,6 +21,10 @@
 import java.io.File;
 import java.io.Serializable;
 import java.net.InetAddress;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 import java.util.Objects;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
@@ -469,7 +473,7 @@ public ConfigBuilder withoutEncryption()
 
         /**
          * Specify how to determine the authenticity of an encryption certificate provided by the Neo4j instance we are connecting to. This defaults to {@link
-         * TrustStrategy#trustSystemCertificates()}. See {@link TrustStrategy#trustCustomCertificateSignedBy(File)} for using certificate signatures instead to
+         * TrustStrategy#trustSystemCertificates()}. See {@link TrustStrategy#trustCustomCertificateSignedBy(File...)} for using certificate signatures instead to
          * verify trust.
          * <p>
          * This is an important setting to understand, because unless we know that the remote server we have an encrypted connection to is really Neo4j, there
@@ -798,19 +802,20 @@ public enum Strategy
         }
 
         private final Strategy strategy;
-        private final File certFile;
+        private final List<File> certFiles;
         private boolean hostnameVerificationEnabled = true;
         private RevocationStrategy revocationStrategy = RevocationStrategy.NO_CHECKS;
 
         private TrustStrategy( Strategy strategy )
         {
-            this( strategy, null );
+            this( strategy, Collections.emptyList() );
         }
 
-        private TrustStrategy( Strategy strategy, File certFile )
+        private TrustStrategy( Strategy strategy, List<File> certFiles )
         {
+            Objects.requireNonNull( certFiles, "certFiles can't be null" );
             this.strategy = strategy;
-            this.certFile = certFile;
+            this.certFiles = Collections.unmodifiableList( new ArrayList<>( certFiles ) );
         }
 
         /**
@@ -827,10 +832,22 @@ public Strategy strategy()
          * Return the configured certificate file.
          *
          * @return configured certificate or {@code null} if trust strategy does not require a certificate.
+         * @deprecated superseded by {@link TrustStrategy#certFiles()}
          */
+        @Deprecated
         public File certFile()
         {
-            return certFile;
+            return certFiles.isEmpty() ? null : certFiles.get( 0 );
+        }
+
+        /**
+         * Return the configured certificate files.
+         *
+         * @return configured certificate files or empty list if trust strategy does not require certificates.
+         */
+        public List<File> certFiles()
+        {
+            return certFiles;
         }
 
         /**
@@ -866,18 +883,18 @@ public TrustStrategy withoutHostnameVerification()
         }
 
         /**
-         * Only encrypted connections to Neo4j instances with certificates signed by a trusted certificate will be accepted.
-         * The file specified should contain one or more trusted X.509 certificates.
+         * Only encrypted connections to Neo4j instances with certificates signed by a trusted certificate will be accepted. The file(s) specified should
+         * contain one or more trusted X.509 certificates.
          * <p>
-         * The certificate(s) in the file must be encoded using PEM encoding, meaning the certificates in the file should be encoded using Base64,
-         * and each certificate is bounded at the beginning by "-----BEGIN CERTIFICATE-----", and bounded at the end by "-----END CERTIFICATE-----".
+         * The certificate(s) in the file(s) must be encoded using PEM encoding, meaning the certificates in the file(s) should be encoded using Base64, and
+         * each certificate is bounded at the beginning by "-----BEGIN CERTIFICATE-----", and bounded at the end by "-----END CERTIFICATE-----".
          *
-         * @param certFile the trusted certificate file
+         * @param certFiles the trusted certificate files
          * @return an authentication config
          */
-        public static TrustStrategy trustCustomCertificateSignedBy( File certFile )
+        public static TrustStrategy trustCustomCertificateSignedBy( File... certFiles )
         {
-            return new TrustStrategy( Strategy.TRUST_CUSTOM_CA_SIGNED_CERTIFICATES, certFile );
+            return new TrustStrategy( Strategy.TRUST_CUSTOM_CA_SIGNED_CERTIFICATES, Arrays.asList( certFiles ) );
         }
 
         /**

driver/src/main/java/org/neo4j/driver/internal/SecuritySettings.java

@@ -21,7 +21,6 @@
 import java.io.IOException;
 import java.io.Serializable;
 import java.security.GeneralSecurityException;
-import java.util.Objects;
 
 import org.neo4j.driver.Config;
 import org.neo4j.driver.exceptions.ClientException;
@@ -73,7 +72,7 @@ private boolean hasEqualTrustStrategy( SecuritySettings other )
         }
 
         return t1.isHostnameVerificationEnabled() == t2.isHostnameVerificationEnabled() && t1.strategy() == t2.strategy() &&
-                Objects.equals( t1.certFile(), t2.certFile() ) && t1.revocationStrategy() == t2.revocationStrategy();
+               t1.certFiles().equals( t2.certFiles() ) && t1.revocationStrategy() == t2.revocationStrategy();
     }
 
     public SecurityPlan createSecurityPlan( String uriScheme )
@@ -131,7 +130,7 @@ private static SecurityPlan createSecurityPlanImpl( boolean encrypted, Config.Tr
             switch ( trustStrategy.strategy() )
             {
             case TRUST_CUSTOM_CA_SIGNED_CERTIFICATES:
-                return SecurityPlanImpl.forCustomCASignedCertificates( trustStrategy.certFile(), hostnameVerificationEnabled, revocationStrategy );
+                return SecurityPlanImpl.forCustomCASignedCertificates( trustStrategy.certFiles(), hostnameVerificationEnabled, revocationStrategy );
             case TRUST_SYSTEM_CA_SIGNED_CERTIFICATES:
                 return SecurityPlanImpl.forSystemCASignedCertificates( hostnameVerificationEnabled, revocationStrategy );
             case TRUST_ALL_CERTIFICATES:

driver/src/main/java/org/neo4j/driver/internal/security/SecurityPlanImpl.java

@@ -29,6 +29,8 @@
 import java.security.cert.PKIXBuilderParameters;
 import java.security.cert.X509CertSelector;
 import java.security.cert.X509Certificate;
+import java.util.Collections;
+import java.util.List;
 import javax.net.ssl.CertPathTrustManagerParameters;
 import javax.net.ssl.KeyManager;
 import javax.net.ssl.SSLContext;
@@ -55,31 +57,31 @@ public static SecurityPlan forAllCertificates( boolean requiresHostnameVerificat
         return new SecurityPlanImpl( true, sslContext, requiresHostnameVerification, revocationStrategy );
     }
 
-    public static SecurityPlan forCustomCASignedCertificates( File certFile, boolean requiresHostnameVerification,
+    public static SecurityPlan forCustomCASignedCertificates( List<File> certFiles, boolean requiresHostnameVerification,
                                                               RevocationStrategy revocationStrategy )
             throws GeneralSecurityException, IOException
     {
-        SSLContext sslContext = configureSSLContext( certFile, revocationStrategy );
+        SSLContext sslContext = configureSSLContext( certFiles, revocationStrategy );
         return new SecurityPlanImpl( true, sslContext, requiresHostnameVerification, revocationStrategy );
     }
 
     public static SecurityPlan forSystemCASignedCertificates( boolean requiresHostnameVerification, RevocationStrategy revocationStrategy )
             throws GeneralSecurityException, IOException
     {
-        SSLContext sslContext = configureSSLContext( null, revocationStrategy );
+        SSLContext sslContext = configureSSLContext( Collections.emptyList(), revocationStrategy );
         return new SecurityPlanImpl( true, sslContext, requiresHostnameVerification, revocationStrategy );
     }
 
-    private static SSLContext configureSSLContext( File customCertFile, RevocationStrategy revocationStrategy )
+    private static SSLContext configureSSLContext( List<File> customCertFiles, RevocationStrategy revocationStrategy )
             throws GeneralSecurityException, IOException
     {
         KeyStore trustedKeyStore = KeyStore.getInstance( KeyStore.getDefaultType() );
         trustedKeyStore.load( null, null );
 
-        if ( customCertFile != null )
+        if ( !customCertFiles.isEmpty() )
         {
-            // A certificate file is specified so we will load the certificates in the file
-            loadX509Cert( customCertFile, trustedKeyStore );
+            // Certificate files are specified, so we will load the certificates in the file
+            loadX509Cert( customCertFiles, trustedKeyStore );
         }
         else
         {

driver/src/main/java/org/neo4j/driver/internal/util/CertificateTool.java

@@ -32,6 +32,7 @@
 import java.security.cert.CertificateFactory;
 import java.security.cert.X509Certificate;
 import java.util.Base64;
+import java.util.List;
 
 /**
  * A tool used to save, load certs, etc.
@@ -106,35 +107,39 @@ public static void saveX509Cert( Certificate[] certs, File certFile ) throws Gen
     /**
      * Load the certificates written in X.509 format in a file to a key store.
      *
-     * @param certFile
+     * @param certFiles
      * @param keyStore
      * @throws GeneralSecurityException
      * @throws IOException
      */
-    public static void loadX509Cert( File certFile, KeyStore keyStore ) throws GeneralSecurityException, IOException
+    public static void loadX509Cert( List<File> certFiles, KeyStore keyStore ) throws GeneralSecurityException, IOException
     {
-        try ( BufferedInputStream inputStream = new BufferedInputStream( new FileInputStream( certFile ) ) )
+        int certCount = 0; // The files might contain multiple certs
+        for ( File certFile : certFiles )
         {
-            CertificateFactory certFactory = CertificateFactory.getInstance( "X.509" );
-
-            int certCount = 0; // The file might contain multiple certs
-            while ( inputStream.available() > 0 )
+            try ( BufferedInputStream inputStream = new BufferedInputStream( new FileInputStream( certFile ) ) )
             {
-                try
-                {
-                    Certificate cert = certFactory.generateCertificate( inputStream );
-                    certCount++;
-                    loadX509Cert( cert, "neo4j.javadriver.trustedcert." + certCount, keyStore );
-                }
-                catch ( CertificateException e )
+                CertificateFactory certFactory = CertificateFactory.getInstance( "X.509" );
+
+                while ( inputStream.available() > 0 )
                 {
-                    if ( e.getCause() != null && e.getCause().getMessage().equals( "Empty input" ) )
+                    try
+                    {
+                        Certificate cert = certFactory.generateCertificate( inputStream );
+                        certCount++;
+                        loadX509Cert( cert, "neo4j.javadriver.trustedcert." + certCount, keyStore );
+                    }
+                    catch ( CertificateException e )
                     {
-                        // This happens if there is whitespace at the end of the certificate - we load one cert, and then try and load a
-                        // second cert, at which point we fail
-                        return;
+                        if ( e.getCause() != null && e.getCause().getMessage().equals( "Empty input" ) )
+                        {
+                            // This happens if there is whitespace at the end of the certificate - we load one cert, and then try and load a
+                            // second cert, at which point we fail
+                            return;
+                        }
+                        throw new IOException( "Failed to load certificate from `" + certFile.getAbsolutePath() + "`: " + certCount + " : " + e.getMessage(),
+                                               e );
                     }
-                    throw new IOException( "Failed to load certificate from `" + certFile.getAbsolutePath() + "`: " + certCount + " : " + e.getMessage(), e );
                 }
             }
         }

driver/src/test/java/org/neo4j/driver/ConfigTest.java

@@ -80,7 +80,7 @@ void shouldChangeToTrustedCert()
 
         // Then
         assertEquals( authConfig.strategy(), Config.TrustStrategy.Strategy.TRUST_CUSTOM_CA_SIGNED_CERTIFICATES );
-        assertEquals( trustedCert.getAbsolutePath(), authConfig.certFile().getAbsolutePath() );
+        assertEquals( trustedCert.getAbsolutePath(), authConfig.certFiles().get( 0 ).getAbsolutePath() );
     }
 
     @Test
@@ -401,7 +401,7 @@ void shouldSerialize() throws Exception
             assertEquals( config.eventLoopThreads(), verify.eventLoopThreads() );
             assertEquals( config.encrypted(), verify.encrypted() );
             assertEquals( config.trustStrategy().strategy(), verify.trustStrategy().strategy() );
-            assertEquals( config.trustStrategy().certFile(), verify.trustStrategy().certFile() );
+            assertEquals( config.trustStrategy().certFiles(), verify.trustStrategy().certFiles() );
             assertEquals( config.trustStrategy().isHostnameVerificationEnabled(), verify.trustStrategy().isHostnameVerificationEnabled() );
             assertEquals( config.trustStrategy().revocationStrategy(), verify.trustStrategy().revocationStrategy() );
             assertEquals( config.userAgent(), verify.userAgent() );

driver/src/test/java/org/neo4j/driver/util/CertificateUtilTest.java

@@ -24,6 +24,7 @@
 import java.security.KeyStore;
 import java.security.cert.Certificate;
 import java.security.cert.X509Certificate;
+import java.util.Collections;
 import java.util.Enumeration;
 
 import org.neo4j.driver.internal.util.CertificateTool;
@@ -52,7 +53,7 @@ void shouldLoadMultipleCertsIntoKeyStore() throws Throwable
         keyStore.load( null, null );
 
         // When
-        CertificateTool.loadX509Cert( certFile, keyStore );
+        CertificateTool.loadX509Cert( Collections.singletonList( certFile ), keyStore );
 
         // Then
         Enumeration<String> aliases = keyStore.aliases();

testkit-backend/src/main/java/neo4j/org/testkit/backend/messages/requests/GetFeatures.java

@@ -62,7 +62,9 @@ public class GetFeatures implements TestkitRequest
             "Temporary:FullSummary",
             "Temporary:ResultKeys",
             "Temporary:TransactionClose",
-            "Optimization:EagerTransactionBegin"
+            "Optimization:EagerTransactionBegin",
+            "Feature:API:SSLConfig",
+            "Detail:DefaultSecurityConfigValueEquality"
     ) );
 
     private static final Set<String> SYNC_FEATURES = new HashSet<>( Arrays.asList(

testkit-backend/src/main/java/neo4j/org/testkit/backend/messages/requests/NewDriver.java

@@ -31,10 +31,14 @@
 import neo4j.org.testkit.backend.messages.responses.TestkitResponse;
 import reactor.core.publisher.Mono;
 
+import java.io.File;
 import java.net.InetAddress;
 import java.net.URI;
 import java.net.UnknownHostException;
+import java.nio.file.Path;
+import java.nio.file.Paths;
 import java.util.LinkedHashSet;
+import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionStage;
@@ -117,7 +121,8 @@ public TestkitResponse process( TestkitState testkitState )
         Config config = configBuilder.build();
         try
         {
-            driver = driver( URI.create( data.uri ), authToken, config, retrySettings, domainNameResolver, testkitState, id );
+            driver = driver( URI.create( data.uri ), authToken, config, retrySettings, domainNameResolver, configureSecuritySettingsBuilder(), testkitState,
+                             id );
         }
         catch ( RuntimeException e )
         {
@@ -223,11 +228,9 @@ private CompletionStage<TestkitCallbackResult> dispatchTestkitCallback( TestkitS
     }
 
     private org.neo4j.driver.Driver driver( URI uri, AuthToken authToken, Config config, RetrySettings retrySettings, DomainNameResolver domainNameResolver,
-                                            TestkitState testkitState,
-                                            String driverId )
+                                            SecuritySettings.SecuritySettingsBuilder securitySettingsBuilder, TestkitState testkitState, String driverId )
     {
         RoutingSettings routingSettings = RoutingSettings.DEFAULT;
-        SecuritySettings.SecuritySettingsBuilder securitySettingsBuilder = new SecuritySettings.SecuritySettingsBuilder();
         SecuritySettings securitySettings = securitySettingsBuilder.build();
         SecurityPlan securityPlan = securitySettings.createSecurityPlan( uri.getScheme() );
         return new DriverFactoryWithDomainNameResolver( domainNameResolver, testkitState, driverId )
@@ -248,6 +251,41 @@ private Optional<TestkitResponse> handleExceptionAsErrorResponse( TestkitState t
         return response;
     }
 
+    private SecuritySettings.SecuritySettingsBuilder configureSecuritySettingsBuilder()
+    {
+        SecuritySettings.SecuritySettingsBuilder securitySettingsBuilder = new SecuritySettings.SecuritySettingsBuilder();
+        if ( data.encrypted )
+        {
+            securitySettingsBuilder.withEncryption();
+        }
+        else
+        {
+            securitySettingsBuilder.withoutEncryption();
+        }
+
+        if ( data.trustedCertificates != null )
+        {
+            if ( !data.trustedCertificates.isEmpty() )
+            {
+                File[] certs = data.trustedCertificates.stream()
+                                                       .map( cert -> "/usr/local/share/custom-ca-certificates/" + cert )
+                                                       .map( Paths::get )
+                                                       .map( Path::toFile )
+                                                       .toArray( File[]::new );
+                securitySettingsBuilder.withTrustStrategy( Config.TrustStrategy.trustCustomCertificateSignedBy( certs ) );
+            }
+            else
+            {
+                securitySettingsBuilder.withTrustStrategy( Config.TrustStrategy.trustAllCertificates() );
+            }
+        }
+        else
+        {
+            securitySettingsBuilder.withTrustStrategy( Config.TrustStrategy.trustSystemCertificates() );
+        }
+        return securitySettingsBuilder;
+    }
+
     @Setter
     @Getter
     public static class NewDriverBody
@@ -263,6 +301,8 @@ public static class NewDriverBody
         private Long livenessCheckTimeoutMs;
         private Integer maxConnectionPoolSize;
         private Long connectionAcquisitionTimeoutMs;
+        private boolean encrypted;
+        private List<String> trustedCertificates;
     }
 
     @RequiredArgsConstructor