Skip to content

Commit

Permalink
Added configuration to grant full access to procedures
Browse files Browse the repository at this point in the history
  • Loading branch information
OliviaYtterbrink committed Feb 8, 2017
1 parent d5dd203 commit 04a5236
Show file tree
Hide file tree
Showing 14 changed files with 221 additions and 57 deletions.
Expand Up @@ -63,7 +63,7 @@
import org.neo4j.kernel.impl.core.ThreadToStatementContextBridge;
import org.neo4j.kernel.impl.core.TokenNotFoundException;
import org.neo4j.kernel.impl.logging.LogService;
import org.neo4j.kernel.impl.proc.ProcedureAllowedConfig;
import org.neo4j.kernel.impl.proc.ProcedureConfig;
import org.neo4j.kernel.impl.proc.ProcedureGDSFactory;
import org.neo4j.kernel.impl.proc.Procedures;
import org.neo4j.kernel.impl.proc.TerminationGuardProvider;
Expand Down Expand Up @@ -350,7 +350,7 @@ private Procedures setupProcedures( PlatformModule platform, EditionModule editi
Procedures procedures = new Procedures(
new SpecialBuiltInProcedures( Version.getNeo4jVersion(),
platform.databaseInfo.edition.toString() ),
pluginDir, internalLog, new ProcedureAllowedConfig( platform.config ) );
pluginDir, internalLog, new ProcedureConfig( platform.config ) );
platform.life.add( procedures );
platform.dependencies.satisfyDependency( procedures );

Expand Down
Expand Up @@ -25,6 +25,7 @@
import java.util.function.Function;
import java.util.function.Supplier;

import org.neo4j.configuration.Description;
import org.neo4j.configuration.LoadableConfig;
import org.neo4j.graphdb.config.Setting;
import org.neo4j.graphdb.security.URLAccessRule;
Expand Down Expand Up @@ -99,6 +100,10 @@ public static class Configuration implements LoadableConfig
@Internal
public static final Setting<String> editionName =
setting( "unsupported.dbms.edition", Settings.STRING, Edition.unknown.toString() );

@Description("A comma separated list of procedures that are allowed full access to the database, note that this" +
" will enable them to bypass security. Use with care.")
public static final Setting<String> procedure_full_access = setting( "dbms.security.procedures.full_access", Settings.STRING, "" );
}

protected final DatabaseInfo databaseInfo;
Expand Down
Expand Up @@ -26,28 +26,32 @@
import java.util.stream.Stream;

import org.neo4j.kernel.configuration.Config;
import org.neo4j.kernel.impl.factory.GraphDatabaseFacadeFactory.Configuration;

import static java.util.Arrays.stream;

public class ProcedureAllowedConfig
public class ProcedureConfig
{
public static final String PROC_ALLOWED_SETTING_DEFAULT_NAME = "dbms.security.procedures.default_allowed";
public static final String PROC_ALLOWED_SETTING_ROLES = "dbms.security.procedures.roles";

private static final String ROLES_DELIMITER = ",";
private static final String SETTING_DELIMITER = ";";
private static final String MAPPING_DELIMITER = ":";
private static final String PROCEDURE_DELIMITER = ",";

private final String defaultValue;
private final List<ProcMatcher> matchers;
private final List<Pattern> accessPatterns;

private ProcedureAllowedConfig()
private ProcedureConfig()
{
this.defaultValue = "";
this.matchers = Collections.emptyList();
this.accessPatterns = Collections.emptyList();
}

public ProcedureAllowedConfig( Config config )
public ProcedureConfig( Config config )
{
this.defaultValue = config.getValue( PROC_ALLOWED_SETTING_DEFAULT_NAME )
.map( Object::toString )
Expand All @@ -69,6 +73,17 @@ public ProcedureAllowedConfig( Config config )
} )
.collect( Collectors.toList() );
}
String fullAccessProcedures =
config.getValue( Configuration.procedure_full_access.name() ).map( Object::toString ).orElse( "" );
if ( fullAccessProcedures.isEmpty() )
{
this.accessPatterns = Collections.emptyList();
}
else
{
this.accessPatterns = Stream.of( fullAccessProcedures.split( PROCEDURE_DELIMITER ) )
.map( ProcedureConfig::compilePattern ).collect( Collectors.toList() );
}
}

