NIFI-6868: Ensure that when a property value is obtained by calling ProcessContext.getPropertyValue(...).evaluateAttributeExpressions(...).getValue() that we still retain the appropriate value even if the property references a sensitive parameter

* Added additional unit tests and updated ParameterEvaluator to make sure that it follows the same logic as the ParameterExpression in that sensitive parameters cannot be access from within EL

This closes #3889
Signed-off-by: Rob Fellows <rob.fellows@gmail.com>
This commit is contained in:
Mark Payne 2019-11-14 13:49:20 -05:00 committed by Rob Fellows
parent 55c334feb8
commit ea31c1fcbb
No known key found for this signature in database
GPG Key ID: DC06CD21901CA426
5 changed files with 127 additions and 12 deletions

View File

@ -21,14 +21,24 @@ import org.apache.nifi.parameter.Parameter;
public class ParameterExpression implements Expression {
private final String parameterName;
private final boolean allowSensitiveParameterReference;
public ParameterExpression(final String parameterName) {
public ParameterExpression(final String parameterName, final boolean allowSensitiveParameterReference) {
this.parameterName = parameterName;
this.allowSensitiveParameterReference = allowSensitiveParameterReference;
}
@Override
public String evaluate(final EvaluationContext evaluationContext, final AttributeValueDecorator decorator) {
final Parameter parameter = evaluationContext.getParameter(parameterName);
return parameter == null || parameter.getDescriptor().isSensitive() ? null : parameter.getValue();
if (parameter == null) {
return null;
}
if (!allowSensitiveParameterReference && parameter.getDescriptor().isSensitive()) {
return null;
}
return parameter.getValue();
}
}

View File

@ -332,19 +332,19 @@ public class Query {
if (range.getStart() > lastIndex) {
String substring = unescapeLeadingDollarSigns(query.substring(lastIndex, range.getStart()));
addLiteralsAndParameters(parameterParser, substring, expressions);
addLiteralsAndParameters(parameterParser, substring, expressions, true);
}
addLiteralsAndParameters(parameterParser, treeText, expressions);
addLiteralsAndParameters(parameterParser, treeText, expressions, true);
}
if (escapedRanges.isEmpty()) {
addLiteralsAndParameters(parameterParser, query, expressions);
addLiteralsAndParameters(parameterParser, query, expressions, true);
} else {
final Range lastRange = escapedRanges.get(escapedRanges.size() - 1);
if (lastRange.getEnd() + 1 < query.length()) {
final String treeText = unescapeLeadingDollarSigns(query.substring(lastRange.getEnd() + 1));
addLiteralsAndParameters(parameterParser, treeText, expressions);
addLiteralsAndParameters(parameterParser, treeText, expressions, true);
}
}
@ -375,7 +375,11 @@ public class Query {
substring = unescapeTrailingDollarSigns(substring, false);
}
addLiteralsAndParameters(parameterParser, substring, expressions);
// Do not allow sensitive parameters to be referenced because this is within an actual Expression.
// For example, ${#{sensitiveParam}} is not allowed. However, we do support referencing sensitive parameters
// for the use case of simply #{sensitiveParam} outside of an Expression. In such a case, the PreparedQuery will
// still be used to evaluate this, since all Property Values are evaluated through PreparedQueries.
addLiteralsAndParameters(parameterParser, substring, expressions, false);
}
expressions.add(compiledExpression);
@ -386,7 +390,7 @@ public class Query {
final Range lastRange = ranges.get(ranges.size() - 1);
if (lastRange.getEnd() + 1 < query.length()) {
final String treeText = unescapeLeadingDollarSigns(query.substring(lastRange.getEnd() + 1));
addLiteralsAndParameters(parameterParser, treeText, expressions);
addLiteralsAndParameters(parameterParser, treeText, expressions, false);
}
return new StandardPreparedQuery(expressions);
@ -395,7 +399,7 @@ public class Query {
}
}
private static void addLiteralsAndParameters(final ParameterParser parser, final String input, final List<Expression> expressions) {
private static void addLiteralsAndParameters(final ParameterParser parser, final String input, final List<Expression> expressions, final boolean allowSensitiveParameterReference) {
final ParameterTokenList references = parser.parseTokens(input);
int index = 0;
@ -416,7 +420,7 @@ public class Query {
if (token.isParameterReference()) {
final ParameterReference parameterReference = (ParameterReference) token;
expressions.add(new ParameterExpression(parameterReference.getParameterName()));
expressions.add(new ParameterExpression(parameterReference.getParameterName(), allowSensitiveParameterReference));
} else {
expressions.add(new StringLiteralExpression(token.getValue(ParameterLookup.EMPTY)));
}

View File

@ -33,7 +33,7 @@ public class ParameterEvaluator extends StringEvaluator {
@Override
public QueryResult<String> evaluate(final EvaluationContext evaluationContext) {
final Parameter parameter = evaluationContext.getParameter(parameterName);
return new StringQueryResult(parameter == null ? null : parameter.getValue());
return new StringQueryResult( (parameter == null || parameter.getDescriptor().isSensitive()) ? null : parameter.getValue());
}
@Override

View File

@ -16,13 +16,17 @@
*/
package org.apache.nifi.attribute.expression.language;
import org.apache.nifi.parameter.Parameter;
import org.apache.nifi.parameter.ParameterDescriptor;
import org.apache.nifi.parameter.ParameterLookup;
import org.junit.Ignore;
import org.junit.Test;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.TimeUnit;
@ -236,6 +240,34 @@ public class TestStandardPreparedQuery {
assertEquals("false", prepared.evaluateExpressions(new StandardEvaluationContext(attributes), null));
}
@Test
public void testSensitiveParameter() {
final Map<String, Parameter> parameters = new HashMap<>();
parameters.put("param", new Parameter(new ParameterDescriptor.Builder().name("param").build(), "value"));
parameters.put("sensi", new Parameter(new ParameterDescriptor.Builder().name("sensi").sensitive(true).build(), "secret"));
final ParameterLookup parameterLookup = new ParameterLookup() {
@Override
public Optional<Parameter> getParameter(final String parameterName) {
return Optional.ofNullable(parameters.get(parameterName));
}
@Override
public boolean isEmpty() {
return parameters.isEmpty();
}
};
final String value = Query.prepare("#{param}").evaluateExpressions(new StandardEvaluationContext(Collections.emptyMap(), Collections.emptyMap(), parameterLookup), null);
assertEquals("value", value);
final String secret = Query.prepare("#{sensi}").evaluateExpressions(new StandardEvaluationContext(Collections.emptyMap(), Collections.emptyMap(), parameterLookup), null);
assertEquals("secret", secret);
final String invalid = Query.prepare("${#{sensi}}").evaluateExpressions(new StandardEvaluationContext(Collections.emptyMap(), Collections.emptyMap(), parameterLookup), null);
assertEquals("", invalid);
}
@Test
public void testVariableImpacted() {
final Set<String> attr = new HashSet<>();

View File

@ -16,6 +16,8 @@
*/
package org.apache.nifi.integration.parameters;
import org.apache.nifi.components.PropertyDescriptor;
import org.apache.nifi.components.PropertyValue;
import org.apache.nifi.connectable.Connection;
import org.apache.nifi.controller.ProcessorNode;
import org.apache.nifi.controller.queue.FlowFileQueue;
@ -25,6 +27,7 @@ import org.apache.nifi.integration.processors.GenerateProcessor;
import org.apache.nifi.integration.processors.UpdateAttributeCreateOwnProperty;
import org.apache.nifi.integration.processors.UpdateAttributeNoEL;
import org.apache.nifi.integration.processors.UpdateAttributeWithEL;
import org.apache.nifi.integration.processors.UsernamePasswordProcessor;
import org.apache.nifi.parameter.Parameter;
import org.apache.nifi.parameter.ParameterContext;
import org.apache.nifi.parameter.ParameterDescriptor;
@ -32,7 +35,10 @@ import org.apache.nifi.parameter.ParameterReference;
import org.apache.nifi.parameter.ParameterReferenceManager;
import org.apache.nifi.parameter.StandardParameterContext;
import org.apache.nifi.parameter.StandardParameterReferenceManager;
import org.apache.nifi.processor.ProcessContext;
import org.apache.nifi.processor.Processor;
import org.apache.nifi.processor.StandardProcessContext;
import org.junit.Assert;
import org.junit.Test;
import java.util.Collections;
@ -364,8 +370,71 @@ public class ParametersIT extends FrameworkIntegrationTest {
updateAttribute.setProperties(properties);
referencedParameters = updateAttribute.getReferencedParameterNames();
assertEquals(allParamNames, referencedParameters);
}
@Test
public void testSensitivePropertyReferenceParameterSupportsEL() {
final ProcessorNode usernamePassword = createProcessorNode(UsernamePasswordProcessor.class);
final ParameterReferenceManager referenceManager = new StandardParameterReferenceManager(getFlowController().getFlowManager());
final ParameterContext parameterContext = new StandardParameterContext(UUID.randomUUID().toString(), "param-context", referenceManager, null);
parameterContext.setParameters(Collections.singletonMap("pass", new Parameter(new ParameterDescriptor.Builder().name("pass").sensitive(true).build(), "secret")));
getRootGroup().setParameterContext(parameterContext);
final Map<String, String> properties = new HashMap<>();
properties.put("password", "#{pass}");
usernamePassword.setProperties(properties);
final ProcessContext processContext = new StandardProcessContext(usernamePassword, getFlowController().getControllerServiceProvider(), getFlowController().getEncryptor(),
getFlowController().getStateManagerProvider().getStateManager(usernamePassword.getIdentifier()), () -> false);
final PropertyDescriptor descriptor = usernamePassword.getPropertyDescriptor("password");
final PropertyValue propertyValue = processContext.getProperty(descriptor);
final PropertyValue evaluatedPropertyValue = propertyValue.evaluateAttributeExpressions();
final String evaluatedPassword = evaluatedPropertyValue.getValue();
assertEquals("secret", evaluatedPassword);
}
@Test
public void testSensitivePropertyNotAccessibleFromWithinEL() {
final ProcessorNode usernamePassword = createProcessorNode(UsernamePasswordProcessor.class);
final ParameterReferenceManager referenceManager = new StandardParameterReferenceManager(getFlowController().getFlowManager());
final ParameterContext parameterContext = new StandardParameterContext(UUID.randomUUID().toString(), "param-context", referenceManager, null);
parameterContext.setParameters(Collections.singletonMap("pass", new Parameter(new ParameterDescriptor.Builder().name("pass").sensitive(true).build(), "secret")));
getRootGroup().setParameterContext(parameterContext);
final Map<String, String> properties = new HashMap<>();
properties.put("username", "${#{pass}}");
try {
usernamePassword.setProperties(properties);
Assert.fail("Was able to set properties when referencing sensitive parameter from within EL");
} catch (final IllegalArgumentException iae) {
// Expected. Since the parameter is sensitive, it may referenced by a sensitive property
}
}
@Test
public void testSensitivePropertyCannotBeSetToReferenceParamFromEL() {
final ProcessorNode usernamePassword = createProcessorNode(UsernamePasswordProcessor.class);
final ParameterReferenceManager referenceManager = new StandardParameterReferenceManager(getFlowController().getFlowManager());
final ParameterContext parameterContext = new StandardParameterContext(UUID.randomUUID().toString(), "param-context", referenceManager, null);
parameterContext.setParameters(Collections.singletonMap("pass", new Parameter(new ParameterDescriptor.Builder().name("pass").sensitive(true).build(), "secret")));
getRootGroup().setParameterContext(parameterContext);
final Map<String, String> properties = new HashMap<>();
properties.put("password", "${#{pass}}");
try {
usernamePassword.setProperties(properties);
Assert.fail("Was able to set properties when referencing sensitive parameter from within EL");
} catch (final IllegalArgumentException iae) {
// Expected. Since the property is sensitive, it may reference a parameter only if that is the only value.
}
}
}