Skip to content

Commit

Permalink
Configurable HPKP in server
Browse files Browse the repository at this point in the history 
The HTTP Public Key Pinning (HPKP) is a security feature that tells
browsers to associate specified public keys with the webpage. Such
webpage is less vulnerable to man-in-the-middle attacks.

This commit adds a setting to configure value of the
`Public-Key-Pins` response header for neo4j server. Setting
can be used to make neo4j browser endpoint send the HPKP header with
specified public key fingerprints, max-age and other directives when
accessed through HTTPS.

See
https://developer.mozilla.org/en-US/docs/Web/HTTP/Public_Key_Pinning
for more details.
  • Loading branch information
@lutovich
lutovich committed May 16, 2018
1 parent cd2a378 commit b75fe12
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 31 deletions.
6 changes: 6 additions & 0 deletions community/server/src/main/java/org/neo4j/server/configuration/ServerSettings.java
View file
Expand Up @@ -192,6 +192,12 @@ private ThirdPartyJaxRsPackage createThirdPartyJaxRsPackage( String packageAndMo
"Value is expected to contain dirictives like 'max-age', 'includeSubDomains' and 'preload'." ) "Value is expected to contain dirictives like 'max-age', 'includeSubDomains' and 'preload'." )
public static final Setting<String> http_strict_transport_security = setting( "dbms.security.http_strict_transport_security", STRING, NO_DEFAULT ); public static final Setting<String> http_strict_transport_security = setting( "dbms.security.http_strict_transport_security", STRING, NO_DEFAULT );


@Description( "Value of the HTTP Public Key Pinning (HPKP) response header. " +
"This header tells browsers about the public keys that belong to a webpage. It is attached to every HTTPS response. " +
"Setting is not set by default so 'Public-Key-Pins' header is not sent. " +
"Value is expected to contain dirictives like 'pin-sha256', 'max-age', 'includeSubDomains' and 'report-uri'." )
public static final Setting<String> http_public_key_pins = setting( "dbms.security.http_public_key_pins", STRING, NO_DEFAULT );

@SuppressWarnings( "unused" ) // accessed from the browser @SuppressWarnings( "unused" ) // accessed from the browser
@Description( "Commands to be run when Neo4j Browser successfully connects to this server. Separate multiple " + @Description( "Commands to be run when Neo4j Browser successfully connects to this server. Separate multiple " +
"commands with semi-colon." ) "commands with semi-colon." )
Expand Down
31 changes: 27 additions & 4 deletions community/server/src/main/java/org/neo4j/server/security/ssl/HttpsRequestCustomizer.java
View file
Expand Up @@ -28,37 +28,60 @@
import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Request;


import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.configuration.Config;
import org.neo4j.server.configuration.ServerSettings;


import static org.eclipse.jetty.http.HttpHeader.STRICT_TRANSPORT_SECURITY; import static org.eclipse.jetty.http.HttpHeader.STRICT_TRANSPORT_SECURITY;
import static org.neo4j.server.configuration.ServerSettings.http_public_key_pins;
import static org.neo4j.server.configuration.ServerSettings.http_strict_transport_security;


