From 31ec01b5f9a78bf96eefb4990dd59202fcd6b5bd Mon Sep 17 00:00:00 2001 From: Andy LoPresto Date: Wed, 4 Jan 2017 19:20:52 -0800 Subject: [PATCH] NIFI-3004 Added failing unit test to SSLContextServiceTest to demonstrate that customValidate caches result from previous validation. NIFI-3004 Added logic to expire StandardSSLContextService customValidate cache after 5 invocations. Updated unit test to demonstrate this logic. This closes #1375. Signed-off-by: Andy LoPresto --- .../nifi/ssl/StandardSSLContextService.java | 32 +++++-- .../nifi/ssl/SSLContextServiceTest.java | 90 +++++++++++++++++-- 2 files changed, 110 insertions(+), 12 deletions(-) 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 d10c840b26..9ac27a350c 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 @@ -101,7 +101,7 @@ public class StandardSSLContextService extends AbstractControllerService impleme .name("key-password") .displayName("Key Password") .description("The password for the key. If this is not specified, but the Keystore Filename, Password, and Type are specified, " - + "then the Keystore Password will be assumed to be the same as the Key Password.") + + "then the Keystore Password will be assumed to be the same as the Key Password.") .addValidator(StandardValidators.NON_EMPTY_VALIDATOR) .sensitive(true) .required(false) @@ -120,6 +120,10 @@ public class StandardSSLContextService extends AbstractControllerService impleme private ConfigurationContext configContext; private boolean isValidated; + // TODO: This can be made configurable if necessary + private static final int VALIDATION_CACHE_EXPIRATION = 5; + private int validationCacheCount = 0; + static { List props = new ArrayList<>(); props.add(KEYSTORE); @@ -165,7 +169,7 @@ public class StandardSSLContextService extends AbstractControllerService impleme @Override public void onPropertyModified(PropertyDescriptor descriptor, String oldValue, String newValue) { super.onPropertyModified(descriptor, oldValue, newValue); - isValidated = false; + resetValidationCache(); } private static Validator createFileExistsAndReadableValidator() { @@ -208,8 +212,13 @@ public class StandardSSLContextService extends AbstractControllerService impleme protected Collection customValidate(ValidationContext validationContext) { final Collection results = new ArrayList<>(); - if(isValidated) { - return results; + if (isValidated) { + validationCacheCount++; + if (validationCacheCount > VALIDATION_CACHE_EXPIRATION) { + resetValidationCache(); + } else { + return results; + } } results.addAll(validateStore(validationContext.getProperties(), KeystoreValidationGroup.KEYSTORE)); @@ -246,6 +255,15 @@ public class StandardSSLContextService extends AbstractControllerService impleme return results; } + private void resetValidationCache() { + validationCacheCount = 0; + isValidated = false; + } + + protected int getValidationCacheExpiration() { + return VALIDATION_CACHE_EXPIRATION; + } + private void verifySslConfig(final ValidationContext validationContext) throws ProcessException { final String protocol = validationContext.getProperty(SSL_ALGORITHM).getValue(); try { @@ -266,7 +284,7 @@ public class StandardSSLContextService extends AbstractControllerService impleme SslContextFactory.createSslContext( validationContext.getProperty(KEYSTORE).getValue(), validationContext.getProperty(KEYSTORE_PASSWORD).getValue().toCharArray(), - keyPassword, + keyPassword, validationContext.getProperty(KEYSTORE_TYPE).getValue(), protocol); return; @@ -275,7 +293,7 @@ public class StandardSSLContextService extends AbstractControllerService impleme SslContextFactory.createSslContext( validationContext.getProperty(KEYSTORE).getValue(), validationContext.getProperty(KEYSTORE_PASSWORD).getValue().toCharArray(), - keyPassword, + keyPassword, validationContext.getProperty(KEYSTORE_TYPE).getValue(), validationContext.getProperty(TRUSTSTORE).getValue(), validationContext.getProperty(TRUSTSTORE_PASSWORD).getValue().toCharArray(), @@ -447,7 +465,7 @@ public class StandardSSLContextService extends AbstractControllerService impleme return count; } - public static enum KeystoreValidationGroup { + public enum KeystoreValidationGroup { KEYSTORE, TRUSTSTORE } diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/test/java/org/apache/nifi/ssl/SSLContextServiceTest.java b/nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/test/java/org/apache/nifi/ssl/SSLContextServiceTest.java index 03aacc0801..60b09bd4db 100644 --- a/nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/test/java/org/apache/nifi/ssl/SSLContextServiceTest.java +++ b/nifi-nar-bundles/nifi-standard-services/nifi-ssl-context-bundle/nifi-ssl-context-service/src/test/java/org/apache/nifi/ssl/SSLContextServiceTest.java @@ -16,17 +16,36 @@ */ package org.apache.nifi.ssl; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; +import java.util.Collection; import java.util.HashMap; import java.util.Map; - +import org.apache.nifi.components.ValidationContext; +import org.apache.nifi.components.ValidationResult; import org.apache.nifi.reporting.InitializationException; import org.apache.nifi.ssl.SSLContextService.ClientAuth; +import org.apache.nifi.util.MockProcessContext; +import org.apache.nifi.util.MockValidationContext; import org.apache.nifi.util.TestRunner; import org.apache.nifi.util.TestRunners; import org.junit.Assert; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class SSLContextServiceTest { + private static final Logger logger = LoggerFactory.getLogger(SSLContextServiceTest.class); + + @Rule + public TemporaryFolder tmp = new TemporaryFolder(new File("src/test/resources")); @Test public void testBad1() throws InitializationException { @@ -132,7 +151,7 @@ public class SSLContextServiceTest { runner.assertValid(service); runner.disableControllerService(service); - runner.setProperty(service,StandardSSLContextService.KEYSTORE.getName(), "src/test/resources/DOES-NOT-EXIST.jks"); + runner.setProperty(service, StandardSSLContextService.KEYSTORE.getName(), "src/test/resources/DOES-NOT-EXIST.jks"); runner.assertNotValid(service); runner.setProperty(service, StandardSSLContextService.KEYSTORE.getName(), "src/test/resources/localhost-ks.jks"); @@ -144,6 +163,67 @@ public class SSLContextServiceTest { runner.assertValid(service); } + @Test + public void testValidationResultsCacheShouldExpire() throws InitializationException, IOException { + // Arrange + + // Copy the keystore and truststore to a tmp directory so the originals are not modified + File originalKeystore = new File("src/test/resources/localhost-ks.jks"); + File originalTruststore = new File("src/test/resources/localhost-ts.jks"); + + File tmpKeystore = tmp.newFile("keystore-tmp.jks"); + File tmpTruststore = tmp.newFile("truststore-tmp.jks"); + + Files.copy(originalKeystore.toPath(), tmpKeystore.toPath(), StandardCopyOption.REPLACE_EXISTING); + Files.copy(originalTruststore.toPath(), tmpTruststore.toPath(), StandardCopyOption.REPLACE_EXISTING); + + final TestRunner runner = TestRunners.newTestRunner(TestProcessor.class); + SSLContextService service = new StandardSSLContextService(); + final String serviceIdentifier = "test-should-expire"; + runner.addControllerService(serviceIdentifier, service); + runner.setProperty(service, StandardSSLContextService.KEYSTORE.getName(), tmpKeystore.getAbsolutePath()); + runner.setProperty(service, StandardSSLContextService.KEYSTORE_PASSWORD.getName(), "localtest"); + runner.setProperty(service, StandardSSLContextService.KEYSTORE_TYPE.getName(), "JKS"); + runner.setProperty(service, StandardSSLContextService.TRUSTSTORE.getName(), tmpTruststore.getAbsolutePath()); + runner.setProperty(service, StandardSSLContextService.TRUSTSTORE_PASSWORD.getName(), "localtest"); + runner.setProperty(service, StandardSSLContextService.TRUSTSTORE_TYPE.getName(), "JKS"); + runner.enableControllerService(service); + + runner.setProperty("SSL Context Svc ID", serviceIdentifier); + runner.assertValid(service); + + final StandardSSLContextService sslContextService = (StandardSSLContextService) service; + + // Act + boolean isDeleted = tmpKeystore.delete(); + assert isDeleted; + assert !tmpKeystore.exists(); + logger.info("Deleted keystore file"); + + // Manually validate the service (expecting cached result to be returned) + final MockProcessContext processContext = (MockProcessContext) runner.getProcessContext(); + // This service does not use the state manager or variable registry + final ValidationContext validationContext = new MockValidationContext(processContext, null, null); + + // Even though the keystore file is no longer present, because no property changed, the cached result is still valid + Collection validationResults = sslContextService.customValidate(validationContext); + assertTrue("validation results is not empty", validationResults.isEmpty()); + logger.info("(1) StandardSSLContextService#customValidate() returned true even though the keystore file is no longer available"); + + // Assert + + // Have to exhaust the cached result by checking n-1 more times + for (int i = 2; i < sslContextService.getValidationCacheExpiration(); i++) { + validationResults = sslContextService.customValidate(validationContext); + assertTrue("validation results is not empty", validationResults.isEmpty()); + logger.info("(" + i + ") StandardSSLContextService#customValidate() returned true even though the keystore file is no longer available"); + } + + validationResults = sslContextService.customValidate(validationContext); + assertFalse("validation results is empty", validationResults.isEmpty()); + logger.info("(" + sslContextService.getValidationCacheExpiration() + ") StandardSSLContextService#customValidate() returned false because the cache expired"); + } + @Test public void testGoodTrustOnly() { try { @@ -159,7 +239,7 @@ public class SSLContextServiceTest { runner.setProperty("SSL Context Svc ID", "test-good2"); runner.assertValid(); Assert.assertNotNull(service); - Assert.assertTrue(service instanceof StandardSSLContextService); + assertTrue(service instanceof StandardSSLContextService); service.createSSLContext(ClientAuth.NONE); } catch (InitializationException e) { } @@ -180,7 +260,7 @@ public class SSLContextServiceTest { runner.setProperty("SSL Context Svc ID", "test-good3"); runner.assertValid(); Assert.assertNotNull(service); - Assert.assertTrue(service instanceof StandardSSLContextService); + assertTrue(service instanceof StandardSSLContextService); SSLContextService sslService = service; sslService.createSSLContext(ClientAuth.NONE); } catch (Exception e) { @@ -205,7 +285,7 @@ public class SSLContextServiceTest { runner.setProperty("SSL Context Svc ID", "test-diff-keys"); runner.assertValid(); Assert.assertNotNull(service); - Assert.assertTrue(service instanceof StandardSSLContextService); + assertTrue(service instanceof StandardSSLContextService); SSLContextService sslService = service; sslService.createSSLContext(ClientAuth.NONE); } catch (Exception e) {