Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Escape pipe character for injected users #5175

Merged
merged 3 commits into from
Mar 19, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -111,6 +111,7 @@

import static org.opensearch.security.OpenSearchSecurityPlugin.traceAction;
import static org.opensearch.security.support.ConfigConstants.OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT;
import static org.opensearch.security.support.SecurityUtils.escapePipe;

public class PrivilegesEvaluator {

@@ -275,12 +276,14 @@ public boolean isInitialized() {
private void setUserInfoInThreadContext(User user) {
if (threadContext.getTransient(OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT) == null) {
StringJoiner joiner = new StringJoiner("|");
joiner.add(user.getName());
joiner.add(String.join(",", user.getRoles()));
joiner.add(String.join(",", user.getSecurityRoles()));
// Escape any pipe characters in the values before joining
joiner.add(escapePipe(user.getName()));
joiner.add(escapePipe(String.join(",", user.getRoles())));
joiner.add(escapePipe(String.join(",", user.getSecurityRoles())));

String requestedTenant = user.getRequestedTenant();
if (!Strings.isNullOrEmpty(requestedTenant)) {
joiner.add(requestedTenant);
joiner.add(escapePipe(requestedTenant));
}
threadContext.putTransient(OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT, joiner.toString());
}
Original file line number Diff line number Diff line change
@@ -140,4 +140,12 @@ private static String resolveEnvVar(String envVarName, String mode, boolean bc,
return bc ? Hasher.hash(envVarValue.toCharArray(), settings) : envVarValue;
}
}

// Helper method to escape pipe characters
public static String escapePipe(String input) {
if (input == null) {
return "";
}
return input.replace("|", "\\|");
}
}
Original file line number Diff line number Diff line change
@@ -70,4 +70,35 @@ private void checkKeysWithPredicate(Collection<String> keys, String predicateNam
);
});
}

@Test
public void testEscapePipe_NullInput() {
assertThat("Null input should return empty string", SecurityUtils.escapePipe(null), equalTo(""));
}

@Test
public void testEscapePipe_EmptyString() {
assertThat("Empty string should return empty string", SecurityUtils.escapePipe(""), equalTo(""));
}

@Test
public void testEscapePipe_StringWithoutPipe() {
assertThat("String without pipe should remain unchanged", SecurityUtils.escapePipe("normal string"), equalTo("normal string"));
}

@Test
public void testEscapePipe_SinglePipe() {
assertThat("Single pipe should be escaped", SecurityUtils.escapePipe("before|after"), equalTo("before\\|after"));
}

@Test
public void testEscapePipe_MultiplePipes() {
assertThat("Multiple pipes should all be escaped", SecurityUtils.escapePipe("a|b|c"), equalTo("a\\|b\\|c"));
}

@Test
public void testEscapePipe_MultiplePipesConsecutive() {
assertThat("Consecutive pipes should all be escaped", SecurityUtils.escapePipe("|||"), equalTo("\\|\\|\\|"));
}

}