Merge pull request #216 from zhenlineo/1.0-permission-to-known-host-file

Added permission check to the known_host file

Author: pontusmelke

driver/src/main/java/org/neo4j/driver/internal/connector/socket/TrustOnFirstUseTrustManager.java

@@ -30,9 +30,10 @@
 import java.security.cert.X509Certificate;
 import javax.net.ssl.X509TrustManager;
 
-import org.neo4j.driver.v1.Logger;
 import org.neo4j.driver.internal.util.BytePrinter;
+import org.neo4j.driver.v1.Logger;
 
+import static java.lang.String.format;
 import static org.neo4j.driver.internal.util.CertificateTool.X509CertToString;
 
 /**
@@ -77,6 +78,8 @@ private void load() throws IOException
             return;
         }
 
+        assertKnownHostFileReadable();
+
         BufferedReader reader = new BufferedReader( new FileReader( knownHosts ) );
         String line;
         while ( (line = reader.readLine()) != null )
@@ -107,12 +110,38 @@ private void saveTrustedHost( String fingerprint ) throws IOException
         logger.warn( "Adding %s as known and trusted certificate for %s.", fingerprint, serverId );
         createKnownCertFileIfNotExists();
 
+        assertKnownHostFileWritable();
         BufferedWriter writer = new BufferedWriter( new FileWriter( knownHosts, true ) );
         writer.write( serverId + " " + this.fingerprint );
         writer.newLine();
         writer.close();
     }
 
+
+    private void assertKnownHostFileReadable() throws IOException
+    {
+        if( !knownHosts.canRead() )
+        {
+            throw new IOException( format(
+                    "Failed to load certificates from file %s as you have no read permissions to it.\n" +
+                    "Try configuring the Neo4j driver to use a file system location you do have read permissions to.",
+                    knownHosts.getAbsolutePath()
+            ) );
+        }
+    }
+
+    private void assertKnownHostFileWritable() throws IOException
+    {
+        if( !knownHosts.canWrite() )
+        {
+            throw new IOException( format(
+                    "Failed to write certificates to file %s as you have no write permissions to it.\n" +
+                    "Try configuring the Neo4j driver to use a file system location you do have write permissions to.",
+                    knownHosts.getAbsolutePath()
+            ) );
+        }
+    }
+
     /*
      * Disallow all client connection to this client
      */
@@ -140,7 +169,7 @@ public void checkServerTrusted( X509Certificate[] chain, String authType )
             }
             catch ( IOException e )
             {
-                throw new CertificateException( String.format(
+                throw new CertificateException( format(
                         "Failed to save the server ID and the certificate received from the server to file %s.\n" +
                         "Server ID: %s\nReceived cert:\n%s",
                         knownHosts.getAbsolutePath(), serverId, X509CertToString( cert ) ), e );
@@ -150,7 +179,7 @@ public void checkServerTrusted( X509Certificate[] chain, String authType )
         {
             if ( !this.fingerprint.equals( cert ) )
             {
-                throw new CertificateException( String.format(
+                throw new CertificateException( format(
                         "Unable to connect to neo4j at `%s`, because the certificate the server uses has changed. " +
                         "This is a security feature to protect against man-in-the-middle attacks.\n" +
                         "If you trust the certificate the server uses now, simply remove the line that starts with " +

driver/src/test/java/org/neo4j/driver/internal/connector/socket/TrustOnFirstUseTrustManagerTest.java

@@ -25,14 +25,17 @@
 import org.junit.rules.TemporaryFolder;
 
 import java.io.File;
+import java.io.IOException;
 import java.io.PrintWriter;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
 import java.util.Scanner;
 
 import org.neo4j.driver.v1.Logger;
 
+import static org.hamcrest.CoreMatchers.containsString;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
@@ -141,4 +144,54 @@ private String nextLine( Scanner reader )
         while ( line.trim().startsWith( "#" ) );
         return line;
     }
+
+    @Test
+    public void shouldThrowMeaningfulExceptionIfHasNoReadPermissionToKnownHostFile() throws Throwable
+    {
+        // Given
+        File knownHostFile = mock( File.class );
+        when( knownHostFile.canRead() ).thenReturn( false );
+        when( knownHostFile.exists() ).thenReturn( true );
+
+        // When & Then
+        try
+        {
+            new TrustOnFirstUseTrustManager( null, -1, knownHostFile, null );
+            fail( "Should have failed in load certs" );
+        }
+        catch( IOException e )
+        {
+            assertThat( e.getMessage(), containsString( "you have no read permissions to it" ) );
+        }
+        catch( Exception e )
+        {
+            fail( "Should not get any other error besides no permission to read" );
+        }
+    }
+
+    @Test
+    public void shouldThrowMeaningfulExceptionIfHasNoWritePermissionToKnownHostFile() throws Throwable
+    {
+        // Given
+        File knownHostFile = mock( File.class );
+        when( knownHostFile.exists() ).thenReturn( false /*skip reading*/, true );
+        when( knownHostFile.canWrite() ).thenReturn( false );
+
+        // When & Then
+        try
+        {
+            TrustOnFirstUseTrustManager manager =
+                    new TrustOnFirstUseTrustManager( null, -1, knownHostFile, mock( Logger.class ) );
+            manager.checkServerTrusted( new X509Certificate[]{ knownCertificate}, null );
+            fail( "Should have failed in write to certs" );
+        }
+        catch( CertificateException e )
+        {
+            assertThat( e.getCause().getMessage(), containsString( "you have no write permissions to it" ) );
+        }
+        catch( Exception e )
+        {
+            fail( "Should not get any other error besides no permission to write" );
+        }
+    }
 }