Skip to content

Commit

Permalink
Merge branch '2.x' into security-subject-2.x-legacy-authz
Browse files Browse the repository at this point in the history
  • Loading branch information
cwperks committed Jan 17, 2025
2 parents 9bdaeee + a7d2c57 commit 9f91927
Show file tree
Hide file tree
Showing 15 changed files with 138 additions and 17 deletions.
2 changes: 1 addition & 1 deletion .github/actions/run-bwc-suite/action.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ runs:
-Dbwc.version.previous=${{ steps.build-previous.outputs.built-version }}
-Dbwc.version.next=${{ steps.build-next.outputs.built-version }} -i
- uses: actions/upload-artifact@v3
- uses: actions/upload-artifact@v4
if: always()
with:
name: ${{ inputs.report-artifact-name }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ jobs:
arguments: |
integrationTest -Dbuild.snapshot=false
- uses: alehechka/upload-tartifact@v2
- uses: actions/upload-artifact@v4
if: always()
with:
name: integration-${{ matrix.platform }}-JDK${{ matrix.jdk }}-reports
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/integration-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:

- run: OPENDISTRO_SECURITY_TEST_OPENSSL_OPT=true ./gradlew test

- uses: actions/upload-artifact@v3
- uses: actions/upload-artifact@v4
if: always()
with:
name: ${{ matrix.jdk }}-${{ matrix.test-run }}-reports
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,28 @@ public void wildcard() throws Exception {
);
}

@Test
public void wildcardByUsername() throws Exception {
SecurityDynamicConfiguration<RoleV7> roles = SecurityDynamicConfiguration.empty(CType.ROLES);

ActionPrivileges subject = new ActionPrivileges(
roles,
FlattenedActionGroups.EMPTY,
null,
Settings.EMPTY,
Map.of("plugin:org.opensearch.sample.SamplePlugin", Set.of("*"))
);

assertThat(
subject.hasClusterPrivilege(ctxByUsername("plugin:org.opensearch.sample.SamplePlugin"), "cluster:whatever"),
isAllowed()
);
assertThat(
subject.hasClusterPrivilege(ctx("plugin:org.opensearch.other.OtherPlugin"), "cluster:whatever"),
isForbidden(missingPrivileges("cluster:whatever"))
);
}

