You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "ferencerdei (via GitHub)" <gi...@apache.org> on 2023/03/29 13:14:31 UTC
[GitHub] [nifi] ferencerdei opened a new pull request, #7098: NIFI-11344 Make minifi fips compatible
ferencerdei opened a new pull request, #7098:
URL: https://github.com/apache/nifi/pull/7098
<!-- 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. -->
# Summary
[NIFI-11344](https://issues.apache.org/jira/browse/NIFI-11344)
# Tracking
Please complete the following tracking steps prior to pull request creation.
### Issue Tracking
- [ ] [Apache NiFi Jira](https://issues.apache.org/jira/browse/NIFI) issue created
### Pull Request Tracking
- [ ] Pull Request title starts with Apache NiFi Jira issue number, such as `NIFI-00000`
- [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such `NIFI-00000`
### Pull Request Formatting
- [ ] Pull Request based on current revision of the `main` branch
- [ ] Pull Request refers to a feature branch with one commit containing changes
# Verification
Please indicate the verification steps performed prior to pull request creation.
### Build
- [ ] Build completed using `mvn clean install -P contrib-check`
- [ ] JDK 11
- [ ] JDK 17
### Licensing
- [ ] New dependencies are compatible with the [Apache License 2.0](https://apache.org/licenses/LICENSE-2.0) according to the [License Policy](https://www.apache.org/legal/resolved.html)
- [ ] New dependencies are documented in applicable `LICENSE` and `NOTICE` files
### Documentation
- [ ] Documentation formatting appears as expected in rendered files
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] ferencerdei commented on a diff in pull request #7098: NIFI-11344 Make MiNiFi FIPS compatible
Posted by "ferencerdei (via GitHub)" <gi...@apache.org>.
ferencerdei commented on code in PR #7098:
URL: https://github.com/apache/nifi/pull/7098#discussion_r1154089484
##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/configuration/ingestors/RestChangeIngestor.java:
##########
@@ -172,39 +182,53 @@ private void createConnector(Properties properties) {
http.setIdleTimeout(30000L);
jetty.addConnector(http);
- logger.info("Added an http connector on the host '{}' and port '{}'", new Object[]{http.getHost(), http.getPort()});
+ logger.info("Added an http connector on the host '{}' and port '{}'", http.getHost(), http.getPort());
}
private void createSecureConnector(Properties properties) {
- SslContextFactory ssl = new SslContextFactory();
-
- if (properties.getProperty(KEYSTORE_LOCATION_KEY) != null) {
- ssl.setKeyStorePath(properties.getProperty(KEYSTORE_LOCATION_KEY));
- ssl.setKeyStorePassword(properties.getProperty(KEYSTORE_PASSWORD_KEY));
- ssl.setKeyStoreType(properties.getProperty(KEYSTORE_TYPE_KEY));
+ KeyStore keyStore;
+ KeyStore truststore = null;
+
+ try (FileInputStream keyStoreStream = new FileInputStream(properties.getProperty(KEYSTORE_LOCATION_KEY))) {
Review Comment:
There is a check already before we call the createSecureConnector, that's why I removed the duplicated check. I'm going to extend the exception message.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] bejancsaba closed pull request #7098: NIFI-11344 Make MiNiFi FIPS compatible
Posted by "bejancsaba (via GitHub)" <gi...@apache.org>.
bejancsaba closed pull request #7098: NIFI-11344 Make MiNiFi FIPS compatible
URL: https://github.com/apache/nifi/pull/7098
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] exceptionfactory commented on a diff in pull request #7098: NIFI-11344 Make MiNiFi FIPS compatible
Posted by "exceptionfactory (via GitHub)" <gi...@apache.org>.
exceptionfactory commented on code in PR #7098:
URL: https://github.com/apache/nifi/pull/7098#discussion_r1153559962
##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/configuration/ingestors/RestChangeIngestor.java:
##########
@@ -172,39 +182,53 @@ private void createConnector(Properties properties) {
http.setIdleTimeout(30000L);
jetty.addConnector(http);
- logger.info("Added an http connector on the host '{}' and port '{}'", new Object[]{http.getHost(), http.getPort()});
+ logger.info("Added an http connector on the host '{}' and port '{}'", http.getHost(), http.getPort());
}
private void createSecureConnector(Properties properties) {
- SslContextFactory ssl = new SslContextFactory();
-
- if (properties.getProperty(KEYSTORE_LOCATION_KEY) != null) {
- ssl.setKeyStorePath(properties.getProperty(KEYSTORE_LOCATION_KEY));
- ssl.setKeyStorePassword(properties.getProperty(KEYSTORE_PASSWORD_KEY));
- ssl.setKeyStoreType(properties.getProperty(KEYSTORE_TYPE_KEY));
+ KeyStore keyStore;
+ KeyStore truststore = null;
+
+ try (FileInputStream keyStoreStream = new FileInputStream(properties.getProperty(KEYSTORE_LOCATION_KEY))) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(KEYSTORE_TYPE_KEY))
+ .inputStream(keyStoreStream)
+ .password(properties.getProperty(KEYSTORE_PASSWORD_KEY).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
}
if (properties.getProperty(TRUSTSTORE_LOCATION_KEY) != null) {
- ssl.setTrustStorePath(properties.getProperty(TRUSTSTORE_LOCATION_KEY));
- ssl.setTrustStorePassword(properties.getProperty(TRUSTSTORE_PASSWORD_KEY));
- ssl.setTrustStoreType(properties.getProperty(TRUSTSTORE_TYPE_KEY));
- ssl.setNeedClientAuth(Boolean.parseBoolean(properties.getProperty(NEED_CLIENT_AUTH_KEY, "true")));
+ try (FileInputStream trustStoreStream = new FileInputStream(properties.getProperty(TRUSTSTORE_LOCATION_KEY))) {
+ truststore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(TRUSTSTORE_TYPE_KEY))
+ .inputStream(trustStoreStream)
+ .password(properties.getProperty(TRUSTSTORE_PASSWORD_KEY).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
Review Comment:
```suggestion
} catch (final IOException ioe) {
throw new UncheckedIOException("Trust Store loading failed", ioe);
```
##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/configuration/ingestors/RestChangeIngestor.java:
##########
@@ -172,39 +182,53 @@ private void createConnector(Properties properties) {
http.setIdleTimeout(30000L);
jetty.addConnector(http);
- logger.info("Added an http connector on the host '{}' and port '{}'", new Object[]{http.getHost(), http.getPort()});
+ logger.info("Added an http connector on the host '{}' and port '{}'", http.getHost(), http.getPort());
}
private void createSecureConnector(Properties properties) {
- SslContextFactory ssl = new SslContextFactory();
-
- if (properties.getProperty(KEYSTORE_LOCATION_KEY) != null) {
- ssl.setKeyStorePath(properties.getProperty(KEYSTORE_LOCATION_KEY));
- ssl.setKeyStorePassword(properties.getProperty(KEYSTORE_PASSWORD_KEY));
- ssl.setKeyStoreType(properties.getProperty(KEYSTORE_TYPE_KEY));
+ KeyStore keyStore;
+ KeyStore truststore = null;
+
+ try (FileInputStream keyStoreStream = new FileInputStream(properties.getProperty(KEYSTORE_LOCATION_KEY))) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(KEYSTORE_TYPE_KEY))
+ .inputStream(keyStoreStream)
+ .password(properties.getProperty(KEYSTORE_PASSWORD_KEY).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
Review Comment:
Recommend adding a descriptive message to the exception.
```suggestion
} catch (final IOException ioe) {
throw new UncheckedIOException("Key Store loading failed", ioe);
```
##########
minifi/minifi-c2/minifi-c2-jetty/src/main/java/org/apache/nifi/minifi/c2/jetty/JettyServer.java:
##########
@@ -100,7 +113,42 @@ public static void main(String[] args) throws Exception {
server.join();
}
- private static WebAppContext loadWar(final File warFile, final String contextPath, final ClassLoader parentClassLoader) throws IOException {
+ private static SSLContext buildSSLContext(C2Properties properties) {
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ File keyStoreFile = Paths.get(C2_SERVER_HOME).resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
Review Comment:
Placeholders should be used instead of string concatenation:
```suggestion
logger.debug("Loading Key Store [{}]", keyStoreFile.getPath());
```
##########
minifi/minifi-c2/minifi-c2-jetty/src/main/java/org/apache/nifi/minifi/c2/jetty/JettyServer.java:
##########
@@ -100,7 +113,42 @@ public static void main(String[] args) throws Exception {
server.join();
}
- private static WebAppContext loadWar(final File warFile, final String contextPath, final ClassLoader parentClassLoader) throws IOException {
+ private static SSLContext buildSSLContext(C2Properties properties) {
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ File keyStoreFile = Paths.get(C2_SERVER_HOME).resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
+ try (FileInputStream keyStoreStream = new FileInputStream(keyStoreFile)) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_TYPE))
+ .inputStream(keyStoreStream)
+ .password(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ }
+
+ File trustStoreFile = Paths.get(C2_SERVER_HOME).resolve(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE)).toFile();
+ logger.debug("trustStore path: " + trustStoreFile.getPath());
Review Comment:
```suggestion
logger.debug("Loading Trust Store [{}]", trustStoreFile.getPath());
```
##########
minifi/minifi-c2/minifi-c2-jetty/src/main/java/org/apache/nifi/minifi/c2/jetty/JettyServer.java:
##########
@@ -100,7 +113,42 @@ public static void main(String[] args) throws Exception {
server.join();
}
- private static WebAppContext loadWar(final File warFile, final String contextPath, final ClassLoader parentClassLoader) throws IOException {
+ private static SSLContext buildSSLContext(C2Properties properties) {
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ File keyStoreFile = Paths.get(C2_SERVER_HOME).resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
+ try (FileInputStream keyStoreStream = new FileInputStream(keyStoreFile)) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_TYPE))
+ .inputStream(keyStoreStream)
+ .password(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ }
+
+ File trustStoreFile = Paths.get(C2_SERVER_HOME).resolve(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE)).toFile();
+ logger.debug("trustStore path: " + trustStoreFile.getPath());
+ try (FileInputStream trustStoreStream = new FileInputStream(trustStoreFile)) {
+ truststore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE_TYPE))
+ .inputStream(trustStoreStream)
+ .password(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
Review Comment:
```suggestion
throw new UncheckedIOException("Trust Store loading failed", ioe);
```
##########
minifi/minifi-c2/minifi-c2-provider/minifi-c2-provider-util/src/main/java/org/apache/nifi/minifi/c2/provider/util/HttpConnector.java:
##########
@@ -89,6 +102,43 @@ public HttpConnector(String baseUrl, String proxyHost, int proxyPort, String pro
}
}
+ private SSLContext buildSSLContext() {
+ C2Properties properties = C2Properties.getInstance();
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ Path c2ServerHome = Paths.get(C2_SERVER_HOME);
+ File keyStoreFile = c2ServerHome.resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
+ try (FileInputStream keyStoreStream = new FileInputStream(keyStoreFile)) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_TYPE))
+ .inputStream(keyStoreStream)
+ .password(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ }
+
+ File trustStoreFile = c2ServerHome.resolve(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE)).toFile();
+ logger.debug("trustStore path: " + trustStoreFile.getPath());
Review Comment:
```suggestion
logger.debug("Loading Trust Store [{}]", trustStoreFile.getPath());
```
##########
minifi/minifi-c2/minifi-c2-provider/minifi-c2-provider-util/src/main/java/org/apache/nifi/minifi/c2/provider/util/HttpConnector.java:
##########
@@ -89,6 +102,43 @@ public HttpConnector(String baseUrl, String proxyHost, int proxyPort, String pro
}
}
+ private SSLContext buildSSLContext() {
+ C2Properties properties = C2Properties.getInstance();
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ Path c2ServerHome = Paths.get(C2_SERVER_HOME);
+ File keyStoreFile = c2ServerHome.resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
+ try (FileInputStream keyStoreStream = new FileInputStream(keyStoreFile)) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_TYPE))
+ .inputStream(keyStoreStream)
+ .password(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ }
+
+ File trustStoreFile = c2ServerHome.resolve(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE)).toFile();
+ logger.debug("trustStore path: " + trustStoreFile.getPath());
+ try (FileInputStream trustStoreStream = new FileInputStream(trustStoreFile)) {
+ truststore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE_TYPE))
+ .inputStream(trustStoreStream)
+ .password(properties.getProperty(MINIFI_C2_SERVER_TRUSTSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
Review Comment:
```suggestion
throw new UncheckedIOException("Trust Store loading failed", ioe);
```
##########
minifi/minifi-c2/minifi-c2-jetty/src/main/java/org/apache/nifi/minifi/c2/jetty/JettyServer.java:
##########
@@ -100,7 +113,42 @@ public static void main(String[] args) throws Exception {
server.join();
}
- private static WebAppContext loadWar(final File warFile, final String contextPath, final ClassLoader parentClassLoader) throws IOException {
+ private static SSLContext buildSSLContext(C2Properties properties) {
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ File keyStoreFile = Paths.get(C2_SERVER_HOME).resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
+ try (FileInputStream keyStoreStream = new FileInputStream(keyStoreFile)) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_TYPE))
+ .inputStream(keyStoreStream)
+ .password(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
Review Comment:
```suggestion
throw new UncheckedIOException("Key Store loading failed", ioe);
```
##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/configuration/ingestors/RestChangeIngestor.java:
##########
@@ -172,39 +182,53 @@ private void createConnector(Properties properties) {
http.setIdleTimeout(30000L);
jetty.addConnector(http);
- logger.info("Added an http connector on the host '{}' and port '{}'", new Object[]{http.getHost(), http.getPort()});
+ logger.info("Added an http connector on the host '{}' and port '{}'", http.getHost(), http.getPort());
}
private void createSecureConnector(Properties properties) {
- SslContextFactory ssl = new SslContextFactory();
-
- if (properties.getProperty(KEYSTORE_LOCATION_KEY) != null) {
- ssl.setKeyStorePath(properties.getProperty(KEYSTORE_LOCATION_KEY));
- ssl.setKeyStorePassword(properties.getProperty(KEYSTORE_PASSWORD_KEY));
- ssl.setKeyStoreType(properties.getProperty(KEYSTORE_TYPE_KEY));
+ KeyStore keyStore;
+ KeyStore truststore = null;
+
+ try (FileInputStream keyStoreStream = new FileInputStream(properties.getProperty(KEYSTORE_LOCATION_KEY))) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(KEYSTORE_TYPE_KEY))
+ .inputStream(keyStoreStream)
+ .password(properties.getProperty(KEYSTORE_PASSWORD_KEY).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
}
if (properties.getProperty(TRUSTSTORE_LOCATION_KEY) != null) {
- ssl.setTrustStorePath(properties.getProperty(TRUSTSTORE_LOCATION_KEY));
- ssl.setTrustStorePassword(properties.getProperty(TRUSTSTORE_PASSWORD_KEY));
- ssl.setTrustStoreType(properties.getProperty(TRUSTSTORE_TYPE_KEY));
- ssl.setNeedClientAuth(Boolean.parseBoolean(properties.getProperty(NEED_CLIENT_AUTH_KEY, "true")));
+ try (FileInputStream trustStoreStream = new FileInputStream(properties.getProperty(TRUSTSTORE_LOCATION_KEY))) {
+ truststore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(TRUSTSTORE_TYPE_KEY))
+ .inputStream(trustStoreStream)
+ .password(properties.getProperty(TRUSTSTORE_PASSWORD_KEY).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
+ }
}
- // build the connector
- final ServerConnector https = new ServerConnector(jetty, ssl);
+ SSLContext sslContext = new StandardSslContextBuilder()
+ .keyStore(keyStore)
+ .keyPassword(properties.getProperty(KEYSTORE_PASSWORD_KEY).toCharArray())
+ .trustStore(truststore)
+ .build();
- // set host and port
- https.setPort(Integer.parseInt(properties.getProperty(PORT_KEY, "0")));
- https.setHost(properties.getProperty(HOST_KEY, "localhost"));
+ StandardServerConnectorFactory serverConnectorFactory = new StandardServerConnectorFactory(jetty, Integer.parseInt(properties.getProperty(PORT_KEY, "0")));
+ serverConnectorFactory.setNeedClientAuth(Boolean.parseBoolean(properties.getProperty(NEED_CLIENT_AUTH_KEY, "true")));
+ serverConnectorFactory.setSslContext(sslContext);
+ serverConnectorFactory.setIncludeSecurityProtocols(TlsPlatform.getPreferredProtocols().toArray(new String[0]));
- // Severely taxed environments may have significant delays when executing.
- https.setIdleTimeout(30000L);
+ ServerConnector https = serverConnectorFactory.getServerConnector();
+ https.setHost(properties.getProperty(HOST_KEY, "localhost"));
// add the connector
jetty.addConnector(https);
- logger.info("Added an https connector on the host '{}' and port '{}'", new Object[]{https.getHost(), https.getPort()});
+ logger.info("Added an https connector on the host '{}' and port '{}'", https.getHost(), https.getPort());
Review Comment:
This log could probably be removed, since Jetty logs an informational message, but if kept, recommend the following adjusted wording.
```suggestion
logger.info("HTTPS Connector added for Host [{}] and Port [{}]", https.getHost(), https.getPort());
```
##########
minifi/minifi-c2/minifi-c2-provider/minifi-c2-provider-util/src/main/java/org/apache/nifi/minifi/c2/provider/util/HttpConnector.java:
##########
@@ -89,6 +102,43 @@ public HttpConnector(String baseUrl, String proxyHost, int proxyPort, String pro
}
}
+ private SSLContext buildSSLContext() {
+ C2Properties properties = C2Properties.getInstance();
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ Path c2ServerHome = Paths.get(C2_SERVER_HOME);
+ File keyStoreFile = c2ServerHome.resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
Review Comment:
```suggestion
logger.debug("Loading Key Store [{}]", keyStoreFile.getPath());
```
##########
minifi/minifi-c2/minifi-c2-jetty/src/main/java/org/apache/nifi/minifi/c2/jetty/JettyServer.java:
##########
@@ -134,7 +182,11 @@ private static WebAppContext loadWar(final File warFile, final String contextPat
// configure the max form size (3x the default)
webappContext.setMaxFormContentSize(600000);
- webappContext.setClassLoader(new WebAppClassLoader(parentClassLoader, webappContext));
+ try {
+ webappContext.setClassLoader(new WebAppClassLoader(parentClassLoader, webappContext));
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
Review Comment:
Is this try-catch necessary?
##########
minifi/minifi-c2/minifi-c2-provider/minifi-c2-provider-util/src/main/java/org/apache/nifi/minifi/c2/provider/util/HttpConnector.java:
##########
@@ -89,6 +102,43 @@ public HttpConnector(String baseUrl, String proxyHost, int proxyPort, String pro
}
}
+ private SSLContext buildSSLContext() {
+ C2Properties properties = C2Properties.getInstance();
+ KeyStore keyStore;
+ KeyStore truststore;
+
+ Path c2ServerHome = Paths.get(C2_SERVER_HOME);
+ File keyStoreFile = c2ServerHome.resolve(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE)).toFile();
+ logger.debug("keystore path: " + keyStoreFile.getPath());
+ try (FileInputStream keyStoreStream = new FileInputStream(keyStoreFile)) {
+ keyStore = new StandardKeyStoreBuilder()
+ .type(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_TYPE))
+ .inputStream(keyStoreStream)
+ .password(properties.getProperty(MINIFI_C2_SERVER_KEYSTORE_PASSWD).toCharArray())
+ .build();
+ } catch (IOException ioe) {
+ throw new UncheckedIOException(ioe);
Review Comment:
```suggestion
throw new UncheckedIOException("Key Store loading failed", ioe);
```
##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/configuration/ingestors/RestChangeIngestor.java:
##########
@@ -172,39 +182,53 @@ private void createConnector(Properties properties) {
http.setIdleTimeout(30000L);
jetty.addConnector(http);
- logger.info("Added an http connector on the host '{}' and port '{}'", new Object[]{http.getHost(), http.getPort()});
+ logger.info("Added an http connector on the host '{}' and port '{}'", http.getHost(), http.getPort());
}
private void createSecureConnector(Properties properties) {
- SslContextFactory ssl = new SslContextFactory();
-
- if (properties.getProperty(KEYSTORE_LOCATION_KEY) != null) {
- ssl.setKeyStorePath(properties.getProperty(KEYSTORE_LOCATION_KEY));
- ssl.setKeyStorePassword(properties.getProperty(KEYSTORE_PASSWORD_KEY));
- ssl.setKeyStoreType(properties.getProperty(KEYSTORE_TYPE_KEY));
+ KeyStore keyStore;
+ KeyStore truststore = null;
+
+ try (FileInputStream keyStoreStream = new FileInputStream(properties.getProperty(KEYSTORE_LOCATION_KEY))) {
Review Comment:
It could be checked for existence, but if it does not exist, `new FileInputStream()` will throw a `FileNotFoundException`, which is already caught, so this approach seems sufficient provided an error message is added to the `UncheckedIOException`.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] ferencerdei commented on pull request #7098: NIFI-11344 Make MiNiFi FIPS compatible
Posted by "ferencerdei (via GitHub)" <gi...@apache.org>.
ferencerdei commented on PR #7098:
URL: https://github.com/apache/nifi/pull/7098#issuecomment-1491457502
Thanks for the review comments. I've applied the requested changes.
As @exceptionfactory mentioned with this change, we made MiNiFi compatible with FIPS-enabled env, but it doesn't mean that we become FIPS compliant. We will need to reiterate this later and encrypt for example the bootstrap and other used config properties as well.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] ferencerdei commented on a diff in pull request #7098: NIFI-11344 Make MiNiFi FIPS compatible
Posted by "ferencerdei (via GitHub)" <gi...@apache.org>.
ferencerdei commented on code in PR #7098:
URL: https://github.com/apache/nifi/pull/7098#discussion_r1154096539
##########
minifi/minifi-c2/minifi-c2-jetty/src/main/java/org/apache/nifi/minifi/c2/jetty/JettyServer.java:
##########
@@ -134,7 +182,11 @@ private static WebAppContext loadWar(final File warFile, final String contextPat
// configure the max form size (3x the default)
webappContext.setMaxFormContentSize(600000);
- webappContext.setClassLoader(new WebAppClassLoader(parentClassLoader, webappContext));
+ try {
+ webappContext.setClassLoader(new WebAppClassLoader(parentClassLoader, webappContext));
+ } catch (IOException e) {
+ throw new UncheckedIOException(e);
+ }
Review Comment:
Yes, otherwise I couldn't call the method within lambda as it throws a checked IO exception. I'm adding an error message to the exception.
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nifi] bejancsaba commented on a diff in pull request #7098: NIFI-11344 Make MiNiFi FIPS compatible
Posted by "bejancsaba (via GitHub)" <gi...@apache.org>.
bejancsaba commented on code in PR #7098:
URL: https://github.com/apache/nifi/pull/7098#discussion_r1153333593
##########
minifi/minifi-bootstrap/src/main/java/org/apache/nifi/minifi/bootstrap/configuration/ingestors/RestChangeIngestor.java:
##########
@@ -172,39 +182,53 @@ private void createConnector(Properties properties) {
http.setIdleTimeout(30000L);
jetty.addConnector(http);
- logger.info("Added an http connector on the host '{}' and port '{}'", new Object[]{http.getHost(), http.getPort()});
+ logger.info("Added an http connector on the host '{}' and port '{}'", http.getHost(), http.getPort());
}
private void createSecureConnector(Properties properties) {
- SslContextFactory ssl = new SslContextFactory();
-
- if (properties.getProperty(KEYSTORE_LOCATION_KEY) != null) {
- ssl.setKeyStorePath(properties.getProperty(KEYSTORE_LOCATION_KEY));
- ssl.setKeyStorePassword(properties.getProperty(KEYSTORE_PASSWORD_KEY));
- ssl.setKeyStoreType(properties.getProperty(KEYSTORE_TYPE_KEY));
+ KeyStore keyStore;
+ KeyStore truststore = null;
+
+ try (FileInputStream keyStoreStream = new FileInputStream(properties.getProperty(KEYSTORE_LOCATION_KEY))) {
Review Comment:
Shouldn't it be checked whether KEYSTORE_LOCATION_KEY is set (similarly to the original code or as it is done for trust store?
##########
minifi/minifi-c2/minifi-c2-api/src/main/java/org/apache/nifi/minifi/c2/api/properties/C2Properties.java:
##########
@@ -59,31 +52,6 @@ public static C2Properties getInstance() {
}
public boolean isSecure() {
- return Boolean.valueOf(getProperty(MINIFI_C2_SERVER_SECURE, "false"));
- }
-
- public SslContextFactory getSslContextFactory() throws GeneralSecurityException, IOException {
Review Comment:
Thanks for cleaning this up. It was incorrectly placed here. After the cleanup the "logger" and "C2_SERVER_HOME" became unused could you please remove those as well?
--
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: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org