From 5150dff70b4aa210abb86e6d8c1aeb03c8c364e3 Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Wed, 9 Aug 2017 20:03:33 -0700 Subject: [PATCH] NIFI-4274 Removed EL evaluation logic from custom file validator. Signed-off-by: Pierre Villard This closes #2071. --- .../nifi/ssl/StandardSSLContextService.java | 44 ++--- .../ssl/StandardSSLContextServiceTest.groovy | 157 ++++++++++++++++++ 2 files changed, 173 insertions(+), 28 deletions(-) create mode 100644 nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/test/groovy/org/apache/nifi/ssl/StandardSSLContextServiceTest.groovy diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/main/java/org/apache/nifi/ssl/StandardSSLContextService.java b/nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/main/java/org/apache/nifi/ssl/StandardSSLContextService.java index 9ac27a350c..2570855757 100644 --- a/nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/main/java/org/apache/nifi/ssl/StandardSSLContextService.java +++ b/nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/main/java/org/apache/nifi/ssl/StandardSSLContextService.java @@ -16,6 +16,18 @@ */ package org.apache.nifi.ssl; +import java.io.File; +import java.net.MalformedURLException; +import java.security.NoSuchAlgorithmException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import javax.net.ssl.SSLContext; import org.apache.nifi.annotation.documentation.CapabilityDescription; import org.apache.nifi.annotation.documentation.Tags; import org.apache.nifi.annotation.lifecycle.OnEnabled; @@ -34,20 +46,7 @@ import org.apache.nifi.security.util.CertificateUtils; import org.apache.nifi.security.util.KeystoreType; import org.apache.nifi.security.util.SslContextFactory; -import javax.net.ssl.SSLContext; -import java.io.File; -import java.net.MalformedURLException; -import java.security.NoSuchAlgorithmException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Set; - -@Tags({"ssl", "secure", "certificate", "keystore", "truststore", "jks", "p12", "pkcs12", "pkcs"}) +@Tags({"ssl", "secure", "certificate", "keystore", "truststore", "jks", "p12", "pkcs12", "pkcs", "tls"}) @CapabilityDescription("Standard implementation of the SSLContextService. Provides the ability to configure " + "keystore and/or truststore properties once and reuse that configuration throughout the application") public class StandardSSLContextService extends AbstractControllerService implements SSLContextService { @@ -108,10 +107,11 @@ public class StandardSSLContextService extends AbstractControllerService impleme .build(); public static final PropertyDescriptor SSL_ALGORITHM = new PropertyDescriptor.Builder() .name("SSL Protocol") + .displayName("TLS Protocol") .defaultValue("TLS") .required(false) .allowableValues(buildAlgorithmAllowableValues()) - .description("The algorithm to use for this SSL context") + .description("The algorithm to use for this TLS/SSL context") .addValidator(StandardValidators.NON_EMPTY_VALIDATOR) .sensitive(false) .build(); @@ -178,19 +178,7 @@ public class StandardSSLContextService extends AbstractControllerService impleme // allow expression language @Override public ValidationResult validate(String subject, String input, ValidationContext context) { - final String substituted; - try { - substituted = context.newPropertyValue(input).evaluateAttributeExpressions().getValue(); - } catch (final Exception e) { - return new ValidationResult.Builder() - .subject(subject) - .input(input) - .valid(false) - .explanation("Not a valid Expression Language value: " + e.getMessage()) - .build(); - } - - final File file = new File(substituted); + final File file = new File(input); final boolean valid = file.exists() && file.canRead(); final String explanation = valid ? null : "File " + file + " does not exist or cannot be read"; return new ValidationResult.Builder() diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/test/groovy/org/apache/nifi/ssl/StandardSSLContextServiceTest.groovy b/nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/test/groovy/org/apache/nifi/ssl/StandardSSLContextServiceTest.groovy new file mode 100644 index 0000000000..881eb02df1 --- /dev/null +++ b/nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/test/groovy/org/apache/nifi/ssl/StandardSSLContextServiceTest.groovy @@ -0,0 +1,157 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License") you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.ssl + +import org.apache.nifi.components.ValidationContext +import org.apache.nifi.components.ValidationResult +import org.apache.nifi.components.Validator +import org.apache.nifi.state.MockStateManager +import org.apache.nifi.util.MockProcessContext +import org.apache.nifi.util.MockValidationContext +import org.apache.nifi.util.MockVariableRegistry +import org.apache.nifi.util.TestRunner +import org.apache.nifi.util.TestRunners +import org.bouncycastle.jce.provider.BouncyCastleProvider +import org.junit.After +import org.junit.AfterClass +import org.junit.Before +import org.junit.BeforeClass +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.junit.runner.RunWith +import org.junit.runners.JUnit4 +import org.slf4j.Logger +import org.slf4j.LoggerFactory + +import java.security.Security + +import static groovy.test.GroovyAssert.shouldFail + +@RunWith(JUnit4.class) +class StandardSSLContextServiceTest { + private static final Logger logger = LoggerFactory.getLogger(StandardSSLContextServiceTest.class) + + private static final String KEYSTORE_PATH = "src/test/resources/localhost-ks.jks" + private static final String TRUSTSTORE_PATH = "src/test/resources/localhost-ts.jks" + private static final String TRUSTSTORE_PATH_WITH_EL = "\${someAttribute}/localhost-ts.jks" + + private static final String KEYSTORE_PASSWORD = "localtest" + private static final String TRUSTSTORE_PASSWORD = "localtest" + + private static final String KEYSTORE_TYPE = "JKS" + private static final String TRUSTSTORE_TYPE = "JKS" + + @Rule + public TemporaryFolder tmp = new TemporaryFolder(new File("src/test/resources")) + + @BeforeClass + static void setUpOnce() throws Exception { + Security.addProvider(new BouncyCastleProvider()) + + logger.metaClass.methodMissing = { String name, args -> + logger.info("[${name?.toUpperCase()}] ${(args as List).join(" ")}") + } + } + + @Before + void setUp() throws Exception { + } + + @After + void tearDown() throws Exception { + } + + @AfterClass + static void tearDownOnce() throws Exception { + } + + @Test + void testShouldValidateSimpleFileValidatorPath() { + // Arrange + TestRunner runner = TestRunners.newTestRunner(TestProcessor.class) + String controllerServiceId = "ssl-context" + final SSLContextService sslContextService = new StandardSSLContextService() + runner.addControllerService(controllerServiceId, sslContextService) + runner.setProperty(sslContextService, StandardSSLContextService.TRUSTSTORE, TRUSTSTORE_PATH) + runner.setProperty(sslContextService, StandardSSLContextService.TRUSTSTORE_PASSWORD, TRUSTSTORE_PASSWORD) + runner.setProperty(sslContextService, StandardSSLContextService.TRUSTSTORE_TYPE, TRUSTSTORE_TYPE) + runner.enableControllerService(sslContextService) + + // Act + runner.assertValid(sslContextService) + + // Assert + final MockProcessContext processContext = (MockProcessContext) runner.getProcessContext() + assert processContext.getControllerServiceProperties(sslContextService).get(StandardSSLContextService.TRUSTSTORE, "") == TRUSTSTORE_PATH + } + + @Test + void testShouldNotValidateExpressionLanguageInFileValidator() { + // Arrange + TestRunner runner = TestRunners.newTestRunner(TestProcessor.class) + String controllerServiceId = "ssl-context" + final SSLContextService sslContextService = new StandardSSLContextService() + runner.addControllerService(controllerServiceId, sslContextService) + runner.setProperty(sslContextService, StandardSSLContextService.TRUSTSTORE, TRUSTSTORE_PATH_WITH_EL) + runner.setProperty(sslContextService, StandardSSLContextService.TRUSTSTORE_PASSWORD, TRUSTSTORE_PASSWORD) + runner.setProperty(sslContextService, StandardSSLContextService.TRUSTSTORE_TYPE, TRUSTSTORE_TYPE) + + // Act + def msg = shouldFail { + runner.enableControllerService(sslContextService) + } + + // Assert + assert msg =~ "invalid because Cannot access file" + runner.assertNotValid(sslContextService) + } + + @Test + void testShouldNotEvaluateExpressionLanguageInFileValidator() { + // Arrange + final String VALID_TRUSTSTORE_PATH_WITH_EL = "\${literal(''):trim()}${TRUSTSTORE_PATH}" + + TestRunner runner = TestRunners.newTestRunner(TestProcessor.class) + String controllerServiceId = "ssl-context" + final SSLContextService sslContextService = new StandardSSLContextService() + runner.addControllerService(controllerServiceId, sslContextService) + runner.setProperty(sslContextService, StandardSSLContextService.TRUSTSTORE, VALID_TRUSTSTORE_PATH_WITH_EL) + runner.setProperty(sslContextService, StandardSSLContextService.TRUSTSTORE_PASSWORD, TRUSTSTORE_PASSWORD) + runner.setProperty(sslContextService, StandardSSLContextService.TRUSTSTORE_TYPE, TRUSTSTORE_TYPE) + + // The verifySslConfig and customValidate methods correctly do not evaluate EL, but the custom file validator does, so extract it alone and validate + Validator fileValidator = StandardSSLContextService.createFileExistsAndReadableValidator() + final ValidationContext mockValidationContext = new MockValidationContext( + runner.getProcessContext() as MockProcessContext, + new MockStateManager(sslContextService), + new MockVariableRegistry()) + + // Act + ValidationResult vr = fileValidator.validate(StandardSSLContextService.TRUSTSTORE.name, VALID_TRUSTSTORE_PATH_WITH_EL, mockValidationContext) + logger.info("Custom file validation result: ${vr}") + + // Assert + final MockProcessContext processContext = (MockProcessContext) runner.getProcessContext() + + // If the EL was evaluated, the paths would be identical + assert processContext.getControllerServiceProperties(sslContextService).get(StandardSSLContextService.TRUSTSTORE, "") != TRUSTSTORE_PATH + + // If the EL was evaluated, the path would be valid + assert !vr.isValid() + } +}