String[] rolesFor( String procedureName )
Expand All @@ -86,12 +101,22 @@ String[] rolesFor( String procedureName )
}
}

boolean fullAccessFor( String procedureName )
{
return accessPatterns.stream().anyMatch( pattern -> pattern.matcher( procedureName ).matches() );
}

private static Pattern compilePattern( String procedure )
{
return Pattern.compile( procedure.trim().replaceAll( "\\.", "\\\\." ).replaceAll( "\\*", ".*" ) );
}

private String[] getDefaultValue()
{
return defaultValue == null || defaultValue.isEmpty() ? new String[0] : new String[]{defaultValue};
}

static final ProcedureAllowedConfig DEFAULT = new ProcedureAllowedConfig();
static final ProcedureConfig DEFAULT = new ProcedureConfig();

private static class ProcMatcher
{
Expand Down
Expand Up @@ -59,11 +59,11 @@ public class Procedures extends LifecycleAdapter

public Procedures()
{
this( new SpecialBuiltInProcedures( "N/A", "N/A" ), null, NullLog.getInstance(), ProcedureAllowedConfig.DEFAULT );
this( new SpecialBuiltInProcedures( "N/A", "N/A" ), null, NullLog.getInstance(), ProcedureConfig.DEFAULT );
}

public Procedures( ThrowingConsumer<Procedures,ProcedureException> builtin, File pluginDir, Log log,
ProcedureAllowedConfig config )
ProcedureConfig config )
{
this.builtin = builtin;
this.pluginDir = pluginDir;
Expand Down
Expand Up @@ -72,10 +72,10 @@ class ReflectiveProcedureCompiler
private final FieldInjections allFieldInjections;
private final Log log;
private final TypeMappers typeMappers;
private final ProcedureAllowedConfig config;
private final ProcedureConfig config;

ReflectiveProcedureCompiler( TypeMappers typeMappers, ComponentRegistry safeComponents,
ComponentRegistry allComponents, Log log, ProcedureAllowedConfig config )
ComponentRegistry allComponents, Log log, ProcedureConfig config )
{
inputSignatureDeterminer = new MethodSignatureCompiler( typeMappers );
outputMappers = new OutputMappers( typeMappers );
Expand Down Expand Up @@ -201,7 +201,7 @@ private ReflectiveProcedure compileProcedure( Class<?> procDefinition, MethodHan
OutputMapper outputMapper = outputMappers.mapper( method );
MethodHandle procedureMethod = lookup.unreflect( method );
List<FieldInjections.FieldSetter> setters;
if ( fullAccess )
if ( fullAccess || config.fullAccessFor( procName.toString() ) )
{
setters = allFieldInjections.setters( procDefinition );
}
Expand Down
Expand Up @@ -36,7 +36,9 @@
import org.neo4j.kernel.api.proc.BasicContext;
import org.neo4j.kernel.api.proc.CallableProcedure;
import org.neo4j.kernel.api.proc.ProcedureSignature;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.logging.NullLog;
import org.neo4j.procedure.Context;
import org.neo4j.procedure.Name;
import org.neo4j.procedure.Procedure;

Expand All @@ -45,9 +47,12 @@
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.junit.Assert.assertThat;
import static org.neo4j.helpers.collection.Iterators.asList;
import static org.neo4j.helpers.collection.MapUtil.genericMap;
import static org.neo4j.kernel.api.proc.Neo4jTypes.NTInteger;
import static org.neo4j.kernel.api.proc.ProcedureSignature.procedureSignature;
import static org.neo4j.kernel.impl.factory.GraphDatabaseFacadeFactory.Configuration.procedure_full_access;

@SuppressWarnings( "WeakerAccess" )
public class ProcedureJarLoaderTest
{
@Rule
Expand All @@ -57,7 +62,7 @@ public class ProcedureJarLoaderTest

private final ProcedureJarLoader jarloader =
new ProcedureJarLoader( new ReflectiveProcedureCompiler( new TypeMappers(), new ComponentRegistry(),
new ComponentRegistry(), NullLog.getInstance(), ProcedureAllowedConfig.DEFAULT ), NullLog.getInstance() );
registryWithUnsafeAPI(), NullLog.getInstance(), procedureConfig() ), NullLog.getInstance() );

@Test
public void shouldLoadProcedureFromJar() throws Throwable
Expand Down Expand Up @@ -199,6 +204,40 @@ public void shouldGiveHelpfulErrorOnGenericStreamProcedure() throws Throwable
jarloader.loadProcedures( jar );
}

