Merge remote-tracking branch 'upstream/1.0' into 1.1

Author: pontusmelke

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

@@ -22,12 +22,17 @@
 import org.neo4j.driver.internal.net.BoltServerAddress;
 import org.neo4j.driver.v1.*;
 
+import javax.net.ssl.KeyManager;
+import javax.net.ssl.SSLContext;
 import javax.net.ssl.TrustManager;
 import javax.net.ssl.TrustManagerFactory;
 import java.io.File;
 import java.io.IOException;
 import java.security.GeneralSecurityException;
+import java.security.KeyManagementException;
 import java.security.KeyStore;
+import java.security.KeyStoreException;
+import java.security.NoSuchAlgorithmException;
 
 import static org.neo4j.driver.internal.util.CertificateTool.loadX509Cert;
 
@@ -50,37 +55,48 @@ public static SecurityPlan forSignedCertificates( File certFile )
         // Create TrustManager from TrustedKeyStore
         TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance( "SunX509" );
         trustManagerFactory.init( trustedKeyStore );
-        return new SecurityPlan( true, trustManagerFactory.getTrustManagers() );
+
+        SSLContext sslContext = SSLContext.getInstance( "TLS" );
+        sslContext.init( new KeyManager[0], trustManagerFactory.getTrustManagers(), null );
+
+        return new SecurityPlan( true, sslContext);
+    }
+
+    public static SecurityPlan forSystemCertificates() throws NoSuchAlgorithmException, KeyStoreException
+    {
+        return new SecurityPlan( true, SSLContext.getDefault() );
     }
 
+
     public static SecurityPlan forTrustOnFirstUse( File knownHosts, BoltServerAddress address, Logger logger )
-            throws IOException
+            throws IOException, KeyManagementException, NoSuchAlgorithmException
     {
-        return new SecurityPlan( true, new TrustOnFirstUseTrustManager( address, knownHosts, logger ) );
+        SSLContext sslContext = SSLContext.getInstance( "TLS" );
+        sslContext.init( new KeyManager[0], new TrustManager[]{new TrustOnFirstUseTrustManager( address, knownHosts, logger )}, null );
+
+        return new SecurityPlan( true, sslContext);
     }
 
     public static SecurityPlan insecure()
     {
-        return new SecurityPlan( false );
+        return new SecurityPlan( false, null );
     }
 
     private final boolean requiresEncryption;
-    private final TrustManager[] trustManagers;
+    private final SSLContext sslContext;
 
-    public SecurityPlan( boolean requiresEncryption, TrustManager... trustManagers )
+    private SecurityPlan( boolean requiresEncryption, SSLContext sslContext)
     {
         this.requiresEncryption = requiresEncryption;
-        this.trustManagers = trustManagers;
+        this.sslContext = sslContext;
     }
 
     public boolean requiresEncryption()
     {
         return requiresEncryption;
     }
 
-    public TrustManager[] trustManagers()
-    {
-        return trustManagers;
-    }
+
+    public SSLContext sslContext() {return sslContext;}
 
 }

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

@@ -32,6 +32,9 @@
 import org.neo4j.driver.internal.net.BoltServerAddress;
 import org.neo4j.driver.v1.Logger;
 import org.neo4j.driver.internal.util.BytePrinter;
+import org.neo4j.driver.internal.util.BytePrinter;
+import org.neo4j.driver.v1.Config.TrustStrategy;
+import org.neo4j.driver.v1.Logger;
 import org.neo4j.driver.v1.exceptions.ClientException;
 
 import static javax.net.ssl.SSLEngineResult.HandshakeStatus.FINISHED;
@@ -67,8 +70,7 @@ public class TLSSocketChannel implements ByteChannel
     public TLSSocketChannel( BoltServerAddress address, SecurityPlan securityPlan, ByteChannel channel, Logger logger )
             throws GeneralSecurityException, IOException
     {
-        this(channel, logger, createSSLEngine( address, createSSLContext( securityPlan ) ) );
-
+        this( channel, logger, createSSLEngine( address, securityPlan.sslContext() ) );
     }
 
     public TLSSocketChannel( ByteChannel channel, Logger logger, SSLEngine sslEngine ) throws GeneralSecurityException, IOException
