Skip to content

Commit

Permalink
Add config option for default allowed value
Browse files Browse the repository at this point in the history
When procedures and UDFs come from a third party, where modifying source code
is not possible, it was previously not possible to assign an `allowed` role to them.
This commit introduces a new config setting that may be used to set a specific role
to any procedures and UDFs that do not have their `allowed` annotation set.
  • Loading branch information
Mats-SX committed Oct 26, 2016
1 parent 1c10b34 commit adf18dc
Show file tree
Hide file tree
Showing 13 changed files with 193 additions and 20 deletions.
Expand Up @@ -38,15 +38,12 @@
import org.neo4j.kernel.AvailabilityGuard;
import org.neo4j.kernel.DatabaseAvailability;
import org.neo4j.kernel.NeoStoreDataSource;
import org.neo4j.kernel.api.security.SecurityContext;
import org.neo4j.kernel.impl.proc.TerminationGuardProvider;
import org.neo4j.procedure.TerminationGuard;
import org.neo4j.kernel.api.KernelAPI;
import org.neo4j.kernel.api.KernelTransaction;
import org.neo4j.kernel.api.Statement;
import org.neo4j.kernel.api.exceptions.KernelException;
import org.neo4j.kernel.api.legacyindex.AutoIndexing;
import org.neo4j.kernel.api.security.AuthSubject;
import org.neo4j.kernel.api.security.SecurityContext;
import org.neo4j.kernel.builtinprocs.SpecialBuiltInProcedures;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.kernel.guard.Guard;
Expand All @@ -66,8 +63,10 @@
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.ProcedureGDSFactory;
import org.neo4j.kernel.impl.proc.Procedures;
import org.neo4j.kernel.impl.proc.TerminationGuardProvider;
import org.neo4j.kernel.impl.proc.TypeMappers.SimpleConverter;
import org.neo4j.kernel.impl.query.QueryExecutionEngine;
import org.neo4j.kernel.impl.store.StoreId;
Expand All @@ -83,9 +82,10 @@
import org.neo4j.kernel.lifecycle.LifeSupport;
import org.neo4j.kernel.lifecycle.LifecycleAdapter;
import org.neo4j.logging.Log;
import org.neo4j.procedure.TerminationGuard;

import static org.neo4j.kernel.api.proc.Context.SECURITY_CONTEXT;
import static org.neo4j.kernel.api.proc.Context.KERNEL_TRANSACTION;
import static org.neo4j.kernel.api.proc.Context.SECURITY_CONTEXT;
import static org.neo4j.kernel.api.proc.Neo4jTypes.NTGeometry;
import static org.neo4j.kernel.api.proc.Neo4jTypes.NTNode;
import static org.neo4j.kernel.api.proc.Neo4jTypes.NTPath;
Expand Down Expand Up @@ -352,7 +352,7 @@ private Procedures setupProcedures( PlatformModule platform, EditionModule editi
Procedures procedures = new Procedures(
new SpecialBuiltInProcedures( Version.getNeo4jVersion(),
platform.databaseInfo.edition.toString() ),
pluginDir, internalLog );
pluginDir, internalLog, new ProcedureAllowedConfig( platform.config ) );
platform.life.add( procedures );
platform.dependencies.satisfyDependency( procedures );