@Test
public void shouldGiveHelpfulErrorOnUnsafeRestrictedProcedure() throws Throwable
{
// Given
URL jar = createJarFor( ClassWithUnsafeComponent.class );

// Expect
exception.expect( ProcedureException.class );
exception.expectMessage( "Unable to set up injection for procedure `ClassWithUnsafeComponent`, the field" +
" `api` has type `class org.neo4j.kernel.impl.proc.ProcedureJarLoaderTest$UnsafeAPI` which is not a" +
" known injectable component.");

// When
jarloader.loadProcedures( jar );
}

@Test
public void shouldLoadUnsafeAllowedProcedureFromJar() throws Throwable
{
// Given
URL jar = createJarFor( ClassWithUnsafeConfiguredComponent.class );

// When
List<CallableProcedure> procedures = jarloader.loadProcedures( jar ).procedures();

// Then
List<ProcedureSignature> signatures = procedures.stream().map( CallableProcedure::signature ).collect( toList() );
assertThat( signatures, contains(
procedureSignature( "org","neo4j", "kernel", "impl", "proc", "unsafeFullAccessProcedure" ).out( "someNumber", NTInteger ).build() ));

assertThat( asList( procedures.get( 0 ).apply( new BasicContext(), new Object[0] ) ),
contains( IsEqual.equalTo( new Object[]{7331L} )) );
}

public URL createJarFor( Class<?> ... targets ) throws IOException
{
return new JarBuilder().createJarFor( tmpdir.newFile( new Random().nextInt() + ".jar" ), targets );
Expand Down Expand Up @@ -289,4 +328,50 @@ public Stream<List<Output>> genericStream()
return Stream.of( Collections.singletonList( new Output() ));
}
}

public static class ClassWithUnsafeComponent
{
@Context
public UnsafeAPI api;

@Procedure
public Stream<Output> unsafeProcedure()
{
return Stream.of( new Output( api.getNumber() ) );
}
}

public static class ClassWithUnsafeConfiguredComponent
{
@Context
public UnsafeAPI api;

@Procedure
public Stream<Output> unsafeFullAccessProcedure()
{
return Stream.of( new Output( api.getNumber() ) );
}
}

private static class UnsafeAPI
{
public long getNumber()
{
return 7331;
}
}

private ComponentRegistry registryWithUnsafeAPI()
{
ComponentRegistry allComponents = new ComponentRegistry();
allComponents.register( UnsafeAPI.class, (ctx) -> new UnsafeAPI() );
return allComponents;
}

private ProcedureConfig procedureConfig()
{
Config config = Config.defaults().with(
genericMap( procedure_full_access.name(), "org.neo4j.kernel.impl.proc.unsafeFullAccessProcedure" ) );
return new ProcedureConfig( config );
}
}
Expand Up @@ -69,7 +69,7 @@ public void setUp() throws Exception
{
components = new ComponentRegistry();
procedureCompiler = new ReflectiveProcedureCompiler( new TypeMappers(), components, components,
NullLog.getInstance(), ProcedureAllowedConfig.DEFAULT );
NullLog.getInstance(), ProcedureConfig.DEFAULT );
}

@Test
Expand Down Expand Up @@ -276,7 +276,7 @@ public void shouldSupportProcedureDeprecation() throws Throwable
// Given
Log log = mock(Log.class);
ReflectiveProcedureCompiler procedureCompiler = new ReflectiveProcedureCompiler( new TypeMappers(), components,
components, log, ProcedureAllowedConfig.DEFAULT );
components, log, ProcedureConfig.DEFAULT );

