Skip to content
Permalink

Comparing changes

This is a direct comparison between two commits made in this repository or its related repositories. View the default comparison for this range or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: opensearch-project/security
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: 74dfc4bff246b729e8b5a25c46119811253940e1
Choose a base ref
..
head repository: opensearch-project/security
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: b5557a6484be622b10d9011940e6b5783dff0244
Choose a head ref
Showing with 195 additions and 110 deletions.
  1. +1 −1 .github/workflows/plugin_install.yml
  2. +6 −6 build.gradle
  3. +19 −0 src/main/java/org/opensearch/security/configuration/SecurityIndexSearcherWrapper.java
  4. +1 −1 src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java
  5. +20 −0 src/main/java/org/opensearch/security/securityconf/ConfigModelV6.java
  6. +18 −0 src/main/java/org/opensearch/security/securityconf/ConfigModelV7.java
  7. +2 −0 src/main/java/org/opensearch/security/securityconf/SecurityRoles.java
  8. +2 −1 src/main/java/org/opensearch/security/ssl/transport/SecuritySSLRequestHandler.java
  9. +0 −2 src/main/java/org/opensearch/security/support/ConfigConstants.java
  10. +35 −0 src/main/java/org/opensearch/security/support/SerializationFormat.java
  11. +18 −12 src/main/java/org/opensearch/security/transport/SecurityInterceptor.java
  12. +0 −48 src/test/java/org/opensearch/security/dlic/rest/api/SecurityHealthActionTest.java
  13. +0 −23 src/test/java/org/opensearch/security/dlic/rest/api/legacy/LegacySecurityHealthActionTests.java
  14. +11 −0 src/test/java/org/opensearch/security/securityconf/SecurityRolesPermissionsV6Test.java
  15. +23 −0 src/test/java/org/opensearch/security/support/Base64HelperTest.java
  16. +2 −1 src/test/java/org/opensearch/security/system_indices/SystemIndexPermissionEnabledTests.java
  17. +15 −13 src/test/java/org/opensearch/security/transport/SecurityInterceptorTests.java
  18. +22 −2 src/test/java/org/opensearch/security/transport/SecuritySSLRequestHandlerTests.java
2 changes: 1 addition & 1 deletion .github/workflows/plugin_install.yml
Original file line number Diff line number Diff line change
@@ -40,7 +40,7 @@ jobs:
shell: bash

