diff --git a/driver/src/main/java/org/neo4j/driver/internal/connector/socket/TrustOnFirstUseTrustManager.java b/driver/src/main/java/org/neo4j/driver/internal/connector/socket/TrustOnFirstUseTrustManager.java index dcafc2b7b5..6d8f9ec323 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/connector/socket/TrustOnFirstUseTrustManager.java +++ b/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 " + diff --git a/driver/src/test/java/org/neo4j/driver/internal/connector/socket/TrustOnFirstUseTrustManagerTest.java b/driver/src/test/java/org/neo4j/driver/internal/connector/socket/TrustOnFirstUseTrustManagerTest.java index 813a8d78ed..f426766410 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/connector/socket/TrustOnFirstUseTrustManagerTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/connector/socket/TrustOnFirstUseTrustManagerTest.java @@ -25,6 +25,7 @@ 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; @@ -32,7 +33,9 @@ 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" ); + } + } }