// When
List<CallableProcedure> procs =
Expand Down
Expand Up @@ -216,7 +216,6 @@ public Stream<MyOutputRecord> defaultValues( @Name( value = "a", defaultValue =
private List<CallableProcedure> compile( Class<?> clazz ) throws KernelException
{
return new ReflectiveProcedureCompiler( new TypeMappers(), new ComponentRegistry(), new ComponentRegistry(),
NullLog.getInstance(),
ProcedureAllowedConfig.DEFAULT ).compileProcedure( clazz, Optional.empty(), true );
NullLog.getInstance(), ProcedureConfig.DEFAULT ).compileProcedure( clazz, Optional.empty(), true );
}
}
Expand Up @@ -70,8 +70,7 @@ public void setUp() throws Exception
{
components = new ComponentRegistry();
procedureCompiler = new ReflectiveProcedureCompiler( new TypeMappers(), components, new ComponentRegistry(),
NullLog.getInstance(),
ProcedureAllowedConfig.DEFAULT );
NullLog.getInstance(), ProcedureConfig.DEFAULT );
}

@Test
Expand Down Expand Up @@ -344,8 +343,8 @@ public void shouldSupportFunctionDeprecation() throws Throwable
{
// Given
Log log = mock(Log.class);
ReflectiveProcedureCompiler procedureCompiler = new ReflectiveProcedureCompiler( new TypeMappers(),
components, new ComponentRegistry(), log, ProcedureAllowedConfig.DEFAULT );
ReflectiveProcedureCompiler procedureCompiler = new ReflectiveProcedureCompiler( new TypeMappers(), components,
new ComponentRegistry(), log, ProcedureConfig.DEFAULT );

// When
List<CallableUserAggregationFunction> funcs = procedureCompiler.compileAggregationFunction( FunctionWithDeprecation.class );
Expand Down
Expand Up @@ -66,7 +66,7 @@ public void setUp() throws Exception
{
components = new ComponentRegistry();
procedureCompiler = new ReflectiveProcedureCompiler( new TypeMappers(), components, new ComponentRegistry(),
NullLog.getInstance(), ProcedureAllowedConfig.DEFAULT );
NullLog.getInstance(), ProcedureConfig.DEFAULT );
}

@Test
Expand Down Expand Up @@ -251,7 +251,7 @@ public void shouldSupportFunctionDeprecation() throws Throwable
// Given
Log log = mock(Log.class);
ReflectiveProcedureCompiler procedureCompiler = new ReflectiveProcedureCompiler( new TypeMappers(), components,
new ComponentRegistry(), log, ProcedureAllowedConfig.DEFAULT );
new ComponentRegistry(), log, ProcedureConfig.DEFAULT );

// When
List<CallableUserFunction> funcs = procedureCompiler.compileFunction( FunctionWithDeprecation.class );
Expand Down
Expand Up @@ -184,6 +184,6 @@ private List<CallableProcedure> compile( Class<?> clazz, boolean safe ) throws K
allComponents.register( MyUnsafeAPI.class, (ctx) -> new MyUnsafeAPI() );

return new ReflectiveProcedureCompiler( new TypeMappers(), safeComponents, allComponents, NullLog.getInstance(),
ProcedureAllowedConfig.DEFAULT ).compileProcedure( clazz, Optional.empty(), safe );
ProcedureConfig.DEFAULT ).compileProcedure( clazz, Optional.empty(), safe );
}
}
Expand Up @@ -43,8 +43,8 @@
import static org.neo4j.kernel.configuration.Settings.min;
import static org.neo4j.kernel.configuration.Settings.options;
import static org.neo4j.kernel.configuration.Settings.setting;
import static org.neo4j.kernel.impl.proc.ProcedureAllowedConfig.PROC_ALLOWED_SETTING_DEFAULT_NAME;
import static org.neo4j.kernel.impl.proc.ProcedureAllowedConfig.PROC_ALLOWED_SETTING_ROLES;
import static org.neo4j.kernel.impl.proc.ProcedureConfig.PROC_ALLOWED_SETTING_DEFAULT_NAME;
import static org.neo4j.kernel.impl.proc.ProcedureConfig.PROC_ALLOWED_SETTING_ROLES;

/**
* Settings for security module
Expand Down

0 comments on commit 04a5236

Please sign in to comment.