public class HttpsRequestCustomizer implements HttpConfiguration.Customizer public class HttpsRequestCustomizer implements HttpConfiguration.Customizer
{ {
public static final String PUBLIC_KEY_PINS_HTTP_HEADER = "Public-Key-Pins";

private final HttpField hstsResponseField; private final HttpField hstsResponseField;
private final HttpField hpkpResponseField;


public HttpsRequestCustomizer( Config config ) public HttpsRequestCustomizer( Config config )
{ {
hstsResponseField = createHstsResponseField( config ); hstsResponseField = createHstsResponseField( config );
hpkpResponseField = createHpkpResponseField( config );
} }


@Override @Override
public void customize( Connector connector, HttpConfiguration channelConfig, Request request ) public void customize( Connector connector, HttpConfiguration channelConfig, Request request )
{ {
request.setScheme( HttpScheme.HTTPS.asString() ); request.setScheme( HttpScheme.HTTPS.asString() );


if ( hstsResponseField != null ) addResponseFieldIfConfigured( request, hstsResponseField );
addResponseFieldIfConfigured( request, hpkpResponseField );
}

private static void addResponseFieldIfConfigured( Request request, HttpField field )
{
if ( field != null )
{ {
request.getResponse().getHttpFields().add( hstsResponseField ); request.getResponse().getHttpFields().add( field );
} }
} }


private static HttpField createHstsResponseField( Config config ) private static HttpField createHstsResponseField( Config config )
{ {
String configuredValue = config.get( ServerSettings.http_strict_transport_security ); String configuredValue = config.get( http_strict_transport_security );
if ( StringUtils.isBlank( configuredValue ) ) if ( StringUtils.isBlank( configuredValue ) )
{ {
return null; return null;
} }
return new PreEncodedHttpField( STRICT_TRANSPORT_SECURITY, configuredValue ); return new PreEncodedHttpField( STRICT_TRANSPORT_SECURITY, configuredValue );
} }

private static HttpField createHpkpResponseField( Config config )
{
String configuredValue = config.get( http_public_key_pins );
if ( StringUtils.isBlank( configuredValue ) )
{
return null;
}
// unable to use PreEncodedHttpField because of a bug, see https://github.com/eclipse/jetty.project/pull/2488
// should be possible to handle HSTS and HPKP field creation after bug is fixed in jetty version we depend on
return new HttpField( PUBLIC_KEY_PINS_HTTP_HEADER, configuredValue );
}
} }
70 changes: 55 additions & 15 deletions community/server/src/test/java/org/neo4j/server/HttpHeadersIT.java
View file
Expand Up @@ -25,17 +25,16 @@
import com.sun.jersey.api.client.config.ClientConfig; import com.sun.jersey.api.client.config.ClientConfig;
import com.sun.jersey.api.client.config.DefaultClientConfig; import com.sun.jersey.api.client.config.DefaultClientConfig;
import com.sun.jersey.client.urlconnection.HTTPSProperties; import com.sun.jersey.client.urlconnection.HTTPSProperties;
import org.eclipse.jetty.http.HttpHeader;
import org.junit.After; import org.junit.After;
import org.junit.Test; import org.junit.Test;


import java.net.URI; import java.net.URI;
import java.util.List; import java.util.List;
import java.util.Map;
import javax.net.ssl.HostnameVerifier; import javax.net.ssl.HostnameVerifier;
import javax.net.ssl.HttpsURLConnection; import javax.net.ssl.HttpsURLConnection;
import javax.net.ssl.SSLContext; import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManager;
import javax.ws.rs.core.MultivaluedMap;


import org.neo4j.server.configuration.ServerSettings; import org.neo4j.server.configuration.ServerSettings;
import org.neo4j.server.helpers.CommunityServerBuilder; import org.neo4j.server.helpers.CommunityServerBuilder;
Expand All @@ -50,9 +49,16 @@
import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull; import static org.junit.Assert.assertNull;
import static org.neo4j.server.helpers.CommunityServerBuilder.serverOnRandomPorts; import static org.neo4j.server.helpers.CommunityServerBuilder.serverOnRandomPorts;
import static org.neo4j.server.security.ssl.HttpsRequestCustomizer.PUBLIC_KEY_PINS_HTTP_HEADER;