- name: Run Opensearch with A Single Plugin
uses: derek-ho/start-opensearch@v3
uses: derek-ho/start-opensearch@v4
with:
opensearch-version: ${{ env.OPENSEARCH_VERSION }}
plugins: "file:$(pwd)/${{ env.PLUGIN_NAME }}.zip"
12 changes: 6 additions & 6 deletions build.gradle
Original file line number Diff line number Diff line change
@@ -64,8 +64,8 @@ plugins {
id 'maven-publish'
id 'com.diffplug.spotless' version '6.25.0'
id 'checkstyle'
id 'com.netflix.nebula.ospackage' version "11.8.1"
id "org.gradle.test-retry" version "1.5.8"
id 'com.netflix.nebula.ospackage' version "11.9.0"
id "org.gradle.test-retry" version "1.5.9"
id 'eclipse'
id "com.github.spotbugs" version "5.2.5"
id "com.google.osdetector" version "1.7.3"
@@ -469,7 +469,7 @@ bundlePlugin {
configurations {
all {
resolutionStrategy {
force 'commons-codec:commons-codec:1.16.1'
force 'commons-codec:commons-codec:1.17.0'
force 'org.slf4j:slf4j-api:1.7.36'
force 'org.scala-lang:scala-library:2.13.13'
force "com.fasterxml.jackson:jackson-bom:${versions.jackson}"
@@ -494,7 +494,7 @@ configurations {
// For integrationTest
force "org.apache.httpcomponents:httpclient:4.5.14"
force "org.apache.httpcomponents:httpcore:4.4.16"
force "com.google.errorprone:error_prone_annotations:2.26.1"
force "com.google.errorprone:error_prone_annotations:2.27.0"
force "org.checkerframework:checker-qual:3.42.0"
force "ch.qos.logback:logback-classic:1.5.6"
}
@@ -603,9 +603,9 @@ dependencies {

runtimeOnly 'com.sun.activation:jakarta.activation:1.2.2'
runtimeOnly 'com.eclipsesource.minimal-json:minimal-json:0.9.5'
runtimeOnly 'commons-codec:commons-codec:1.16.1'
runtimeOnly 'commons-codec:commons-codec:1.17.0'
runtimeOnly 'org.cryptacular:cryptacular:1.2.6'
compileOnly 'com.google.errorprone:error_prone_annotations:2.26.1'
compileOnly 'com.google.errorprone:error_prone_annotations:2.27.0'
runtimeOnly 'com.sun.istack:istack-commons-runtime:4.2.0'
runtimeOnly 'jakarta.xml.bind:jakarta.xml.bind-api:4.0.2'
runtimeOnly 'org.ow2.asm:asm:9.7'
Original file line number Diff line number Diff line change
@@ -41,6 +41,7 @@
import org.opensearch.index.IndexService;
import org.opensearch.security.privileges.PrivilegesEvaluator;
import org.opensearch.security.securityconf.ConfigModel;
import org.opensearch.security.securityconf.SecurityRoles;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.support.HeaderHelper;
import org.opensearch.security.support.WildcardMatcher;
@@ -64,6 +65,8 @@ public class SecurityIndexSearcherWrapper implements CheckedFunction<DirectoryRe
private final Boolean systemIndexEnabled;
private final WildcardMatcher systemIndexMatcher;

private final Boolean systemIndexPermissionEnabled;

// constructor is called per index, so avoid costly operations here
public SecurityIndexSearcherWrapper(
final IndexService indexService,
@@ -91,6 +94,11 @@ public SecurityIndexSearcherWrapper(
ConfigConstants.SECURITY_SYSTEM_INDICES_ENABLED_DEFAULT
);
this.systemIndexMatcher = WildcardMatcher.from(settings.getAsList(ConfigConstants.SECURITY_SYSTEM_INDICES_KEY));

this.systemIndexPermissionEnabled = settings.getAsBoolean(
ConfigConstants.SECURITY_SYSTEM_INDICES_PERMISSIONS_ENABLED_KEY,
ConfigConstants.SECURITY_SYSTEM_INDICES_PERMISSIONS_DEFAULT
);
}

@Subscribe
@@ -144,6 +152,17 @@ protected final boolean isBlockedProtectedIndexRequest() {
}

protected final boolean isBlockedSystemIndexRequest() {
if (systemIndexPermissionEnabled) {
final User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
if (user == null) {
// allow request without user from plugin.
return systemIndexMatcher.test(index.getName());
}
final TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS);
final Set<String> mappedRoles = evaluator.mapRoles(user, caller);
final SecurityRoles securityRoles = evaluator.getSecurityRoles(mappedRoles);
return !securityRoles.isPermittedOnSystemIndex(index.getName());
}
return systemIndexMatcher.test(index.getName());
}

Original file line number Diff line number Diff line change
@@ -193,7 +193,7 @@ public void onDynamicConfigModelChanged(DynamicConfigModel dcm) {
this.dcm = dcm;
}

private SecurityRoles getSecurityRoles(Set<String> roles) {
public SecurityRoles getSecurityRoles(Set<String> roles) {
return configModel.getSecurityRoles().filter(roles);
}

Original file line number Diff line number Diff line change
@@ -542,6 +542,26 @@ public boolean impliesTypePermGlobal(
roles.stream().forEach(p -> ipatterns.addAll(p.getIpatterns()));
return ConfigModelV6.impliesTypePerm(ipatterns, resolved, user, actions, resolver, cs);
}

@Override
public boolean isPermittedOnSystemIndex(String indexName) {
boolean isPatternMatched = false;
boolean isPermitted = false;
for (SecurityRole role : roles) {
for (IndexPattern ip : role.getIpatterns()) {
WildcardMatcher wildcardMatcher = WildcardMatcher.from(ip.indexPattern);
if (wildcardMatcher.test(indexName)) {
isPatternMatched = true;
}
for (TypePerm tp : ip.getTypePerms()) {
if (tp.perms.contains(ConfigConstants.SYSTEM_INDEX_PERMISSION)) {
isPermitted = true;
}
}
}
}
return isPatternMatched && isPermitted;
}
}

public static class SecurityRole {
Original file line number Diff line number Diff line change
@@ -563,6 +563,24 @@ private boolean containsDlsFlsConfig() {

return false;
}

@Override
public boolean isPermittedOnSystemIndex(String indexName) {
boolean isPatternMatched = false;
boolean isPermitted = false;
for (SecurityRole role : roles) {
for (IndexPattern ip : role.getIpatterns()) {
WildcardMatcher wildcardMatcher = WildcardMatcher.from(ip.indexPattern);
if (wildcardMatcher.test(indexName)) {
isPatternMatched = true;
}
if (ip.perms.contains(ConfigConstants.SYSTEM_INDEX_PERMISSION)) {
isPermitted = true;
}
}
}
return isPatternMatched && isPermitted;
}
}

public static class SecurityRole {
Original file line number Diff line number Diff line change
@@ -96,4 +96,6 @@ Set<String> getAllPermittedIndicesForDashboards(
);

SecurityRoles filter(Set<String> roles);

boolean isPermittedOnSystemIndex(String indexName);
}
Original file line number Diff line number Diff line change
@@ -34,6 +34,7 @@
import org.opensearch.security.ssl.util.ExceptionUtils;
import org.opensearch.security.ssl.util.SSLRequestHelper;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.support.SerializationFormat;
import org.opensearch.tasks.Task;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.TransportChannel;
@@ -92,7 +93,7 @@ public final void messageReceived(T request, TransportChannel channel, Task task

threadContext.putTransient(
ConfigConstants.USE_JDK_SERIALIZATION,
channel.getVersion().before(ConfigConstants.FIRST_CUSTOM_SERIALIZATION_SUPPORTED_OS_VERSION)
SerializationFormat.determineFormat(channel.getVersion()) == SerializationFormat.JDK
);

if (SSLRequestHelper.containsBadHeader(threadContext, "_opendistro_security_ssl_")) {
Original file line number Diff line number Diff line change
@@ -35,7 +35,6 @@
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;

import org.opensearch.Version;
import org.opensearch.common.settings.Settings;
import org.opensearch.security.auditlog.impl.AuditCategory;

@@ -334,7 +333,6 @@ public enum RolesMappingResolution {
public static final String TENANCY_GLOBAL_TENANT_DEFAULT_NAME = "";

public static final String USE_JDK_SERIALIZATION = "plugins.security.use_jdk_serialization";
public static final Version FIRST_CUSTOM_SERIALIZATION_SUPPORTED_OS_VERSION = Version.V_2_11_0;

// On-behalf-of endpoints settings
// CS-SUPPRESS-SINGLE: RegexpSingleline get Extensions Settings
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

package org.opensearch.security.support;

import org.opensearch.Version;

public enum SerializationFormat {
/** Uses Java's native serialization system */
JDK,
/** Uses a custom serializer built ontop of OpenSearch 2.11 */
CustomSerializer_2_11;

private static final Version FIRST_CUSTOM_SERIALIZATION_SUPPORTED_OS_VERSION = Version.V_2_11_0;
private static final Version CUSTOM_SERIALIZATION_NO_LONGER_SUPPORTED_OS_VERSION = Version.V_2_14_0;

/**
* Determines the format of serialization that should be used from a version identifier
*/
public static SerializationFormat determineFormat(final Version version) {
if (version.onOrAfter(FIRST_CUSTOM_SERIALIZATION_SUPPORTED_OS_VERSION)
&& version.before(CUSTOM_SERIALIZATION_NO_LONGER_SUPPORTED_OS_VERSION)) {
return SerializationFormat.CustomSerializer_2_11;
}
return SerializationFormat.JDK;
}
}
Original file line number Diff line number Diff line change
@@ -62,6 +62,7 @@
import org.opensearch.security.support.Base64Helper;
import org.opensearch.security.support.ConfigConstants;
import org.opensearch.security.support.HeaderHelper;
import org.opensearch.security.support.SerializationFormat;
import org.opensearch.security.user.User;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.transport.Transport.Connection;
@@ -150,7 +151,8 @@ public <T extends TransportResponse> void sendRequestDecorate(
final String origCCSTransientMf = getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_MASKED_FIELD_CCS);

final boolean isDebugEnabled = log.isDebugEnabled();
final boolean useJDKSerialization = connection.getVersion().before(ConfigConstants.FIRST_CUSTOM_SERIALIZATION_SUPPORTED_OS_VERSION);

final var serializationFormat = SerializationFormat.determineFormat(connection.getVersion());
final boolean isSameNodeRequest = localNode != null && localNode.equals(connection.getNode());

try (ThreadContext.StoredContext stashedContext = getThreadContext().stashContext()) {
@@ -228,25 +230,28 @@ && getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROL
);
}

if (useJDKSerialization) {
Map<String, String> jdkSerializedHeaders = new HashMap<>();
HeaderHelper.getAllSerializedHeaderNames()
.stream()
.filter(k -> headerMap.get(k) != null)
.forEach(k -> jdkSerializedHeaders.put(k, Base64Helper.ensureJDKSerialized(headerMap.get(k))));
headerMap.putAll(jdkSerializedHeaders);
try {
if (serializationFormat == SerializationFormat.JDK) {
Map<String, String> jdkSerializedHeaders = new HashMap<>();
HeaderHelper.getAllSerializedHeaderNames()
.stream()
.filter(k -> headerMap.get(k) != null)
.forEach(k -> jdkSerializedHeaders.put(k, Base64Helper.ensureJDKSerialized(headerMap.get(k))));
headerMap.putAll(jdkSerializedHeaders);
}
getThreadContext().putHeader(headerMap);
} catch (IllegalArgumentException iae) {
log.debug("Failed to add headers information onto on thread context", iae);
}

getThreadContext().putHeader(headerMap);

ensureCorrectHeaders(
remoteAddress0,
user0,
origin0,
injectedUserString,
injectedRolesString,
isSameNodeRequest,
useJDKSerialization
serializationFormat
);

if (actionTraceEnabled.get()) {
@@ -275,7 +280,7 @@ private void ensureCorrectHeaders(
final String injectedUserString,
final String injectedRolesString,
final boolean isSameNodeRequest,
final boolean useJDKSerialization
final SerializationFormat format
) {
// keep original address

@@ -313,6 +318,7 @@ && getThreadContext().getHeader(ConfigConstants.OPENDISTRO_SECURITY_ORIGIN_HEADE
getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER, injectedUserString);
}
} else {
final var useJDKSerialization = format == SerializationFormat.JDK;
if (transportAddress != null) {
getThreadContext().putHeader(
ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS_HEADER,

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -126,6 +126,17 @@ public void hasExplicitIndexPermission() {
);
}

@Test
public void isPermittedOnSystemIndex() {
final SecurityRoles securityRoleWithExplicitAccess = configModel.getSecurityRoles()
.filter(ImmutableSet.of("has_system_index_permission"));
Assert.assertTrue(securityRoleWithExplicitAccess.isPermittedOnSystemIndex(TEST_INDEX));

final SecurityRoles securityRoleWithStarAccess = configModel.getSecurityRoles()
.filter(ImmutableSet.of("all_access_without_system_index_permission"));
Assert.assertFalse(securityRoleWithStarAccess.isPermittedOnSystemIndex(TEST_INDEX));
}

static <T> SecurityDynamicConfiguration<T> createRolesConfig() throws IOException {
final ObjectNode rolesNode = DefaultObjectMapper.objectMapper.createObjectNode();
NO_EXPLICIT_SYSTEM_INDEX_PERMISSION.forEach(rolesNode::set);
Loading