Expand Down
@@ -0,0 +1,46 @@
/*
* Copyright (c) 2002-2016 "Neo Technology,"
* Network Engine for Objects in Lund AB [http://neotechnology.com]
*
* This file is part of Neo4j.
*
* Neo4j is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/
package org.neo4j.kernel.impl.proc;

import org.neo4j.kernel.configuration.Config;

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

private final String defaultValue;

private ProcedureAllowedConfig()
{
this.defaultValue = "";
}

public ProcedureAllowedConfig( Config config )
{
this.defaultValue = config.getParams().get( PROC_ALLOWED_SETTING_DEFAULT_NAME );
}

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

static final ProcedureAllowedConfig DEFAULT = new ProcedureAllowedConfig();
}
Expand Up @@ -55,15 +55,16 @@ public class Procedures extends LifecycleAdapter

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

public Procedures( ThrowingConsumer<Procedures, ProcedureException> builtin, File pluginDir, Log log )
public Procedures( ThrowingConsumer<Procedures,ProcedureException> builtin, File pluginDir, Log log,
ProcedureAllowedConfig config )
{
this.builtin = builtin;
this.pluginDir = pluginDir;
this.log = log;
this.compiler = new ReflectiveProcedureCompiler(typeMappers, components, log);
this.compiler = new ReflectiveProcedureCompiler( typeMappers, components, log, config );
}

/**
Expand Down
Expand Up @@ -65,14 +65,17 @@ public class ReflectiveProcedureCompiler
private final FieldInjections fieldInjections;
private final Log log;
private final TypeMappers typeMappers;
private final ProcedureAllowedConfig config;

public ReflectiveProcedureCompiler( TypeMappers typeMappers, ComponentRegistry components, Log log )
public ReflectiveProcedureCompiler( TypeMappers typeMappers, ComponentRegistry components, Log log,
ProcedureAllowedConfig config )
{
inputSignatureDeterminer = new MethodSignatureCompiler( typeMappers );
outputMappers = new OutputMappers( typeMappers );
this.fieldInjections = new FieldInjections( components );
this.log = log;
this.typeMappers = typeMappers;
this.config = config;
}

public List<CallableUserFunction> compileFunction( Class<?> fcnDefinition ) throws KernelException
Expand Down Expand Up @@ -174,9 +177,10 @@ private ReflectiveProcedure compileProcedure( Class<?> procDefinition, MethodHan
Optional<String> deprecated = deprecated( method, procedure::deprecatedBy,
"Use of @Procedure(deprecatedBy) without @Deprecated in " + procName );

String[] allowed = procedure.allowed().length == 0 ? config.getDefaultValue() : procedure.allowed();
ProcedureSignature signature =
new ProcedureSignature( procName, inputSignature, outputMapper.signature(),
mode, deprecated, procedure.allowed(), description );
mode, deprecated, allowed, description );

return new ReflectiveProcedure( signature, constructor, procedureMethod, outputMapper, setters );
}
Expand Down Expand Up @@ -207,8 +211,10 @@ private ReflectiveUserFunction compileFunction( Class<?> procDefinition, MethodH
Optional<String> deprecated = deprecated( method, function::deprecatedBy,
"Use of @UserFunction(deprecatedBy) without @Deprecated in " + procName );

String[] allowed = function.allowed().length == 0 ? config.getDefaultValue() : function.allowed();
UserFunctionSignature signature =
new UserFunctionSignature( procName, inputSignature, valueConverter.type(), deprecated, function.allowed(), description );
new UserFunctionSignature( procName, inputSignature, valueConverter.type(), deprecated,
allowed, description );

return new ReflectiveUserFunction( signature, constructor, procedureMethod, valueConverter, setters );
}
Expand Down
Expand Up @@ -45,13 +45,13 @@
import org.neo4j.kernel.api.index.IndexDescriptor;
import org.neo4j.kernel.api.index.InternalIndexState;
import org.neo4j.kernel.api.proc.BasicContext;
import org.neo4j.kernel.api.proc.CallableProcedure;
import org.neo4j.kernel.api.proc.Key;
import org.neo4j.kernel.api.proc.ProcedureSignature;
import org.neo4j.kernel.impl.factory.Edition;
import org.neo4j.kernel.impl.proc.Procedures;
import org.neo4j.kernel.impl.proc.TypeMappers;
import org.neo4j.kernel.internal.GraphDatabaseAPI;
import org.neo4j.kernel.internal.Version;
import org.neo4j.storageengine.api.Token;

import static java.util.Collections.emptyIterator;
Expand Down
Expand Up @@ -57,7 +57,7 @@ public class ProcedureJarLoaderTest

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

@Test
public void shouldLoadProcedureFromJar() throws Throwable
Expand Down
Expand Up @@ -66,7 +66,7 @@ public class ReflectiveProcedureTest
public void setUp() throws Exception
{
components = new ComponentRegistry();
procedureCompiler = new ReflectiveProcedureCompiler( new TypeMappers(), components, NullLog.getInstance() );
procedureCompiler = new ReflectiveProcedureCompiler( new TypeMappers(), components, NullLog.getInstance(), ProcedureAllowedConfig.DEFAULT );
}

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

// When
List<CallableProcedure> procs = procedureCompiler.compileProcedure( ProcedureWithDeprecation.class );
Expand Down
Expand Up @@ -214,6 +214,6 @@ public Stream<MyOutputRecord> defaultValues( @Name( value = "a", defaultValue =

private List<CallableProcedure> compile( Class<?> clazz ) throws KernelException
{
return new ReflectiveProcedureCompiler( new TypeMappers(), new ComponentRegistry(), NullLog.getInstance() ).compileProcedure( clazz );
return new ReflectiveProcedureCompiler( new TypeMappers(), new ComponentRegistry(), NullLog.getInstance(), ProcedureAllowedConfig.DEFAULT ).compileProcedure( clazz );
}
}
Expand Up @@ -65,7 +65,7 @@ public class ReflectiveUserFunctionTest
public void setUp() throws Exception
{
components = new ComponentRegistry();
procedureCompiler = new ReflectiveProcedureCompiler( new TypeMappers(), components, NullLog.getInstance() );
procedureCompiler = new ReflectiveProcedureCompiler( new TypeMappers(), components, NullLog.getInstance(), ProcedureAllowedConfig.DEFAULT );
}

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

// When
List<CallableUserFunction> funcs = procedureCompiler.compileFunction( FunctionWithDeprecation.class );
Expand Down
Expand Up @@ -128,6 +128,6 @@ private List<CallableProcedure> compile( Class<?> clazz ) throws KernelException
{
ComponentRegistry components = new ComponentRegistry();
components.register( MyAwesomeAPI.class, (ctx) -> new MyAwesomeAPI() );
return new ReflectiveProcedureCompiler( new TypeMappers(), components, NullLog.getInstance() ).compileProcedure( clazz );
return new ReflectiveProcedureCompiler( new TypeMappers(), components, NullLog.getInstance(), ProcedureAllowedConfig.DEFAULT ).compileProcedure( clazz );
}
}
Expand Up @@ -42,6 +42,7 @@
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;

/**
* Settings for security module
Expand Down Expand Up @@ -281,4 +282,16 @@ public class SecuritySettings
@Description( "Maximum number of history files for the security log." )
public static final Setting<Integer> store_security_log_max_archives =
setting( "dbms.logs.security.rotation.keep_number", INTEGER, "7", min(1) );

//=========================================================================
// Procedure security settings
//=========================================================================

@Description( "The default role to assign to each procedure and user-defined function with an empty `allowed` " +
"annotation field. This can be used to enable fine grained permission " +
"control over third-party procedures and functions for which modifying source code is not possible. " +
"Procedures with non-empty `allowed` fields will be unaffected by this setting. " +
"If this setting is the empty string (default), procedures will be executed according to the same " +
"security rules as normal Cypher statements." )
public static final Setting<String> default_allowed = setting( PROC_ALLOWED_SETTING_DEFAULT_NAME, STRING, "" );
}
Expand Up @@ -29,6 +29,7 @@

import java.time.OffsetDateTime;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
Expand All @@ -40,12 +41,16 @@
import org.neo4j.graphdb.Result;
import org.neo4j.graphdb.factory.GraphDatabaseSettings;
import org.neo4j.kernel.api.KernelTransaction;
import org.neo4j.kernel.api.proc.ProcedureSignature;
import org.neo4j.kernel.api.proc.QualifiedName;
import org.neo4j.kernel.api.proc.UserFunctionSignature;
import org.neo4j.kernel.enterprise.builtinprocs.QueryId;
import org.neo4j.kernel.impl.proc.Procedures;
import org.neo4j.procedure.Context;
import org.neo4j.procedure.Procedure;
import org.neo4j.procedure.TerminationGuard;
import org.neo4j.server.security.enterprise.auth.plugin.api.PredefinedRoles;
import org.neo4j.server.security.enterprise.configuration.SecuritySettings;
import org.neo4j.test.Barrier;
import org.neo4j.test.DoubleLatch;
import org.neo4j.test.rule.concurrent.ThreadingRule;
Expand All @@ -58,6 +63,8 @@
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.anyOf;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.isA;
Expand Down Expand Up @@ -676,6 +683,93 @@ public void shouldTerminateLongRunningProcedureThatChecksTheGuardRegularlyOnTime
result.close();
}

@Test
public void shouldSetAllowedToConfigSetting() throws Throwable
{
neo.tearDown();
neo = setUpNeoServer( stringMap( SecuritySettings.default_allowed.name(), "nonEmpty" ) );
Procedures procedures = neo
.getLocalGraph()
.getDependencyResolver()
.resolveDependency( Procedures.class );
procedures.registerProcedure( ClassWithProcedures.class );

ProcedureSignature numNodes = procedures.procedure( new QualifiedName( new String[]{"test"}, "numNodes" ) );
assertThat( Arrays.asList( numNodes.allowed() ), containsInAnyOrder( "nonEmpty" ) );

ProcedureSignature allowedRead = procedures.procedure( new QualifiedName( new String[]{"test"}, "allowedReadProcedure" ) );
assertThat( Arrays.asList( allowedRead.allowed() ), containsInAnyOrder( "role1" ) );
}

@Test
public void shouldSetAllowedToDefaultValueAndRunningWorks() throws Throwable
{
neo.tearDown();
neo = setUpNeoServer( stringMap( SecuritySettings.default_allowed.name(), "role1" ) );
reSetUp();

userManager.newRole( "role1", "noneSubject" );
assertSuccess( noneSubject, "CALL test.numNodes", itr -> assertKeyIs( itr, "count", "3" ) );
}

@Test
public void shouldNotSetProcedureAllowedIfSettingNotSet() throws Throwable
{
Procedures procedures = neo
.getLocalGraph()
.getDependencyResolver()
.resolveDependency( Procedures.class );

ProcedureSignature numNodes = procedures.procedure( new QualifiedName( new String[]{"test"}, "numNodes" ) );
assertThat( Arrays.asList( numNodes.allowed() ), empty() );
}

@SuppressWarnings( "OptionalGetWithoutIsPresent" )
@Test
public void shouldSetAllowedToConfigSettingForUDF() throws Throwable
{
neo.tearDown();
neo = setUpNeoServer( stringMap( SecuritySettings.default_allowed.name(), "nonEmpty" ) );
Procedures procedures = neo
.getLocalGraph()
.getDependencyResolver()
.resolveDependency( Procedures.class );
procedures.registerFunction( ClassWithFunctions.class );

UserFunctionSignature funcSig = procedures.function(
new QualifiedName( new String[]{"test"}, "nonAllowedFunc" ) ).get();
assertThat( Arrays.asList( funcSig.allowed() ), containsInAnyOrder( "nonEmpty" ) );

UserFunctionSignature f2 = procedures.function( new QualifiedName( new String[]{"test"}, "allowedFunc" ) ).get();
assertThat( Arrays.asList( f2.allowed() ), containsInAnyOrder( "role1" ) );
}

@Test
public void shouldSetAllowedToDefaultValueAndRunningWorksForUDF() throws Throwable
{
neo.tearDown();
neo = setUpNeoServer( stringMap( SecuritySettings.default_allowed.name(), "role1" ) );
reSetUp();

userManager.newRole( "role1", "noneSubject" );
assertSuccess( neo.login( "noneSubject", "abc" ), "RETURN test.allowedFunc() AS c",
itr -> assertKeyIs( itr, "c", "success for role1" ) );
}

@SuppressWarnings( "OptionalGetWithoutIsPresent" )
@Test
public void shouldNotSetProcedureAllowedIfSettingNotSetForUDF() throws Throwable
{
Procedures procedures = neo
.getLocalGraph()
.getDependencyResolver()
.resolveDependency( Procedures.class );

UserFunctionSignature funcSig = procedures.function(
new QualifiedName( new String[]{"test"}, "nonAllowedFunc" ) ).get();
assertThat( Arrays.asList( funcSig.allowed() ), empty() );
}

@Test
public void shouldHandleWriteAfterAllowedReadProcedureWithAuthDisabled() throws Throwable
{
Expand Down

0 comments on commit adf18dc

Please sign in to comment.