NIFI-7744 Add Support for Unicode in X-ProxiedEntitiesChain (#4664)

- Adds detection and encoding of non-ascii characters to creation of chain
- Adds unit tests that use proxied entities with Unicode
This commit is contained in:
Kevin Doran 2020-11-17 10:23:30 -05:00 committed by GitHub
parent d5dc63ded9
commit a0328ff8d1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 334 additions and 63 deletions

View File

@ -16,8 +16,10 @@
*/
package org.apache.nifi.web.security;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Base64;
import java.util.List;
import java.util.stream.Collectors;
import javax.servlet.http.HttpServletRequest;
@ -29,9 +31,6 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.security.core.AuthenticationException;
/**
*
*/
public class ProxiedEntitiesUtils {
private static final Logger logger = LoggerFactory.getLogger(ProxiedEntitiesUtils.class);
@ -45,59 +44,33 @@ public class ProxiedEntitiesUtils {
private static final String ESCAPED_LT = "\\\\<";
private static final String ANONYMOUS_CHAIN = "<>";
private static final String ANONYMOUS_IDENTITY = "";
/**
* Formats the specified DN to be set as a HTTP header using well known conventions.
* Formats a list of DN/usernames to be set as a HTTP header using well known conventions.
*
* @param dn raw dn
* @return the dn formatted as an HTTP header
* @param proxiedEntities the raw identities (usernames and DNs) to be formatted as a chain
* @return the value to use in the X-ProxiedEntitiesChain header
*/
public static String formatProxyDn(String dn) {
return LT + sanitizeDn(dn) + GT;
public static String getProxiedEntitiesChain(final String... proxiedEntities) {
return getProxiedEntitiesChain(Arrays.asList(proxiedEntities));
}
/**
* If a user provides a DN with the sequence '><', they could escape the tokenization process and impersonate another user.
* <p>
* Example:
* <p>
* Provided DN: {@code jdoe><alopresto} -> {@code <jdoe><alopresto><proxy...>} would allow the user to impersonate jdoe
* Formats a list of DN/usernames to be set as a HTTP header using well known conventions.
*
* @param rawDn the unsanitized DN
* @return the sanitized DN
* @param proxiedEntities the raw identities (usernames and DNs) to be formatted as a chain
* @return the value to use in the X-ProxiedEntitiesChain header
*/
private static String sanitizeDn(String rawDn) {
if (StringUtils.isEmpty(rawDn)) {
return rawDn;
} else {
String sanitizedDn = rawDn.replaceAll(GT, ESCAPED_GT).replaceAll(LT, ESCAPED_LT);
if (!sanitizedDn.equals(rawDn)) {
logger.warn("The provided DN [" + rawDn + "] contained dangerous characters that were escaped to [" + sanitizedDn + "]");
}
return sanitizedDn;
public static String getProxiedEntitiesChain(final List<String> proxiedEntities) {
if (proxiedEntities == null) {
return null;
}
}
/**
* Reconstitutes the original DN from the sanitized version passed in the proxy chain.
* <p>
* Example:
* <p>
* {@code alopresto\>\<proxy1} -> {@code alopresto><proxy1}
*
* @param sanitizedDn the sanitized DN
* @return the original DN
*/
private static String unsanitizeDn(String sanitizedDn) {
if (StringUtils.isEmpty(sanitizedDn)) {
return sanitizedDn;
} else {
String unsanitizedDn = sanitizedDn.replaceAll(ESCAPED_GT, GT).replaceAll(ESCAPED_LT, LT);
if (!unsanitizedDn.equals(sanitizedDn)) {
logger.warn("The provided DN [" + sanitizedDn + "] had been escaped, and was reconstituted to the dangerous DN [" + unsanitizedDn + "]");
}
return unsanitizedDn;
}
final List<String> proxiedEntityChain = proxiedEntities.stream()
.map(org.apache.nifi.registry.security.util.ProxiedEntitiesUtils::formatProxyDn)
.collect(Collectors.toList());
return StringUtils.join(proxiedEntityChain, "");
}
/**
@ -106,28 +79,26 @@ public class ProxiedEntitiesUtils {
* @param rawProxyChain raw chain
* @return tokenized proxy chain
*/
public static List<String> tokenizeProxiedEntitiesChain(String rawProxyChain) {
public static List<String> tokenizeProxiedEntitiesChain(final String rawProxyChain) {
final List<String> proxyChain = new ArrayList<>();
if (!StringUtils.isEmpty(rawProxyChain)) {
// Split the String on the >< token
List<String> elements = Arrays.asList(StringUtils.splitByWholeSeparatorPreserveAllTokens(rawProxyChain, "><"));
// Unsanitize each DN and collect back
elements = elements.stream().map(ProxiedEntitiesUtils::unsanitizeDn).collect(Collectors.toList());
// Remove the leading < from the first element
elements.set(0, elements.get(0).replaceFirst(LT, ""));
// Remove the trailing > from the last element
int last = elements.size() - 1;
String lastElement = elements.get(last);
if (lastElement.endsWith(GT)) {
elements.set(last, lastElement.substring(0, lastElement.length() - 1));
if (!isValidChainFormat(rawProxyChain)) {
throw new IllegalArgumentException("Proxy chain format is not recognized and can not safely be converted to a list.");
}
proxyChain.addAll(elements);
if (rawProxyChain.equals(ANONYMOUS_CHAIN)) {
proxyChain.add(ANONYMOUS_IDENTITY);
} else {
// Split the String on the `><` token, use substring to remove leading `<` and trailing `>`
final String[] elements = StringUtils.splitByWholeSeparatorPreserveAllTokens(
rawProxyChain.substring(1, rawProxyChain.length() - 1), "><");
// Unsanitize each DN and add it to the proxy chain list
Arrays.stream(elements)
.map(ProxiedEntitiesUtils::unsanitizeDn)
.forEach(proxyChain::add);
}
}
return proxyChain;
}
@ -153,7 +124,7 @@ public class ProxiedEntitiesUtils {
* @param request The proxied client request that was successfully authenticated.
* @param response A servlet response to the client containing the successful authentication details.
*/
public static void successfulAuthentication(HttpServletRequest request, HttpServletResponse response) {
public static void successfulAuthentication(final HttpServletRequest request, final HttpServletResponse response) {
if (StringUtils.isNotBlank(request.getHeader(PROXY_ENTITIES_CHAIN))) {
response.setHeader(PROXY_ENTITIES_ACCEPTED, Boolean.TRUE.toString());
}
@ -166,9 +137,168 @@ public class ProxiedEntitiesUtils {
* @param response Servlet response to the client containing the unsuccessful authentication attempt details.
* @param failed The related exception thrown and explanation for the unsuccessful authentication attempt.
*/
public static void unsuccessfulAuthentication(HttpServletRequest request, HttpServletResponse response, AuthenticationException failed) {
public static void unsuccessfulAuthentication(
final HttpServletRequest request,
final HttpServletResponse response,
final AuthenticationException failed) {
if (StringUtils.isNotBlank(request.getHeader(PROXY_ENTITIES_CHAIN))) {
response.setHeader(PROXY_ENTITIES_DETAILS, failed.getMessage());
}
}
/**
* Formats the specified DN to be set as a HTTP header using well known conventions.
*
* @param dn raw dn
* @return the dn formatted as an HTTP header
*/
public static String formatProxyDn(final String dn) {
return LT + sanitizeDn(dn) + GT;
}
/**
* Sanitizes a DN for safe and lossless transmission.
*
* Sanitization requires:
* <ol>
* <li>Encoded so that it can be sent losslessly using US-ASCII (the character set of HTTP Header values)</li>
* <li>Resilient to a DN with the sequence '><' to attempt to escape the tokenization process and impersonate another user.</li>
* </ol>
*
* <p>
* Example:
* <p>
* Provided DN: {@code jdoe><alopresto} -> {@code <jdoe><alopresto><proxy...>} would allow the user to impersonate jdoe
* <p>Алйс
* Provided DN: {@code Алйс} -> {@code <Алйс>} cannot be encoded/decoded as ASCII
*
* @param rawDn the unsanitized DN
* @return the sanitized DN
*/
private static String sanitizeDn(final String rawDn) {
if (StringUtils.isEmpty(rawDn)) {
return rawDn;
} else {
// First, escape any GT [>] or LT [<] characters, which are not safe
final String escapedDn = rawDn.replaceAll(GT, ESCAPED_GT).replaceAll(LT, ESCAPED_LT);
if (!escapedDn.equals(rawDn)) {
logger.warn("The provided DN [" + rawDn + "] contained dangerous characters that were escaped to [" + escapedDn + "]");
}
// Second, check for characters outside US-ASCII.
// This is necessary because X509 Certs can contain international/Unicode characters,
// but this value will be passed in an HTTP Header which must be US-ASCII.
// If non-ascii characters are present, base64 encode the DN and wrap in <angled-brackets>,
// to indicate to the receiving end that the value must be decoded.
// Note: We could have decided to always base64 encode these values,
// not only to avoid the isPureAscii(...) check, but also as a
// method of sanitizing GT [>] or LT [<] chars. However, there
// are advantages to encoding only when necessary, namely:
// 1. Backwards compatibility
// 2. Debugging this X-ProxiedEntitiesChain headers is easier unencoded.
// This algorithm can be revisited as part of the next major version change.
if (isPureAscii(escapedDn)) {
return escapedDn;
} else {
final String encodedDn = base64Encode(escapedDn);
logger.debug("The provided DN [" + rawDn + "] contained non-ASCII characters and was encoded as [" + encodedDn + "]");
return encodedDn;
}
}
}
/**
* Reconstitutes the original DN from the sanitized version passed in the proxy chain.
* <p>
* Example:
* <p>
* {@code alopresto\>\<proxy1} -> {@code alopresto><proxy1}
* <p>
* {@code <0JDQu9C50YE=>} -> {@code Алйс}
*
* @param sanitizedDn the sanitized DN
* @return the original DN
*/
private static String unsanitizeDn(final String sanitizedDn) {
if (StringUtils.isEmpty(sanitizedDn)) {
return sanitizedDn;
} else {
final String decodedDn;
if (isBase64Encoded(sanitizedDn)) {
decodedDn = base64Decode(sanitizedDn);
logger.debug("The provided DN [" + sanitizedDn + "] had been encoded, and was reconstituted to the original DN [" + decodedDn + "]");
} else {
decodedDn = sanitizedDn;
}
final String unsanitizedDn = decodedDn.replaceAll(ESCAPED_GT, GT).replaceAll(ESCAPED_LT, LT);
if (!unsanitizedDn.equals(decodedDn)) {
logger.warn("The provided DN [" + sanitizedDn + "] had been escaped, and was reconstituted to the dangerous DN [" + unsanitizedDn + "]");
}
return unsanitizedDn;
}
}
/**
* Base64 encodes a DN and wraps it in angled brackets to indicate the value is base64 and not a raw DN.
*
* @param rawValue The value to encode
* @return A string containing a wrapped, encoded value.
*/
private static String base64Encode(final String rawValue) {
final String base64String = Base64.getEncoder().encodeToString(rawValue.getBytes(StandardCharsets.UTF_8));
final String wrappedEncodedValue = LT + base64String + GT;
return wrappedEncodedValue;
}
/**
* Performs the reverse of ${@link #base64Encode(String)}.
*
* @param encodedValue the encoded value to decode.
* @return The original, decoded string.
*/
private static String base64Decode(final String encodedValue) {
final String base64String = encodedValue.substring(1, encodedValue.length() - 1);
return new String(Base64.getDecoder().decode(base64String), StandardCharsets.UTF_8);
}
/**
* Check if a String is in the expected format and can be safely tokenized.
*
* @param rawProxiedEntitiesChain the value to check
* @return true if the value is in the valid format to tokenize, false otherwise.
*/
private static boolean isValidChainFormat(final String rawProxiedEntitiesChain) {
return isWrappedInAngleBrackets(rawProxiedEntitiesChain);
}
/**
* Check if a value has been encoded by ${@link #base64Encode(String)}, and therefore needs to be decoded.
*
* @param token the value to check
* @return true if the value is encoded, false otherwise.
*/
private static boolean isBase64Encoded(final String token) {
return isWrappedInAngleBrackets(token);
}
/**
* Check if a string is wrapped with &lt;angle brackets&gt;.
*
* @param string the value to check
* @return true if the value starts with &lt; and ends with &gt; - false otherwise
*/
private static boolean isWrappedInAngleBrackets(final String string) {
return string.startsWith(LT) && string.endsWith(GT);
}
/**
* Check if a string contains only pure ascii characters.
*
* @param stringWithUnknownCharacters - the string to check
* @return true if string can be encoded as ascii. false otherwise.
*/
private static boolean isPureAscii(final String stringWithUnknownCharacters) {
return StandardCharsets.US_ASCII.newEncoder().canEncode(stringWithUnknownCharacters);
}
}

View File

@ -26,6 +26,8 @@ import org.junit.runners.JUnit4
import org.slf4j.Logger
import org.slf4j.LoggerFactory
import java.nio.charset.StandardCharsets
@RunWith(JUnit4.class)
class ProxiedEntitiesUtilsTest {
private static final Logger logger = LoggerFactory.getLogger(ProxiedEntitiesUtils.class)
@ -50,6 +52,12 @@ class ProxiedEntitiesUtilsTest {
final String MALICIOUS_USER_NAME_JOHN_ESCAPED = sanitizeDn(MALICIOUS_USER_NAME_JOHN)
private static final String MALICIOUS_USER_DN_JOHN_ESCAPED = sanitizeDn(MALICIOUS_USER_DN_JOHN)
private static final String UNICODE_DN_1 = "CN=Алйс, OU=Apache NiFi"
private static final String UNICODE_DN_1_ENCODED = "<" + base64Encode(UNICODE_DN_1) + ">"
private static final String UNICODE_DN_2 = "CN=Боб, OU=Apache NiFi"
private static final String UNICODE_DN_2_ENCODED = "<" + base64Encode(UNICODE_DN_2) + ">"
@BeforeClass
static void setUpOnce() throws Exception {
logger.metaClass.methodMissing = { String name, args ->
@ -69,6 +77,10 @@ class ProxiedEntitiesUtilsTest {
dn.replaceAll(/>/, '\\\\>').replaceAll('<', '\\\\<')
}
private static String base64Encode(String dn = "") {
return Base64.getEncoder().encodeToString(dn.getBytes(StandardCharsets.UTF_8))
}
private static String printUnicodeString(final String raw) {
StringBuilder sb = new StringBuilder();
for (int i = 0; i < raw.size(); i++) {
@ -147,6 +159,60 @@ class ProxiedEntitiesUtilsTest {
assert forjohnedProxyDn == EXPECTED_PROXY_DN
}
@Test
void testGetProxiedEntitiesChain() throws Exception {
// Arrange
String[] input = [SAFE_USER_NAME_JOHN, SAFE_USER_DN_PROXY_1, SAFE_USER_DN_PROXY_2]
final String expectedOutput = "<${SAFE_USER_NAME_JOHN}><${SAFE_USER_DN_PROXY_1}><${SAFE_USER_DN_PROXY_2}>"
// Act
def output = ProxiedEntitiesUtils.getProxiedEntitiesChain(input)
// Assert
assert output == expectedOutput
}
@Test
void testGetProxiedEntitiesChainShouldHandleMaliciousInput() throws Exception {
// Arrange
String[] input = [MALICIOUS_USER_DN_JOHN, SAFE_USER_DN_PROXY_1, SAFE_USER_DN_PROXY_2]
final String expectedOutput = "<${sanitizeDn(MALICIOUS_USER_DN_JOHN)}><${SAFE_USER_DN_PROXY_1}><${SAFE_USER_DN_PROXY_2}>"
// Act
def output = ProxiedEntitiesUtils.getProxiedEntitiesChain(input)
// Assert
assert output == expectedOutput
}
@Test
void testGetProxiedEntitiesChainShouldEncodeUnicode() throws Exception {
// Arrange
String[] input = [SAFE_USER_NAME_JOHN, UNICODE_DN_1, UNICODE_DN_2]
final String expectedOutput = "<${SAFE_USER_NAME_JOHN}><${UNICODE_DN_1_ENCODED}><${UNICODE_DN_2_ENCODED}>"
// Act
def output = ProxiedEntitiesUtils.getProxiedEntitiesChain(input)
// Assert
assert output == expectedOutput
}
@Test
void testFormatProxyDnShouldEncodeNonAsciiCharacters() throws Exception {
// Arrange
logger.info(" Provided DN: ${UNICODE_DN_1}")
final String expectedFormattedDn = "<${UNICODE_DN_1_ENCODED}>"
logger.info(" Expected DN: expected")
// Act
String formattedDn = ProxiedEntitiesUtils.formatProxyDn(UNICODE_DN_1)
logger.info("Formatted DN: ${formattedDn}")
// Assert
assert formattedDn == expectedFormattedDn
}
@Test
void testShouldBuildProxyChain() throws Exception {
// Arrange
@ -187,6 +253,20 @@ class ProxiedEntitiesUtilsTest {
assert proxiedEntitiesChain == "<><${SAFE_USER_NAME_PROXY_1}>" as String
}
@Test
void testBuildProxyChainShouldHandleUnicode() throws Exception {
// Arrange
def mockProxy1 = [getIdentity: { -> UNICODE_DN_1 }, getChain: { -> null }, isAnonymous: { -> false}] as NiFiUser
def mockJohn = [getIdentity: { -> SAFE_USER_NAME_JOHN }, getChain: { -> mockProxy1 }, isAnonymous: { -> false}] as NiFiUser
// Act
String proxiedEntitiesChain = ProxiedEntitiesUtils.buildProxiedEntitiesChainString(mockJohn)
logger.info("Proxied entities chain: ${proxiedEntitiesChain}")
// Assert
assert proxiedEntitiesChain == "<${SAFE_USER_NAME_JOHN}><${UNICODE_DN_1_ENCODED}>" as String
}
@Test
void testBuildProxyChainShouldHandleMaliciousUser() throws Exception {
// Arrange
@ -216,6 +296,51 @@ class ProxiedEntitiesUtilsTest {
assert tokenizedNames == NAMES
}
@Test
void testShouldTokenizeAnonymous() throws Exception {
// Arrange
final List NAMES = [""]
final String RAW_PROXY_CHAIN = "<>"
logger.info(" Provided proxy chain: ${RAW_PROXY_CHAIN}")
// Act
def tokenizedNames = ProxiedEntitiesUtils.tokenizeProxiedEntitiesChain(RAW_PROXY_CHAIN)
logger.info("Tokenized proxy chain: ${tokenizedNames}")
// Assert
assert tokenizedNames == NAMES
}
@Test
void testShouldTokenizeDoubleAnonymous() throws Exception {
// Arrange
final List NAMES = ["", ""]
final String RAW_PROXY_CHAIN = "<><>"
logger.info(" Provided proxy chain: ${RAW_PROXY_CHAIN}")
// Act
def tokenizedNames = ProxiedEntitiesUtils.tokenizeProxiedEntitiesChain(RAW_PROXY_CHAIN)
logger.info("Tokenized proxy chain: ${tokenizedNames}")
// Assert
assert tokenizedNames == NAMES
}
@Test
void testShouldTokenizeNestedAnonymous() throws Exception {
// Arrange
final List NAMES = [SAFE_USER_DN_PROXY_1, "", SAFE_USER_DN_PROXY_2]
final String RAW_PROXY_CHAIN = "<${SAFE_USER_DN_PROXY_1}><><${SAFE_USER_DN_PROXY_2}>"
logger.info(" Provided proxy chain: ${RAW_PROXY_CHAIN}")
// Act
def tokenizedNames = ProxiedEntitiesUtils.tokenizeProxiedEntitiesChain(RAW_PROXY_CHAIN)
logger.info("Tokenized proxy chain: ${tokenizedNames}")
// Assert
assert tokenizedNames == NAMES
}
@Test
void testShouldTokenizeProxiedEntitiesChainWithDNs() throws Exception {
// Arrange
@ -262,4 +387,20 @@ class ProxiedEntitiesUtilsTest {
assert tokenizedNames.size() == NAMES.size()
assert !tokenizedNames.contains(SAFE_USER_NAME_JOHN)
}
@Test
void testTokenizeProxiedEntitiesChainShouldDecodeNonAsciiValues() throws Exception {
// Arrange
final String RAW_PROXY_CHAIN = "<${SAFE_USER_NAME_JOHN}><${UNICODE_DN_1_ENCODED}><${UNICODE_DN_2_ENCODED}>"
final List TOKENIZED_NAMES = [SAFE_USER_NAME_JOHN, UNICODE_DN_1, UNICODE_DN_2]
logger.info(" Provided proxy chain: ${RAW_PROXY_CHAIN}")
// Act
def tokenizedNames = ProxiedEntitiesUtils.tokenizeProxiedEntitiesChain(RAW_PROXY_CHAIN)
logger.info("Tokenized proxy chain: ${tokenizedNames.collect { "\"${it}\"" }}")
// Assert
assert tokenizedNames == TOKENIZED_NAMES
assert tokenizedNames.size() == TOKENIZED_NAMES.size()
}
}