From f50a07ec888094cb610e5e12f6b52599143c42ea Mon Sep 17 00:00:00 2001 From: Josh Anderton Date: Sun, 17 Dec 2017 20:01:34 -0600 Subject: [PATCH] NIFI-4655: This closes #2347. Fixed NPE in the InvokeHTTP processor caused when an SSLContextService was assigned without keystore properties - Added check for keystore properties and only initialized keystore when necessary. - Added TestInvokeHttpTwoWaySSL test class to test with two-way SSL - Modified TestInvokeHttpSSL to test with one-way SSL Signed-off-by: joewitt --- .../nifi/processors/standard/InvokeHTTP.java | 73 +++++++++++-------- .../standard/TestInvokeHttpSSL.java | 41 +++++++++-- .../standard/TestInvokeHttpTwoWaySSL.java | 51 +++++++++++++ 3 files changed, 130 insertions(+), 35 deletions(-) create mode 100644 nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestInvokeHttpTwoWaySSL.java diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java index 65a28d502e..300e9cbcec 100644 --- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java @@ -62,6 +62,7 @@ import org.joda.time.format.DateTimeFormat; import org.joda.time.format.DateTimeFormatter; import javax.net.ssl.HostnameVerifier; +import javax.net.ssl.KeyManager; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLSession; @@ -253,14 +254,14 @@ public final class InvokeHTTP extends AbstractProcessor { .build(); public static final PropertyDescriptor PROP_CONTENT_TYPE = new PropertyDescriptor.Builder() - .name("Content-Type") - .description("The Content-Type to specify for when content is being transmitted through a PUT, POST or PATCH. " - + "In the case of an empty value after evaluating an expression language expression, Content-Type defaults to " + DEFAULT_CONTENT_TYPE) - .required(true) - .expressionLanguageSupported(true) - .defaultValue("${" + CoreAttributes.MIME_TYPE.key() + "}") - .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING)) - .build(); + .name("Content-Type") + .description("The Content-Type to specify for when content is being transmitted through a PUT, POST or PATCH. " + + "In the case of an empty value after evaluating an expression language expression, Content-Type defaults to " + DEFAULT_CONTENT_TYPE) + .required(true) + .expressionLanguageSupported(true) + .defaultValue("${" + CoreAttributes.MIME_TYPE.key() + "}") + .addValidator(StandardValidators.createAttributeExpressionLanguageValidator(AttributeExpression.ResultType.STRING)) + .build(); public static final PropertyDescriptor PROP_SEND_BODY = new PropertyDescriptor.Builder() .name("send-message-body") @@ -567,31 +568,42 @@ public final class InvokeHTTP extends AbstractProcessor { */ private void setSslSocketFactory(OkHttpClient.Builder okHttpClientBuilder, SSLContextService sslService, SSLContext sslContext) throws IOException, KeyStoreException, CertificateException, NoSuchAlgorithmException, UnrecoverableKeyException, KeyManagementException { - final String keystoreLocation = sslService.getKeyStoreFile(); - final String keystorePass = sslService.getKeyStorePassword(); - final String keystoreType = sslService.getKeyStoreType(); - - // prepare the keystore - final KeyStore keyStore = KeyStore.getInstance(keystoreType); - - try (FileInputStream keyStoreStream = new FileInputStream(keystoreLocation)) { - keyStore.load(keyStoreStream, keystorePass.toCharArray()); - } final KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); - keyManagerFactory.init(keyStore, keystorePass.toCharArray()); - - // load truststore - final String truststoreLocation = sslService.getTrustStoreFile(); - final String truststorePass = sslService.getTrustStorePassword(); - final String truststoreType = sslService.getTrustStoreType(); - - KeyStore truststore = KeyStore.getInstance(truststoreType); final TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance("X509"); - truststore.load(new FileInputStream(truststoreLocation), truststorePass.toCharArray()); - trustManagerFactory.init(truststore); + // initialize the KeyManager array to null and we will overwrite later if a keystore is loaded + KeyManager[] keyManagers = null; - /* + // we will only initialize the keystore if properties have been supplied by the SSLContextService + if (sslService.isKeyStoreConfigured()) { + final String keystoreLocation = sslService.getKeyStoreFile(); + final String keystorePass = sslService.getKeyStorePassword(); + final String keystoreType = sslService.getKeyStoreType(); + + // prepare the keystore + final KeyStore keyStore = KeyStore.getInstance(keystoreType); + + try (FileInputStream keyStoreStream = new FileInputStream(keystoreLocation)) { + keyStore.load(keyStoreStream, keystorePass.toCharArray()); + } + + keyManagerFactory.init(keyStore, keystorePass.toCharArray()); + keyManagers = keyManagerFactory.getKeyManagers(); + } + + // we will only initialize the truststure if properties have been supplied by the SSLContextService + if (sslService.isTrustStoreConfigured()) { + // load truststore + final String truststoreLocation = sslService.getTrustStoreFile(); + final String truststorePass = sslService.getTrustStorePassword(); + final String truststoreType = sslService.getTrustStoreType(); + + KeyStore truststore = KeyStore.getInstance(truststoreType); + truststore.load(new FileInputStream(truststoreLocation), truststorePass.toCharArray()); + trustManagerFactory.init(truststore); + } + + /* TrustManagerFactory.getTrustManagers returns a trust manager for each type of trust material. Since we are getting a trust manager factory that uses "X509" as it's trust management algorithm, we are able to grab the first (and thus the most preferred) and use it as our x509 Trust Manager @@ -605,7 +617,8 @@ public final class InvokeHTTP extends AbstractProcessor { throw new IllegalStateException("List of trust managers is null"); } - sslContext.init(keyManagerFactory.getKeyManagers(), trustManagerFactory.getTrustManagers(), null); + // if keystore properties were not supplied, the keyManagers array will be null + sslContext.init(keyManagers, trustManagerFactory.getTrustManagers(), null); final SSLSocketFactory sslSocketFactory = sslContext.getSocketFactory(); okHttpClientBuilder.sslSocketFactory(sslSocketFactory, x509TrustManager); diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestInvokeHttpSSL.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestInvokeHttpSSL.java index 655492f219..7f0bcbb827 100644 --- a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestInvokeHttpSSL.java +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestInvokeHttpSSL.java @@ -29,9 +29,15 @@ import java.io.IOException; import java.util.HashMap; import java.util.Map; +/** + * Executes the same tests as TestInvokeHttp but with one-way SSL enabled. The Jetty server created for these tests + * will not require client certificates and the client will not use keystore properties in the SSLContextService. + */ public class TestInvokeHttpSSL extends TestInvokeHttpCommon { - private static Map sslProperties; + protected static Map sslProperties; + protected static Map serverSslProperties; + @BeforeClass public static void beforeClass() throws Exception { @@ -41,7 +47,8 @@ public class TestInvokeHttpSSL extends TestInvokeHttpCommon { // create the SSL properties, which basically store keystore / trustore information // this is used by the StandardSSLContextService and the Jetty Server - sslProperties = createSslProperties(); + serverSslProperties = createServerSslProperties(false); + sslProperties = createSslProperties(false); // create a Jetty server on a random port server = createServer(); @@ -81,15 +88,39 @@ public class TestInvokeHttpSSL extends TestInvokeHttpCommon { runner.shutdown(); } - private static TestServer createServer() throws IOException { - return new TestServer(sslProperties); + protected static TestServer createServer() throws IOException { + return new TestServer(serverSslProperties); } - private static Map createSslProperties() { + protected static Map createServerSslProperties(boolean clientAuth) { final Map map = new HashMap<>(); + // if requesting client auth then we must also provide a truststore + if (clientAuth) { + map.put(TestServer.NEED_CLIENT_AUTH, Boolean.toString(true)); + map.put(StandardSSLContextService.TRUSTSTORE.getName(), "src/test/resources/localhost-ts.jks"); + map.put(StandardSSLContextService.TRUSTSTORE_PASSWORD.getName(), "localtest"); + map.put(StandardSSLContextService.TRUSTSTORE_TYPE.getName(), "JKS"); + } else { + map.put(TestServer.NEED_CLIENT_AUTH, Boolean.toString(false)); + } + // keystore is always required for the server SSL properties map.put(StandardSSLContextService.KEYSTORE.getName(), "src/test/resources/localhost-ks.jks"); map.put(StandardSSLContextService.KEYSTORE_PASSWORD.getName(), "localtest"); map.put(StandardSSLContextService.KEYSTORE_TYPE.getName(), "JKS"); + + return map; + } + + + protected static Map createSslProperties(boolean clientAuth) { + final Map map = new HashMap<>(); + // if requesting client auth then we must provide a keystore + if (clientAuth) { + map.put(StandardSSLContextService.KEYSTORE.getName(), "src/test/resources/localhost-ks.jks"); + map.put(StandardSSLContextService.KEYSTORE_PASSWORD.getName(), "localtest"); + map.put(StandardSSLContextService.KEYSTORE_TYPE.getName(), "JKS"); + } + // truststore is always required for the client SSL properties map.put(StandardSSLContextService.TRUSTSTORE.getName(), "src/test/resources/localhost-ts.jks"); map.put(StandardSSLContextService.TRUSTSTORE_PASSWORD.getName(), "localtest"); map.put(StandardSSLContextService.TRUSTSTORE_TYPE.getName(), "JKS"); diff --git a/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestInvokeHttpTwoWaySSL.java b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestInvokeHttpTwoWaySSL.java new file mode 100644 index 0000000000..cfd96e0500 --- /dev/null +++ b/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestInvokeHttpTwoWaySSL.java @@ -0,0 +1,51 @@ +/* + * 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.processors.standard; + +import org.junit.BeforeClass; + +/** + * This is probably overkill but in keeping with the same pattern as the TestInvokeHttp and TestInvokeHttpSSL class, + * we will execute the same tests using two-way SSL. The Jetty server created for these tests will require client + * certificates and the client will utilize keystore properties in the SSLContextService. + */ +public class TestInvokeHttpTwoWaySSL extends TestInvokeHttpSSL { + + + @BeforeClass + public static void beforeClass() throws Exception { + // useful for verbose logging output + // don't commit this with this property enabled, or any 'mvn test' will be really verbose + // System.setProperty("org.slf4j.simpleLogger.log.nifi.processors.standard", "debug"); + + // create the SSL properties, which basically store keystore / trustore information + // this is used by the StandardSSLContextService and the Jetty Server + serverSslProperties = createServerSslProperties(true); + sslProperties = createSslProperties(true); + + // create a Jetty server on a random port + server = createServer(); + server.startServer(); + + // Allow time for the server to start + Thread.sleep(500); + // this is the base url with the random port + url = server.getSecureUrl(); + } + +}