@@ -163,8 +165,8 @@ private HandshakeStatus runDelegatedTasks()
      * To verify if deciphering is done successfully, we could check if any bytes has been read into {@code buffer},
      * as the deciphered bytes will only be saved into {@code buffer} when deciphering is carried out successfully.
      *
-     * @param buffer
-     * @return
+     * @param buffer to read data into.
+     * @return The status of the current handshake.
      * @throws IOException
      */
     private HandshakeStatus unwrap( ByteBuffer buffer ) throws IOException
@@ -261,7 +263,7 @@ private HandshakeStatus unwrap( ByteBuffer buffer ) throws IOException
      * a loop
      *
      * @param buffer contains the bytes to send to channel
-     * @return
+     * @return The status of the current handshake
      * @throws IOException
      */
     private HandshakeStatus wrap( ByteBuffer buffer ) throws IOException
@@ -277,7 +279,7 @@ private HandshakeStatus wrap( ByteBuffer buffer ) throws IOException
         case OK:
             handshakeStatus = runDelegatedTasks();
             cipherOut.flip();
-            while(cipherOut.hasRemaining())
+            while ( cipherOut.hasRemaining() )
             {
                 channel.write( cipherOut );
             }
@@ -287,19 +289,30 @@ private HandshakeStatus wrap( ByteBuffer buffer ) throws IOException
             // Enlarge the buffer and return the old status
             int curNetSize = cipherOut.capacity();
             int netSize = sslEngine.getSession().getPacketBufferSize();
-            if ( curNetSize >= netSize || buffer.capacity() > netSize )
+            if ( netSize > curNetSize )
             {
-                // TODO
-                throw new ClientException(
-                        String.format( "Failed to enlarge network buffer from %s to %s. This is either because the " +
-                                       "new size is however less than the old size, or because the application " +
-                                       "buffer size %s is so big that the application data still cannot fit into the " +
-                                       "new network buffer.", curNetSize, netSize, buffer.capacity() ) );
+                // enlarge the peer application data buffer
+                cipherOut = ByteBuffer.allocate( netSize );
+                logger.debug( "Enlarged network output buffer from %s to %s. " +
+                              "This operation should be a rare operation.", curNetSize, netSize );
+            }
+            else
+            {
+                // flush as much data as possible
+                cipherOut.flip();
+                int written = channel.write( cipherOut );
+                if (written == 0)
+                {
+                    throw new ClientException(
+                            String.format(
+                                    "Failed to enlarge network buffer from %s to %s. This is either because the " +
+                                    "new size is however less than the old size, or because the application " +
+                                    "buffer size %s is so big that the application data still cannot fit into the " +
+                                    "new network buffer.", curNetSize, netSize, buffer.capacity() ) );
+                }
+                cipherOut.compact();
+                logger.debug( "Network output buffer couldn't be enlarged, flushing data to the channel instead." );
             }
-
-            cipherOut = ByteBuffer.allocate( netSize );
-            logger.debug( "Enlarged network output buffer from %s to %s. " +
-                          "This operation should be a rare operation.", curNetSize, netSize );
             break;
         default:
             throw new ClientException( "Got unexpected status " + status );
@@ -320,9 +333,9 @@ private HandshakeStatus wrap( ByteBuffer buffer ) throws IOException
      * After the method call, the new position of {@code from.pos} will be {@code from.pos + p}, and similarly,
      * the new position of {@code to.pos} will be {@code to.pos + p}
      *
-     * @param from
-     * @param to
-     * @return
+     * @param from buffer to copy from
+     * @param to buffer to copy to
+     * @return the number of transferred bytes
      */
     static int bufferCopy( ByteBuffer from, ByteBuffer to )
     {
@@ -339,25 +352,10 @@ static int bufferCopy( ByteBuffer from, ByteBuffer to )
         return maxTransfer;
     }
 
