greymatter-io / nifi-sdk

Custom processors, scripts, and templates for using Grey Matter Data with Apache NiFi
Apache License 2.0
4 stars 2 forks source link

Improve SSL Context support in GetPolicies #45

Open lucasmoten opened 3 years ago

lucasmoten commented 3 years ago

This same concern may apply to other processors initializing SSL Context in the same way.

There is not adequate support for a Key Password indepdent of the Keystore Password. The NiFi interface allows for specifying both a Keystore Password and a Key Password as shown here Screenshot from 2020-12-11 19-39-33

The current implementation in GetPolicies only takes into account the Keystore Password when initializing the key manager https://github.com/greymatter-io/nifi-sdk/blob/main/gmd-sdk/nifi-data-processors/src/main/scala/com/deciphernow/greymatter/data/nifi/processors/GetPolicies.java#L288-L303

The description for Key Password is as follows: 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.

dborncamp commented 3 years ago

Those lines came directly from the implementation of InvokeHttp: https://github.com/apache/nifi/blob/rel/nifi-1.11.4/nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/InvokeHTTP.java#L691-L706

lucasmoten commented 3 years ago

in 1.11.4 it doesnt call setSslSocketFactory at all from setUpClient. thats dead code

The code block looks like this for lines 647-666

        final SSLContextService sslService = context.getProperty(PROP_SSL_CONTEXT_SERVICE).asControllerService(SSLContextService.class);
        final SSLContext sslContext = sslService == null ? null : sslService.createSSLContext(ClientAuth.NONE);

        // check if the ssl context is set and add the factory if so
        if (sslContext != null) {
            Tuple<SSLContext, TrustManager[]> sslContextTuple =SslContextFactory.createTrustSslContextWithTrustManagers(
                        sslService.getKeyStoreFile(),
                        sslService.getKeyStorePassword() != null ? sslService.getKeyStorePassword().toCharArray() : null,
                        sslService.getKeyPassword() != null ? sslService.getKeyPassword().toCharArray() : null,
                        sslService.getKeyStoreType(),
                        sslService.getTrustStoreFile(),
                        sslService.getTrustStorePassword() != null ? sslService.getTrustStorePassword().toCharArray() : null,
                        sslService.getTrustStoreType(),
                        SslContextFactory.ClientAuth.NONE,
                        sslService.getSslAlgorithm());
            List<X509TrustManager> x509TrustManagers = Arrays.stream(sslContextTuple.getValue())
                    .filter(trustManager -> trustManager instanceof X509TrustManager)
                    .map(trustManager -> (X509TrustManager) trustManager).collect(Collectors.toList());
            okHttpClientBuilder.sslSocketFactory(sslContextTuple.getKey().getSocketFactory(), x509TrustManagers.get(0));
            }

In 1.12.1, the logic changes again in its lines 761-766

        // Apply the TLS configuration if present
        final SSLContextService sslService = context.getProperty(PROP_SSL_CONTEXT_SERVICE).asControllerService(SSLContextService.class);
        if (sslService != null) {
            final TlsConfiguration tlsConfiguration = sslService.createTlsConfiguration();
            OkHttpClientUtils.applyTlsToOkHttpClientBuilder(tlsConfiguration, okHttpClientBuilder);
        }
lucasmoten commented 3 years ago

Needs reassessed for whether NiFi 1.12.1+ code pattern adequately handles setting a Key Password independently of Keystore Password