pvillard31 commented on code in PR #11764: URL: https://github.com/apache/nifi/pull/11764#discussion_r4199813154
########## nifi-extension-bundles/nifi-splunk-bundle/nifi-splunk-processors/src/main/java/org/apache/nifi/processors/splunk/SplunkWebClients.java: ########## @@ -0,0 +1,83 @@ +/* + * 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.splunk; + +import com.splunk.Service; +import com.splunk.WebClientSplunkService; +import org.apache.nifi.logging.ComponentLog; +import org.apache.nifi.ssl.SSLContextProvider; +import org.apache.nifi.web.client.StandardWebClientService; +import org.apache.nifi.web.client.api.WebClientService; +import org.apache.nifi.web.client.ssl.TlsContext; +import org.jspecify.annotations.NonNull; + +import java.net.http.HttpClient; +import java.util.Map; +import java.util.Optional; +import javax.net.ssl.SSLContext; +import javax.net.ssl.X509ExtendedTrustManager; +import javax.net.ssl.X509KeyManager; +import javax.net.ssl.X509TrustManager; + +final class SplunkWebClients { + private static final String USERNAME_ARGUMENT = "username"; + + private SplunkWebClients() { + } + + static StandardWebClientService create(final SSLContextProvider sslContextProvider, final String hostname, final ComponentLog logger) { + final X509TrustManager delegateTrustManager = sslContextProvider.createTrustManager(); + final X509ExtendedTrustManager trustManager = new ConfiguredHostTrustManager(delegateTrustManager, hostname, logger); + final Optional<X509KeyManager> keyManager = sslContextProvider.createKeyManager().map(X509KeyManager.class::cast); + final SSLContext sslContext = sslContextProvider.createContext(); + + return getClient(sslContext, trustManager, keyManager); + } + + static Service connect(final Map<String, Object> serviceArgs, final WebClientService webClientService) { + final WebClientSplunkService service = new WebClientSplunkService(serviceArgs, webClientService); + if (serviceArgs.containsKey(USERNAME_ARGUMENT)) { + service.login(); + } + + return service; + } + + private static @NonNull StandardWebClientService getClient(SSLContext sslContext, X509ExtendedTrustManager trustManager, Optional<X509KeyManager> keyManager) { Review Comment: Can we remove the @NonNull annotation to avoid relying on a transitive jspecify dependency, and make these parameters final? ########## nifi-extension-bundles/nifi-splunk-bundle/nifi-splunk-processors/src/main/java/org/apache/nifi/processors/splunk/ConfiguredHostTrustManager.java: ########## @@ -0,0 +1,124 @@ +/* + * 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.splunk; + +import org.apache.nifi.logging.ComponentLog; + +import java.net.Socket; +import java.security.cert.CertificateException; +import java.security.cert.X509Certificate; +import java.util.Objects; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLSession; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.X509ExtendedTrustManager; +import javax.net.ssl.X509TrustManager; + +/** + * Trust manager that skips certificate subject name matching when the TLS peer host is the hostname + * configured on the processor, and delegates certificate path validation. Any other peer is checked + * by the delegate trust manager. + */ +class ConfiguredHostTrustManager extends X509ExtendedTrustManager { + private final X509TrustManager delegate; + private final String configuredHost; + private final ComponentLog logger; + + ConfiguredHostTrustManager(final X509TrustManager delegate, final String hostname, final ComponentLog logger) { + this.delegate = Objects.requireNonNull(delegate, "Trust Manager required"); + this.logger = Objects.requireNonNull(logger, "Logger required"); + final String requiredHostname = Objects.requireNonNull(hostname, "Hostname required"); + if (requiredHostname.isBlank()) { + throw new IllegalArgumentException("Hostname required"); + } + + this.configuredHost = requiredHostname; + } + + @Override + public void checkClientTrusted(final X509Certificate[] chain, final String authType) throws CertificateException { + delegate.checkClientTrusted(chain, authType); + } + + @Override + public void checkServerTrusted(final X509Certificate[] chain, final String authType) throws CertificateException { + // Callers that do not supply a peer socket or engine cannot be matched to the configured hostname. + delegate.checkServerTrusted(chain, authType); + } + + @Override + public X509Certificate[] getAcceptedIssuers() { + return delegate.getAcceptedIssuers(); + } + + @Override + public void checkClientTrusted(final X509Certificate[] chain, final String authType, final Socket socket) throws CertificateException { + if (delegate instanceof final X509ExtendedTrustManager extendedTrustManager) { + extendedTrustManager.checkClientTrusted(chain, authType, socket); + } else { + delegate.checkClientTrusted(chain, authType); + } + } + + @Override + public void checkServerTrusted(final X509Certificate[] chain, final String authType, final Socket socket) throws CertificateException { + if (socket instanceof final SSLSocket sslSocket) { + final SSLSession session = sslSocket.getHandshakeSession(); + if (session == null) { + throw new CertificateException("No handshake session"); + } + + final String peerHost = session.getPeerHost(); + if (configuredHost.contentEquals(peerHost)) { + logger.debug("Peer Host [{}] matches Configured Host [{}]", peerHost, configuredHost); + delegate.checkServerTrusted(chain, authType); + return; + } + } + + if (delegate instanceof final X509ExtendedTrustManager extendedTrustManager) { + extendedTrustManager.checkServerTrusted(chain, authType, socket); + } else { + delegate.checkServerTrusted(chain, authType); + } + } + + @Override + public void checkClientTrusted(final X509Certificate[] chain, final String authType, final SSLEngine engine) throws CertificateException { + if (delegate instanceof final X509ExtendedTrustManager extendedTrustManager) { + extendedTrustManager.checkClientTrusted(chain, authType, engine); + } else { + delegate.checkClientTrusted(chain, authType); + } + } + + @Override + public void checkServerTrusted(final X509Certificate[] chain, final String authType, final SSLEngine engine) throws CertificateException { + final String peerHost = engine == null ? null : engine.getPeerHost(); + if (peerHost == null) { + logger.debug("Peer Host not provided from SSLEngine"); + delegate.checkServerTrusted(chain, authType); + } else if (configuredHost.contentEquals(peerHost)) { Review Comment: Since the peer host comes from the same configured Host value, does this branch skip certificate hostname verification for every request? Should normal hostname verification remain the default, with any bypass requiring explicit configuration? ########## nifi-extension-bundles/nifi-splunk-bundle/nifi-splunk-processors/src/main/java/org/apache/nifi/processors/splunk/GetSplunk.java: ########## @@ -574,18 +575,45 @@ protected Service createSplunkService(final ProcessContext context) { } final SSLContextProvider sslContextProvider = context.getProperty(SSL_CONTEXT_SERVICE).asControllerService(SSLContextProvider.class); - if (sslContextProvider != null) { - // Service construction reapplies the static security protocol and replaces the Socket Factory when that protocol changes. - // Leaving the protocol unchanged keeps the Socket Factory supplied by the SSL Context Service. - Service.setSSLSocketFactory(sslContextProvider.createContext().getSocketFactory()); + final StandardWebClientService client; + if (sslContextProvider == null) { + client = null; + } else { + client = getWebClientService(sslContextProvider, host, connectTimeout, readTimeout); } final String chosenApiVersion = context.getProperty(API_VERSION).getValue(); final boolean enableV2SearchApi = API_VERSION_V2.getValue().equals(chosenApiVersion); serviceArgs.add("enableV2SearchApi", enableV2SearchApi); - return Service.connect(serviceArgs); + return SplunkWebClients.connect(serviceArgs, client); + } + + private StandardWebClientService getWebClientService( + final SSLContextProvider sslContextProvider, + final String host, + final int connectTimeout, + final int readTimeout + ) { + if (webClientService == null) { + webClientService = SplunkWebClients.create(sslContextProvider, host, getLogger()); + if (connectTimeout > 0) { + webClientService.setConnectTimeout(Duration.ofMillis(connectTimeout)); + } + if (readTimeout > 0) { Review Comment: Does this preserve the previous read timeout while consuming the response body? HttpRequest.timeout may only cover the request until the response headers are received. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