@Test
public void explicit_wellKnown() throws Exception {
SecurityDynamicConfiguration<RoleV7> roles = SecurityDynamicConfiguration.fromYaml("non_explicit_role:\n" + //
Expand Down Expand Up @@ -455,7 +477,8 @@ public IndicesAndAliases(IndexSpec indexSpec, ActionSpec actionSpec, Statefulnes
settings,
WellKnownActions.CLUSTER_ACTIONS,
WellKnownActions.INDEX_ACTIONS,
WellKnownActions.INDEX_ACTIONS
WellKnownActions.INDEX_ACTIONS,
Map.of()
);

if (statefulness == Statefulness.STATEFUL || statefulness == Statefulness.STATEFUL_LIMITED) {
Expand Down Expand Up @@ -1030,4 +1053,19 @@ static PrivilegesEvaluationContext ctx(String... roles) {
null
);
}

static PrivilegesEvaluationContext ctxByUsername(String username) {
User user = new User(username);
user.addAttributes(ImmutableMap.of("attrs.dept_no", "a11"));
return new PrivilegesEvaluationContext(
user,
ImmutableSet.of(),
null,
null,
null,
null,
new IndexNameExpressionResolver(new ThreadContext(Settings.EMPTY)),
null
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

public class IndexDocumentIntoSystemIndexAction extends ActionType<AcknowledgedResponse> {
public static final IndexDocumentIntoSystemIndexAction INSTANCE = new IndexDocumentIntoSystemIndexAction();
public static final String NAME = "mock:systemindex/index";
public static final String NAME = "cluster:mock/systemindex/index";

private IndexDocumentIntoSystemIndexAction() {
super(NAME, AcknowledgedResponse::new);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

public class RunClusterHealthAction extends ActionType<AcknowledgedResponse> {
public static final RunClusterHealthAction INSTANCE = new RunClusterHealthAction();
public static final String NAME = "mock:cluster/monitor/health";
public static final String NAME = "cluster:mock/monitor/health";

private RunClusterHealthAction() {
super(NAME, AcknowledgedResponse::new);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import org.opensearch.SpecialPermission;
import org.opensearch.Version;
import org.opensearch.action.ActionRequest;
import org.opensearch.action.bulk.BulkAction;
import org.opensearch.action.search.PitService;
import org.opensearch.action.search.SearchScrollAction;
import org.opensearch.action.support.ActionFilter;
Expand Down Expand Up @@ -2199,7 +2200,11 @@ public SecurityTokenManager getTokenManager() {

@Override
public PluginSubject getPluginSubject(Plugin plugin) {
return new ContextProvidingPluginSubject(threadPool, settings, plugin);
Set<String> clusterActions = new HashSet<>();
clusterActions.add(BulkAction.NAME);
PluginSubject subject = new ContextProvidingPluginSubject(threadPool, settings, plugin);
sf.updatePluginToClusterActions(subject.getPrincipal().getName(), clusterActions);
return subject;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public boolean authenticate(final SecurityRequestChannel request) {
// PKI authenticated REST call
User superuser = new User(sslPrincipal);
UserSubject subject = new UserSubjectImpl(threadPool, superuser);
threadPool.getThreadContext().putPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER, subject);
threadContext.putPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER, subject);
threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, superuser);
auditLog.logSucceededLogin(sslPrincipal, true, null, request);
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -522,6 +522,15 @@ private boolean checkImmutableIndices(Object request, ActionListener listener) {
return false;
}

public void updatePluginToClusterActions(String pluginIdentifier, Set<String> clusterActions) {
if (evalp instanceof org.opensearch.security.privileges.PrivilegesEvaluatorImpl) {
((org.opensearch.security.privileges.PrivilegesEvaluatorImpl) evalp).updatePluginToClusterActions(
pluginIdentifier,
clusterActions
);
}
}

private boolean isRequestIndexImmutable(Object request) {
final IndexResolverReplacer.Resolved resolved = indexResolverReplacer.resolveRequest(request);
if (resolved.isLocalAll()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@ public class ContextProvidingPluginSubject implements PluginSubject {
public ContextProvidingPluginSubject(ThreadPool threadPool, Settings settings, Plugin plugin) {
super();
this.threadPool = threadPool;
this.pluginPrincipal = new NamedPrincipal(plugin.getClass().getCanonicalName());
String principal = "plugin:" + plugin.getClass().getCanonicalName();
this.pluginPrincipal = new NamedPrincipal(principal);
// Convention for plugin username. Prefixed with 'plugin:'. ':' is forbidden from usernames, so this
// guarantees that a user with this username cannot be created by other means.
this.pluginUser = new User("plugin:" + pluginPrincipal.getName());
this.pluginUser = new User(principal);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,10 @@ public ActionPrivileges(
Settings settings,
ImmutableSet<String> wellKnownClusterActions,
ImmutableSet<String> wellKnownIndexActions,
ImmutableSet<String> explicitlyRequiredIndexActions
ImmutableSet<String> explicitlyRequiredIndexActions,
Map<String, Set<String>> pluginToClusterActions
) {
this.cluster = new ClusterPrivileges(roles, actionGroups, wellKnownClusterActions);
this.cluster = new ClusterPrivileges(roles, actionGroups, wellKnownClusterActions, pluginToClusterActions);
this.index = new IndexPrivileges(roles, actionGroups, wellKnownIndexActions, explicitlyRequiredIndexActions);
this.roles = roles;
this.actionGroups = actionGroups;
Expand All @@ -115,7 +116,27 @@ public ActionPrivileges(
settings,
WellKnownActions.CLUSTER_ACTIONS,
WellKnownActions.INDEX_ACTIONS,
WellKnownActions.EXPLICITLY_REQUIRED_INDEX_ACTIONS
WellKnownActions.EXPLICITLY_REQUIRED_INDEX_ACTIONS,
Map.of()
);
}

public ActionPrivileges(
SecurityDynamicConfiguration<RoleV7> roles,
FlattenedActionGroups actionGroups,
Supplier<Map<String, IndexAbstraction>> indexMetadataSupplier,
Settings settings,
Map<String, Set<String>> pluginToClusterActions
) {
this(
roles,
actionGroups,
indexMetadataSupplier,
settings,
WellKnownActions.CLUSTER_ACTIONS,
WellKnownActions.INDEX_ACTIONS,
WellKnownActions.EXPLICITLY_REQUIRED_INDEX_ACTIONS,
pluginToClusterActions
);
}

Expand Down Expand Up @@ -297,6 +318,8 @@ static class ClusterPrivileges {
*/
private final ImmutableMap<String, WildcardMatcher> rolesToActionMatcher;

private final ImmutableMap<String, WildcardMatcher> usersToActionMatcher;

private final ImmutableSet<String> wellKnownClusterActions;

/**
Expand All @@ -310,14 +333,16 @@ static class ClusterPrivileges {
ClusterPrivileges(
SecurityDynamicConfiguration<RoleV7> roles,
FlattenedActionGroups actionGroups,
ImmutableSet<String> wellKnownClusterActions
ImmutableSet<String> wellKnownClusterActions,
Map<String, Set<String>> pluginToClusterActions
) {
DeduplicatingCompactSubSetBuilder<String> roleSetBuilder = new DeduplicatingCompactSubSetBuilder<>(
roles.getCEntries().keySet()
);
Map<String, DeduplicatingCompactSubSetBuilder.SubSetBuilder<String>> actionToRoles = new HashMap<>();
ImmutableSet.Builder<String> rolesWithWildcardPermissions = ImmutableSet.builder();
ImmutableMap.Builder<String, WildcardMatcher> rolesToActionMatcher = ImmutableMap.builder();
ImmutableMap.Builder<String, WildcardMatcher> usersToActionMatcher = ImmutableMap.builder();

for (Map.Entry<String, RoleV7> entry : roles.getCEntries().entrySet()) {
try {
Expand Down Expand Up @@ -367,13 +392,22 @@ static class ClusterPrivileges {
}
}

if (pluginToClusterActions != null) {
for (String pluginIdentifier : pluginToClusterActions.keySet()) {
Set<String> clusterActions = pluginToClusterActions.get(pluginIdentifier);
WildcardMatcher matcher = WildcardMatcher.from(clusterActions);
usersToActionMatcher.put(pluginIdentifier, matcher);
}
}

DeduplicatingCompactSubSetBuilder.Completed<String> completedRoleSetBuilder = roleSetBuilder.build();

this.actionToRoles = actionToRoles.entrySet()
.stream()
.collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, entry -> entry.getValue().build(completedRoleSetBuilder)));
this.rolesWithWildcardPermissions = rolesWithWildcardPermissions.build();
this.rolesToActionMatcher = rolesToActionMatcher.build();
this.usersToActionMatcher = usersToActionMatcher.build();
this.wellKnownClusterActions = wellKnownClusterActions;
}

Expand Down Expand Up @@ -407,6 +441,14 @@ PrivilegesEvaluatorResponse providesPrivilege(PrivilegesEvaluationContext contex
}
}

// 4: If plugin is performing the action, check if plugin has permission
if (context.getUser().isPluginUser() && this.usersToActionMatcher.containsKey(context.getUser().getName())) {
WildcardMatcher matcher = this.usersToActionMatcher.get(context.getUser().getName());
if (matcher != null && matcher.test(action)) {
return PrivilegesEvaluatorResponse.ok();
}
}

return PrivilegesEvaluatorResponse.insufficient(action);
}

Expand Down Expand Up @@ -476,6 +518,16 @@ PrivilegesEvaluatorResponse providesAnyPrivilege(PrivilegesEvaluationContext con
}
}

// 4: If plugin is performing the action, check if plugin has permission
if (this.usersToActionMatcher.containsKey(context.getUser().getName())) {
WildcardMatcher matcher = this.usersToActionMatcher.get(context.getUser().getName());
for (String action : actions) {
if (matcher != null && matcher.test(action)) {
return PrivilegesEvaluatorResponse.ok();
}
}
}

if (actions.size() == 1) {
return PrivilegesEvaluatorResponse.insufficient(actions.iterator().next());
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
Expand Down Expand Up @@ -142,6 +143,7 @@ public class PrivilegesEvaluatorImpl implements PrivilegesEvaluator {
private final boolean checkSnapshotRestoreWritePrivileges;

private final ClusterInfoHolder clusterInfoHolder;
private final ConfigurationRepository configurationRepository;
private ConfigModel configModel;
private final IndexResolverReplacer irr;
private final SnapshotRestoreEvaluator snapshotRestoreEvaluator;
Expand All @@ -152,6 +154,7 @@ public class PrivilegesEvaluatorImpl implements PrivilegesEvaluator {
private DynamicConfigModel dcm;
private final NamedXContentRegistry namedXContentRegistry;
private final Settings settings;
private final Map<String, Set<String>> pluginToClusterActions;
private final AtomicReference<ActionPrivileges> actionPrivileges = new AtomicReference<>();

public PrivilegesEvaluatorImpl(
Expand All @@ -175,6 +178,7 @@ public PrivilegesEvaluatorImpl(

this.threadContext = threadContext;
this.privilegesInterceptor = privilegesInterceptor;
this.pluginToClusterActions = new HashMap<>();
this.clusterStateSupplier = clusterStateSupplier;
this.settings = settings;

Expand All @@ -191,6 +195,7 @@ public PrivilegesEvaluatorImpl(
termsAggregationEvaluator = new TermsAggregationEvaluator();
pitPrivilegesEvaluator = new PitPrivilegesEvaluator();
this.namedXContentRegistry = namedXContentRegistry;
this.configurationRepository = configurationRepository;

if (configurationRepository != null) {
configurationRepository.subscribeOnChange(configMap -> {
Expand Down Expand Up @@ -231,7 +236,8 @@ void updateConfiguration(
DynamicConfigFactory.addStatics(rolesConfiguration.clone()),
flattenedActionGroups,
() -> clusterStateSupplier.get().metadata().getIndicesLookup(),
settings
settings,
pluginToClusterActions
);
Metadata metadata = clusterStateSupplier.get().metadata();
actionPrivileges.updateStatefulIndexPrivileges(metadata.getIndicesLookup(), metadata.version());
Expand Down Expand Up @@ -843,4 +849,8 @@ private List<String> toString(List<AliasMetadata> aliases) {

return Collections.unmodifiableList(ret);
}

public void updatePluginToClusterActions(String pluginIdentifier, Set<String> clusterActions) {
pluginToClusterActions.put(pluginIdentifier, clusterActions);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ private void evaluateSystemIndicesAccess(
);
}
presponse.allowed = false;
presponse.getMissingPrivileges();
presponse.markComplete();
}
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,9 @@ public class ConfigConstants {
public static final String OPENDISTRO_SECURITY_AUTHENTICATED_USER = OPENDISTRO_SECURITY_CONFIG_PREFIX + "authenticated_user";
public static final String OPENDISTRO_SECURITY_USER_HEADER = OPENDISTRO_SECURITY_CONFIG_PREFIX + "user_header";

// persistent header. This header is set once and cannot be stashed
public static final String OPENDISTRO_SECURITY_AUTHENTICATED_USER = OPENDISTRO_SECURITY_CONFIG_PREFIX + "authenticated_user";

public static final String OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT = OPENDISTRO_SECURITY_CONFIG_PREFIX + "user_info";

public static final String OPENDISTRO_SECURITY_INJECTED_USER = "injected_user";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,13 @@ public void testSecurityUserSubjectRunAs() throws Exception {

final Plugin testPlugin = new TestIdentityAwarePlugin();

final User pluginUser = new User("plugin:" + testPlugin.getClass().getCanonicalName());
final String pluginPrincipal = "plugin:" + testPlugin.getClass().getCanonicalName();

final User pluginUser = new User(pluginPrincipal);

ContextProvidingPluginSubject subject = new ContextProvidingPluginSubject(threadPool, Settings.EMPTY, testPlugin);

assertThat(subject.getPrincipal().getName(), equalTo(testPlugin.getClass().getCanonicalName()));
assertThat(subject.getPrincipal().getName(), equalTo(pluginPrincipal));

assertNull(threadPool.getThreadContext().getTransient(OPENDISTRO_SECURITY_USER));

Expand Down

0 comments on commit 9f91927

Please sign in to comment.