-    /**
-     * Create an SSLContext based on a given SecurityPlan.
-     *
-     * @param securityPlan
-     * @return
-     * @throws GeneralSecurityException
-     * @throws IOException
-     */
-    private static SSLContext createSSLContext( SecurityPlan securityPlan ) throws GeneralSecurityException, IOException
-    {
-        SSLContext sslContext = SSLContext.getInstance( "TLS" );
-        sslContext.init( new KeyManager[0], securityPlan.trustManagers(), null );
-        return sslContext;
-    }
-
     /**
      * Create SSLEngine with the SSLContext just created.
-     * @param address
-     * @param sslContext
+     * @param address the host to connect to
+     * @param sslContext the current ssl context
      */
     private static SSLEngine createSSLEngine( BoltServerAddress address, SSLContext sslContext )
     {
@@ -445,10 +443,10 @@ public void close() throws IOException
             channel.close();
             logger.debug( "~~ [CLOSED SECURE CHANNEL]" );
         }
-        catch(IOException e)
+        catch ( IOException e )
         {
             // Treat this as ok - the connection is closed, even if the TLS session did not exit cleanly.
-            logger.warn( "TLS socket could not be closed cleanly: '"+e.getMessage()+"'", e );
+            logger.warn( "TLS socket could not be closed cleanly: '" + e.getMessage() + "'", e );
         }
     }
 

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

@@ -27,7 +27,7 @@
 import org.neo4j.driver.v1.util.Immutable;
 
 import static java.lang.System.getProperty;
-import static org.neo4j.driver.v1.Config.TrustStrategy.*;
+import static org.neo4j.driver.v1.Config.TrustStrategy.trustOnFirstUse;
 
 /**
  * A configuration class to config driver properties.
@@ -245,7 +245,7 @@ public ConfigBuilder withEncryptionLevel( EncryptionLevel level )
         /**
          * Specify how to determine the authenticity of an encryption certificate provided by the Neo4j instance we are connecting to.
          * This defaults to {@link TrustStrategy#trustOnFirstUse(File)}.
-         * See {@link TrustStrategy#trustSignedBy(File)} for using certificate signatures instead to verify
+         * 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
@@ -296,12 +296,20 @@ public static class TrustStrategy
         public enum Strategy
         {
             TRUST_ON_FIRST_USE,
-            TRUST_SIGNED_CERTIFICATES
+            @Deprecated
+            TRUST_SIGNED_CERTIFICATES,
+            TRUST_CUSTOM_CA_SIGNED_CERTIFICATES,
+            TRUST_SYSTEM_CA_SIGNED_CERTIFICATES
         }
 
         private final Strategy strategy;
         private final File certFile;
 
+        private TrustStrategy( Strategy strategy )
+        {
+            this( strategy, null );
+        }
+
         private TrustStrategy( Strategy strategy, File certFile )
         {
             this.strategy = strategy;
@@ -322,6 +330,15 @@ public File certFile()
             return certFile;
         }
 
+        /**
+         * Use {@link #trustCustomCertificateSignedBy(File)} instead.
+         */
+        @Deprecated
+        public static TrustStrategy trustSignedBy( File certFile )
+        {
+            return new TrustStrategy( Strategy.TRUST_SIGNED_CERTIFICATES, certFile );
+        }
+
         /**
          * 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.
@@ -332,9 +349,14 @@ public File certFile()
          * @param certFile the trusted certificate file
          * @return an authentication config
          */
