[GitHub] [nifi] alopresto commented on a change in pull request #3340: NIFI-6026 - First commit which adds a new tls-toolkit mode called Key…
alopresto commented on a change in pull request #3340: NIFI-6026 - First commit
which adds a new tls-toolkit mode called Key…
URL: https://github.com/apache/nifi/pull/3340#discussion_r273283281
##
File path:
nifi-toolkit/nifi-toolkit-tls/src/test/java/org/apache/nifi/toolkit/tls/util/TlsHelperTest.java
##
@@ -448,4 +460,58 @@ public void testClientDnFilenameSpecialChars() throws
Exception {
assertEquals("CN=testuser_OU=NiFi_Organisation", escapedClientDn);
}
+private KeyStore setupKeystore() throws Exception {
+
+KeyStore ks = KeyStore.getInstance("JKS");
+InputStream readStream =
getClass().getClassLoader().getResourceAsStream("keystore.jks");
+ks.load(readStream, "changeit".toCharArray());
+
+return ks;
+}
+
+@Test
+public void testOutputCertsAsPem() throws Exception {
+File folder = tempFolder.newFolder("splitKeystoreOutputDir");
+KeyStore keyStore = setupKeystore();
+HashMap certs = TlsHelper.extractCerts(keyStore);
+TlsHelper.outputCertsAsPem(certs, folder,".crt");
+
+for(File file : folder.listFiles()) {
+BufferedReader br = new BufferedReader(new FileReader(file));
+PEMParser pemParser = new PEMParser(br);
+X509CertificateHolder key = (X509CertificateHolder)
pemParser.readObject();
+assertNotNull(key.getSignature());
+}
+}
+
+@Test
+public void testOutputKeysAsPem() throws Exception {
+File folder = tempFolder.newFolder("splitKeystoreOutputDir");
+KeyStore keyStore = setupKeystore();
+HashMap keys = TlsHelper.extractKeys(keyStore,
"changeit".toCharArray());
+TlsHelper.outputKeysAsPem(keys, folder, ".key");
+
+for(File file : folder.listFiles()) {
+BufferedReader br = new BufferedReader(new FileReader(file));
+PEMParser pemParser = new PEMParser(br);
+PEMKeyPair key = (PEMKeyPair) pemParser.readObject();
+assertNotNull(key.getPrivateKeyInfo());
+}
+}
+
+@Test
+public void testExtractCerts() throws Exception {
+KeyStore keyStore = setupKeystore();
+HashMap certs = TlsHelper.extractCerts(keyStore);
+assertEquals(2, certs.size());
+certs.forEach((String p, Certificate q) -> assertEquals("X.509",
q.getType()));
+}
+
+@Test
+public void testExtractKeys() throws Exception {
+KeyStore keyStore = setupKeystore();
+HashMap keys = TlsHelper.extractKeys(keyStore,
"changeit".toCharArray());
+assertEquals(1, keys.size());
+keys.forEach((String alias, Key key) -> assertEquals("PKCS#8",
key.getFormat()));
+}
Review comment:
I would like to see test cases documenting the following use cases:
* Run with no keystore provided
* Run with no keystore password provided
* Run with wrong keystore password provided
* Run with keystore with no aliases
* Run with keystore with multiple aliases
* Run with correct keystore password but wrong key password provided
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi] alopresto commented on a change in pull request #3340: NIFI-6026 - First commit which adds a new tls-toolkit mode called Key…
alopresto commented on a change in pull request #3340: NIFI-6026 - First commit
which adds a new tls-toolkit mode called Key…
URL: https://github.com/apache/nifi/pull/3340#discussion_r273283033
##
File path:
nifi-toolkit/nifi-toolkit-tls/src/test/java/org/apache/nifi/toolkit/tls/util/TlsHelperTest.java
##
@@ -448,4 +460,58 @@ public void testClientDnFilenameSpecialChars() throws
Exception {
assertEquals("CN=testuser_OU=NiFi_Organisation", escapedClientDn);
}
+private KeyStore setupKeystore() throws Exception {
+
+KeyStore ks = KeyStore.getInstance("JKS");
+InputStream readStream =
getClass().getClassLoader().getResourceAsStream("keystore.jks");
+ks.load(readStream, "changeit".toCharArray());
+
+return ks;
+}
+
+@Test
+public void testOutputCertsAsPem() throws Exception {
Review comment:
I'd prefer if these tests were more descriptive -- list the expectations for
what is happening here. The keystore should have a known number of entries, and
that number should match the number of outputs. In addition, the key signature
should be compared with that queried from the keystore directly, not just
asserted that it is non-null. The same goes for the key assertions in the
subsequent test.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [nifi] alopresto commented on a change in pull request #3340: NIFI-6026 - First commit which adds a new tls-toolkit mode called Key…
alopresto commented on a change in pull request #3340: NIFI-6026 - First commit
which adds a new tls-toolkit mode called Key…
URL: https://github.com/apache/nifi/pull/3340#discussion_r273279936
##
File path:
nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/standalone/TlsToolkitStandaloneCommandLine.java
##
@@ -113,6 +118,8 @@ protected TlsToolkitStandaloneCommandLine(PasswordUtil
passwordUtil) {
addOptionWithArg(null, NIFI_DN_SUFFIX_ARG, "String to append to
hostname(s) when determining DN.", TlsConfig.DEFAULT_DN_SUFFIX);
addOptionNoArg("O", OVERWRITE_ARG, "Overwrite existing host output.");
addOptionWithArg(null, ADDITIONAL_CA_CERTIFICATE_ARG, "Path to
additional CA certificate (used to sign toolkit CA certificate) in PEM format
if necessary");
+addOptionWithArg("splitKeystore", SPLIT_KEYSTORE_ARG, "Split out a
given keystore into its unencrypted key and certificates. Use -S and -K to
specify the keystore and key passwords.");
Review comment:
Providing the "splitKeystore" string as the first argument to this call
means that `-splitKeystore` and `--splitKeystore` are both supported here.
Please pass `null` as the first argument instead, otherwise this leads to
ambiguity and breaks consistency.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