public class HttpHeadersIT extends ExclusiveServerTestBase public class HttpHeadersIT extends ExclusiveServerTestBase
{ {
private static final String HSTS_HEADER_VALUE = "max-age=31536000; includeSubDomains; preload";
private static final String HPKP_HEADER_VALUE = "max-age=5184000; " +
"pin-sha256=\"d6qzRu9zOECb90Uez27xWltNsj0e1Md7GkYYkVoZWmM=\"; " +
"pin-sha256=\"E9CZ9INDbd+2eRQozYqqbQ2yXLVKB9+xcprMF+44U1g=\"; " +
"includeSubDomains";

private CommunityNeoServer server; private CommunityNeoServer server;


@After @After
Expand Down Expand Up @@ -81,16 +87,15 @@ public void shouldNotSendJettyVersionWithHttpsResponseHeaders() throws Exception
@Test @Test
public void shouldNotSendHstsHeaderWithHttpResponse() throws Exception public void shouldNotSendHstsHeaderWithHttpResponse() throws Exception
{ {
startServer( "max-age=3600" ); startServer( HSTS_HEADER_VALUE, null );
assertNull( runRequestAndGetHstsHeaderValue( httpUri() ) ); assertNull( runRequestAndGetHstsHeaderValue( httpUri() ) );
} }


@Test @Test
public void shouldSendHstsHeaderWithHttpsResponse() throws Exception public void shouldSendHstsHeaderWithHttpsResponse() throws Exception
{ {
String hstsValue = "max-age=31536000; includeSubDomains"; startServer( HSTS_HEADER_VALUE, null );
startServer( hstsValue ); assertEquals( HSTS_HEADER_VALUE, runRequestAndGetHstsHeaderValue( httpsUri() ) );
assertEquals( hstsValue, runRequestAndGetHstsHeaderValue( httpsUri() ) );
} }


@Test @Test
Expand All @@ -100,18 +105,39 @@ public void shouldNotSendHstsHeaderWithHttpsResponseWhenNotConfigured() throws E
assertNull( runRequestAndGetHstsHeaderValue( httpsUri() ) ); assertNull( runRequestAndGetHstsHeaderValue( httpsUri() ) );
} }


@Test
public void shouldNotSendHpkpHeaderWithHttpResponse() throws Exception
{
startServer( null, HPKP_HEADER_VALUE );
assertNull( runRequestAndGetHpkpHeaderValue( httpUri() ) );
}

@Test
public void shouldSendHpkpHeaderWithHttpsResponse() throws Exception
{
startServer( null, HPKP_HEADER_VALUE );
assertEquals( HPKP_HEADER_VALUE, runRequestAndGetHpkpHeaderValue( httpsUri() ) );
}

@Test
public void shouldNotSendHpkpHeaderWithHttpsResponseWhenNotConfigured() throws Exception
{
startServer();
assertNull( runRequestAndGetHpkpHeaderValue( httpsUri() ) );
}

private void startServer() throws Exception private void startServer() throws Exception
{ {
startServer( null ); startServer( null, null );
} }


private void startServer( String hstsValue ) throws Exception private void startServer( String hstsValue, String hpkpValue ) throws Exception
{ {
server = buildServer( hstsValue ); server = buildServer( hstsValue, hpkpValue );
server.start(); server.start();
} }


private CommunityNeoServer buildServer( String hstsValue ) throws Exception private CommunityNeoServer buildServer( String hstsValue, String hpkpValue ) throws Exception
{ {
CommunityServerBuilder builder = serverOnRandomPorts() CommunityServerBuilder builder = serverOnRandomPorts()
.withHttpsEnabled() .withHttpsEnabled()
Expand All @@ -121,6 +147,10 @@ private CommunityNeoServer buildServer( String hstsValue ) throws Exception
{ {
builder.withProperty( ServerSettings.http_strict_transport_security.name(), hstsValue ); builder.withProperty( ServerSettings.http_strict_transport_security.name(), hstsValue );
} }
if ( hpkpValue != null )
{
builder.withProperty( ServerSettings.http_public_key_pins.name(), hpkpValue );
}


return builder.build(); return builder.build();
} }
Expand All @@ -137,7 +167,7 @@ private URI httpsUri()