-        public static TrustStrategy trustSignedBy( File certFile )
+        public static TrustStrategy trustCustomCertificateSignedBy( File certFile )
         {
-            return new TrustStrategy( Strategy.TRUST_SIGNED_CERTIFICATES, certFile );
+            return new TrustStrategy( Strategy.TRUST_CUSTOM_CA_SIGNED_CERTIFICATES, certFile );
+        }
+
+        public static TrustStrategy trustSystemCertifcates()
+        {
+            return new TrustStrategy( Strategy.TRUST_SYSTEM_CA_SIGNED_CERTIFICATES );
         }
 
         /**
@@ -345,7 +367,7 @@ public static TrustStrategy trustSignedBy( File certFile )
          * Each time we reconnect to a known host, we verify that its certificate remains the same, guarding against attackers intercepting our communication.
          * <p>
          * Note that this approach is vulnerable to man-in-the-middle attacks the very first time you connect to a new Neo4j instance.
-         * If you do not trust the network you are connecting over, consider using {@link #trustSignedBy(File) signed certificates} instead, or manually adding the
+         * If you do not trust the network you are connecting over, consider using {@link #trustCustomCertificateSignedBy(File)}  signed certificates} instead, or manually adding the
          * trusted host line into the specified file.
          *
          * @param knownHostsFile a file where known certificates are stored.

driver/src/main/java/org/neo4j/driver/v1/GraphDatabase.java

@@ -30,6 +30,7 @@
 import java.security.GeneralSecurityException;
 
 import static java.lang.String.format;
+import static org.neo4j.driver.internal.security.SecurityPlan.insecure;
 import static org.neo4j.driver.v1.Config.EncryptionLevel.REQUIRED;
 import static org.neo4j.driver.v1.Config.EncryptionLevel.REQUIRED_NON_LOCAL;
 
@@ -186,21 +187,25 @@ private static SecurityPlan createSecurityPlan( BoltServerAddress address, Confi
 
         if ( requiresEncryption )
         {
+            Logger logger = config.logging().getLog( "session" );
             switch ( config.trustStrategy().strategy() )
             {
             case TRUST_SIGNED_CERTIFICATES:
+                logger.warn( "Option `TRUST_SIGNED_CERTIFICATE` has been deprecated and will be removed in a future version " +
+                                                    "of the driver. Please switch to use `TRUST_CUSTOM_CA_SIGNED_CERTIFICATES` instead." );
+                            //intentional fallthrough
+            case TRUST_CUSTOM_CA_SIGNED_CERTIFICATES:
                 return SecurityPlan.forSignedCertificates( config.trustStrategy().certFile() );
             case TRUST_ON_FIRST_USE:
                 return SecurityPlan.forTrustOnFirstUse( config.trustStrategy().certFile(),
-                        address, config.logging().getLog( "session" ) );
+                        address, logger );
             default:
                 throw new ClientException( "Unknown TLS authentication strategy: " + config.trustStrategy().strategy().name() );
             }
         }
         else
         {
-            return new SecurityPlan( false );
+            return insecure();
         }
     }
-
 }

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

@@ -69,13 +69,13 @@ public void shouldChangeToTrustedCert()
     {
         // Given
         File trustedCert = new File( "trusted_cert" );
-        Config config = Config.build().withTrustStrategy( Config.TrustStrategy.trustSignedBy( trustedCert ) ).toConfig();
+        Config config = Config.build().withTrustStrategy( Config.TrustStrategy.trustCustomCertificateSignedBy( trustedCert ) ).toConfig();
 
         // When
         Config.TrustStrategy authConfig = config.trustStrategy();
 
         // Then
-        assertEquals( authConfig.strategy(), Config.TrustStrategy.Strategy.TRUST_SIGNED_CERTIFICATES );
+        assertEquals( authConfig.strategy(), Config.TrustStrategy.Strategy.TRUST_CUSTOM_CA_SIGNED_CERTIFICATES );
         assertEquals( trustedCert.getAbsolutePath(), authConfig.certFile().getAbsolutePath() );
     }
 
@@ -86,7 +86,7 @@ public void shouldConfigureMinIdleTime() throws Throwable
         Config config = Config.build().withSessionLivenessCheckTimeout( 1337 ).toConfig();
 
         // then
-        assertThat( config.idleTimeBeforeConnectionTest(), equalTo( 1337l ) );
+        assertThat( config.idleTimeBeforeConnectionTest(), equalTo( 1337L ) );
     }
 
     public static void deleteDefaultKnownCertFileIfExists()