private static void testNoJettyVersionInResponseHeaders( URI baseUri ) throws Exception private static void testNoJettyVersionInResponseHeaders( URI baseUri ) throws Exception
{ {
MultivaluedMap<String,String> headers = runRequestAndGetHeaders( baseUri ); Map<String,List<String>> headers = runRequestAndGetHeaders( baseUri );


assertNull( headers.get( SERVER.asString() ) ); // no 'Server' header assertNull( headers.get( SERVER.asString() ) ); // no 'Server' header


Expand All @@ -149,7 +179,17 @@ private static void testNoJettyVersionInResponseHeaders( URI baseUri ) throws Ex


private static String runRequestAndGetHstsHeaderValue( URI baseUri ) throws Exception private static String runRequestAndGetHstsHeaderValue( URI baseUri ) throws Exception
{ {
List<String> values = runRequestAndGetHeaderValues( baseUri, STRICT_TRANSPORT_SECURITY ); return runRequestAndGetHeaderValue( baseUri, STRICT_TRANSPORT_SECURITY.asString() );
}

private static String runRequestAndGetHpkpHeaderValue( URI baseUri ) throws Exception
{
return runRequestAndGetHeaderValue( baseUri, PUBLIC_KEY_PINS_HTTP_HEADER );
}

private static String runRequestAndGetHeaderValue( URI baseUri, String header ) throws Exception
{
List<String> values = runRequestAndGetHeaderValues( baseUri, header );
if ( values.isEmpty() ) if ( values.isEmpty() )
{ {
return null; return null;
Expand All @@ -164,12 +204,12 @@ else if ( values.size() == 1 )
} }
} }


private static List<String> runRequestAndGetHeaderValues( URI baseUri, HttpHeader header ) throws Exception private static List<String> runRequestAndGetHeaderValues( URI baseUri, String header ) throws Exception
{ {
return runRequestAndGetHeaders( baseUri ).getOrDefault( header.asString(), emptyList() ); return runRequestAndGetHeaders( baseUri ).getOrDefault( header, emptyList() );
} }


private static MultivaluedMap<String,String> runRequestAndGetHeaders( URI baseUri ) throws Exception private static Map<String,List<String>> runRequestAndGetHeaders( URI baseUri ) throws Exception
{ {
URI uri = baseUri.resolve( "db/data/transaction/commit" ); URI uri = baseUri.resolve( "db/data/transaction/commit" );
ClientRequest request = createClientRequest( uri ); ClientRequest request = createClientRequest( uri );
Expand Down
76 changes: 64 additions & 12 deletions community/server/src/test/java/org/neo4j/server/security/ssl/HttpsRequestCustomizerTest.java
View file
Expand Up @@ -19,6 +19,7 @@
*/ */
package org.neo4j.server.security.ssl; package org.neo4j.server.security.ssl;


import org.eclipse.jetty.http.HttpFields;
import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.Connector;
import org.eclipse.jetty.server.HttpChannel; import org.eclipse.jetty.server.HttpChannel;
import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConfiguration;
Expand All @@ -28,9 +29,12 @@
import org.eclipse.jetty.server.Response; import org.eclipse.jetty.server.Response;
import org.junit.Test; import org.junit.Test;


import java.util.Map;

import org.neo4j.kernel.configuration.Config; import org.neo4j.kernel.configuration.Config;
import org.neo4j.server.configuration.ServerSettings;


import static java.util.Collections.emptyMap;
import static java.util.Collections.unmodifiableMap;
import static org.eclipse.jetty.http.HttpHeader.STRICT_TRANSPORT_SECURITY; import static org.eclipse.jetty.http.HttpHeader.STRICT_TRANSPORT_SECURITY;
import static org.eclipse.jetty.http.HttpScheme.HTTPS; import static org.eclipse.jetty.http.HttpScheme.HTTPS;
import static org.eclipse.jetty.server.HttpConfiguration.Customizer; import static org.eclipse.jetty.server.HttpConfiguration.Customizer;
Expand All @@ -39,9 +43,18 @@
import static org.mockito.Mockito.mock; import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when; import static org.mockito.Mockito.when;
import static org.neo4j.helpers.collection.MapUtil.stringMap;
import static org.neo4j.server.configuration.ServerSettings.http_public_key_pins;
import static org.neo4j.server.configuration.ServerSettings.http_strict_transport_security;
import static org.neo4j.server.security.ssl.HttpsRequestCustomizer.PUBLIC_KEY_PINS_HTTP_HEADER;


public class HttpsRequestCustomizerTest public class HttpsRequestCustomizerTest
{ {
private static final Map<String,String> settingNameToHttpHeader = unmodifiableMap( stringMap(
http_strict_transport_security.name(), STRICT_TRANSPORT_SECURITY.asString(),
http_public_key_pins.name(), PUBLIC_KEY_PINS_HTTP_HEADER
) );

@Test @Test
public void shouldSetRequestSchemeToHttps() public void shouldSetRequestSchemeToHttps()
{ {
Expand All @@ -56,26 +69,65 @@ public void shouldSetRequestSchemeToHttps()
@Test @Test
public void shouldAddHstsHeaderWhenConfigured() public void shouldAddHstsHeaderWhenConfigured()
{ {
String configuredValue = "max-age=3600; includeSubDomains"; testHeadersPresence( stringMap( http_strict_transport_security.name(), "max-age=3600; includeSubDomains" ) );
Customizer customizer = newCustomizer( configuredValue ); }

@Test
public void shouldNotAddHstsHeaderWhenNotConfigured()
{
testHeaderNotPresentWhenConfigurationMissing( STRICT_TRANSPORT_SECURITY.asString() );
}

@Test
public void shouldAddHpkpHeaderWhenConfigured()
{
testHeadersPresence( stringMap( http_public_key_pins.name(), "pin-sha256=\"cUPcTAZWKaASuYWhhneDttWpY3oBAkE3h2+soZS7sWs=\"; " +
"pin-sha256=\"M8HztCzM3elUxkcjR2S5P4hhyBNf6lHkmjAHKhpGPWE=\"; " +
"max-age=5184000; includeSubDomains;" ) );
}

@Test
public void shouldNotAddHpkpHeaderWhenNotConfigured()
{
testHeaderNotPresentWhenConfigurationMissing( PUBLIC_KEY_PINS_HTTP_HEADER );
}

@Test
public void shouldAddBothHstsAndHpkpHeadersWhenConfigured()
{
testHeadersPresence( stringMap(
http_strict_transport_security.name(), "max-age=31536000; includeSubDomains; preload",
http_public_key_pins.name(), "pin-sha256=\"cUPcTAZWKaASuYWhhneDttWpY3oBAkE3h2+soZS7sWs=\"; " +
"pin-sha256=\"M8HztCzM3elUxkcjR2S5P4hhyBNf6lHkmjAHKhpGPWE=\"; " +
"max-age=5184000; includeSubDomains; " +
"report-uri=\"https://www.example.org/hpkp-report\"" ) );
}

private static void testHeadersPresence( Map<String,String> settingsWithValues )
{
Customizer customizer = newCustomizer( settingsWithValues );
Request request = newRequest(); Request request = newRequest();


customize( customizer, request ); customize( customizer, request );


String receivedValue = request.getResponse().getHttpFields().get( STRICT_TRANSPORT_SECURITY ); HttpFields httpFields = request.getResponse().getHttpFields();
assertEquals( configuredValue, receivedValue ); for ( Map.Entry<String,String> entry : settingsWithValues.entrySet() )
{
String settingName = entry.getKey();
String settingValue = entry.getValue();
String headerName = settingNameToHttpHeader.get( settingName );
assertEquals( settingValue, httpFields.get( headerName ) );
}
} }


@Test private static void testHeaderNotPresentWhenConfigurationMissing( String header )
public void shouldNotAddHstsHeaderWhenNotConfigured()
{ {
Customizer customizer = newCustomizer(); Customizer customizer = newCustomizer();
Request request = newRequest(); Request request = newRequest();


customize( customizer, request ); customize( customizer, request );


String hstsValue = request.getResponse().getHttpFields().get( STRICT_TRANSPORT_SECURITY ); assertNull( request.getResponse().getHttpFields().get( header ) );
assertNull( hstsValue );
} }


private static void customize( Customizer customizer, Request request ) private static void customize( Customizer customizer, Request request )
Expand All @@ -95,12 +147,12 @@ private static Request newRequest()


private static Customizer newCustomizer() private static Customizer newCustomizer()
{ {
return newCustomizer( null ); return newCustomizer( emptyMap() );
} }


private static Customizer newCustomizer( String hstsValue ) private static Customizer newCustomizer( Map<String,String> settingsWithValues )
{ {
Config config = Config.defaults( ServerSettings.http_strict_transport_security, hstsValue ); Config config = Config.defaults( settingsWithValues );
return new HttpsRequestCustomizer( config ); return new HttpsRequestCustomizer( config );
} }
} }

0 comments on commit b75fe12

Please sign in to comment.