Uploaded image for project: 'Apache NiFi'
  1. Apache NiFi
  2. NIFI-3331

TLS Toolkit - add the possibility to define a SAN in issued certificates

    Details

    • Type: Improvement
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.1
    • Fix Version/s: 1.2.0
    • Component/s: Tools and Build
    • Labels:

      Description

      To ease the deployment of a load balancer in front of NiFi, it would be nice to allow users to define a SAN in certificates issued by the CA.

      To load balance the access to the UI or even with a ListenHTTP processor, both will cause errors with a "Host mismatch" kind of error because of different fqdn between nodes certificate and LB certificate. This is also discussed here: http://stackoverflow.com/questions/40035356/nifi-load-balancer

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user pvillard31 commented on the issue:

          https://github.com/apache/nifi/pull/1491

          OK, thanks! Will raise a JIRA and have a look. Should be straightforward.

          Show
          githubbot ASF GitHub Bot added a comment - Github user pvillard31 commented on the issue: https://github.com/apache/nifi/pull/1491 OK, thanks! Will raise a JIRA and have a look. Should be straightforward.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on the issue:

          https://github.com/apache/nifi/pull/1491

          Standalone mode still interacts with a CA (something has to sign the issued node certificates), it's just that the CA runs ephemerally on the same system as the command-line invocation as opposed to on another instance in the client/server model.

          I agree that a new Jira is sufficient to capture this feature because it is more likely someone deploying lots of nodes behind a load balancer would be using the client/server model.

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on the issue: https://github.com/apache/nifi/pull/1491 Standalone mode still interacts with a CA (something has to sign the issued node certificates), it's just that the CA runs ephemerally on the same system as the command-line invocation as opposed to on another instance in the client/server model. I agree that a new Jira is sufficient to capture this feature because it is more likely someone deploying lots of nodes behind a load balancer would be using the client/server model.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user pvillard31 commented on the issue:

          https://github.com/apache/nifi/pull/1491

          Hey @alopresto, no worries. Thanks a lot for the review and the unit tests (I was not sure how to tackle it in Groovy but looking at your code is instructive!). Out of curiosity, do you think that the feature should be added in standalone mode? When coding this PR, I felt that a user would not really need this feature without the use of a CA, but I may be missing a use case here, and this could be addressed in another JIRA.

          Show
          githubbot ASF GitHub Bot added a comment - Github user pvillard31 commented on the issue: https://github.com/apache/nifi/pull/1491 Hey @alopresto, no worries. Thanks a lot for the review and the unit tests (I was not sure how to tackle it in Groovy but looking at your code is instructive!). Out of curiosity, do you think that the feature should be added in standalone mode? When coding this PR, I felt that a user would not really need this feature without the use of a CA, but I may be missing a use case here, and this could be addressed in another JIRA.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on the issue:

          https://github.com/apache/nifi/pull/1491

          @pvillard31 Pierre, I apologize, during the rebase & merge, my author overwrote yours on the commit.

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on the issue: https://github.com/apache/nifi/pull/1491 @pvillard31 Pierre, I apologize, during the rebase & merge, my author overwrote yours on the commit.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/nifi/pull/1491

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/nifi/pull/1491
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on the issue:

          https://github.com/apache/nifi/pull/1491

          I added some unit tests for the certificate issuance with SANs and the CSR generation. I also wrote some harness code which executed the CSR generation and visually inspected it for the presence of the SANs:

          ```
          hw12203:/Users/alopresto/Workspace/scratch (master) alopresto
          đŸ”“ 8s @ 21:21:42 $ openssl req -text -noout -in csr.pem
          Certificate Request:
          Data:
          Version: 0 (0x0)
          Subject: CN=testCaHostname, OU=NIFI
          Subject Public Key Info:
          Public Key Algorithm: rsaEncryption
          Public-Key: (2048 bit)
          Modulus:
          00:95:1f:2f:f5:0e:a8:94:27:0e:3e:da:89:eb:e6:
          8a:7b:9d:54:43:03:eb:5b:dd:fc:3a:39:a3:8b:f5:
          e3:1f:f7:00:32:d5:4c:f9:55:e6:4c:04:80:97:c5:
          80:3b:92:22:a4:34:a9:3c:72:18:09:03:56:8f:18:
          74:f9:f7:5d:0a:7f:37:32:16:6b:8a:84:f3:c8:71:
          ce:1d:92:9f:e2:06:7d:bf:92:73:c8:11:d9:54:46:
          e6:3a:4f:4e:6d:90:e3:f6:ee:91:11:6a:66:0c:4c:
          1f:91:76:96:76:2e:c6:ff:35:e9:c5:1f:51:0c:cb:
          ba:5d:39:24:b6:dd:67:75:84:35:c2:a5:5e:a0:ad:
          53:13:ca:ba:67:8f:07:ef:e7:b0:63:65:09:48:d6:
          c0:77:61:c2:77:8a:b8:f1:f8:2e:1f:41:db:4f:49:
          55:ca:01:ab:4c:a7:8a:3f:2f:89:23:7c:89:01:e1:
          56:3b:a9:3a:2b:fe:e2:66:85:2a:4e:8b:9c:5f:ac:
          7c:45:d3:9b:92:3c:b5:5c:36:83:7c:71:5c:33:83:
          7d:20:e4:b5:1a:62:94:93:6a:36:5c:cc:38:63:4e:
          f6:70:58:04:04:62:bd:a5:27:a8:33:1c:c4:a6:50:
          bd:7b:a5:de:01:6d:8e:70:1b:51:ed:b3:d2:6f:e0:
          4f:f1
          Exponent: 65537 (0x10001)
          Attributes:
          Requested Extensions:
          X509v3 Subject Alternative Name:
          DNS:127.0.0.1, DNS:nifi.nifi.apache.org
          Signature Algorithm: sha256WithRSAEncryption
          2b:aa:b5:d3:a6:97:44:e2:cb:28:26:5e:6d:f6:3b:cc:66:a1:
          5b:c7:46:6d:52:30:da:99:12:a5:9e:04:d9:9c:26:17:a0:07:
          75:e6:53:80:ae:93:fc:9b:3b:f4:e9:b2:94:4e:7b:d2:89:d0:
          ab:c3:9d:03:39:c6:c9:e1:ea:0d:c6:14:72:0d:06:43:4d:64:
          a0:cb:e0:ef:58:d7:d6:69:32:7f:6b:30:1a:03:54:f6:e4:49:
          5e:29:58:d5:e3:e8:17:c2:cc:30:28:e0:4a:85:59:fe:d6:ad:
          e1:4d:62:99:52:99:49:b5:f7:54:b8:7f:eb:b6:50:c8:0d:5c:
          2f:d6:26:28:33:5c:53:b5:50:13:7f:08:5b:35:fb:ef:9a:48:
          b1:fa:fd:39:c6:9f:96:ef:99:37:bc:a8:60:13:09:1f:27:3d:
          67:41:33:dc:5d:48:b4:43:dc:69:9b:0b:93:14:6e:40:07:84:
          22:27:ee:be:6b:07:91:99:e2:20:c5:94:bd:49:d3:3b:3d:56:
          75:b8:bf:1c:bf:56:ff:42:64:04:c0:68:ed:1b:f6:fd:4f:ab:
          89:e1:4e:e0:d8:6b:f1:a2:2b:81:1a:c1:9e:41:18:b9:2c:6d:
          3f:31:c1:bc:70:2a:2a:9a:29:91:3f:d4:94:a5:65:54:2e:03:
          1d:f5:96:83
          ```

          All tests pass, contrib-check passes, +1 and merging.

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on the issue: https://github.com/apache/nifi/pull/1491 I added some unit tests for the certificate issuance with SANs and the CSR generation. I also wrote some harness code which executed the CSR generation and visually inspected it for the presence of the SANs: ``` hw12203:/Users/alopresto/Workspace/scratch (master) alopresto đŸ”“ 8s @ 21:21:42 $ openssl req -text -noout -in csr.pem Certificate Request: Data: Version: 0 (0x0) Subject: CN=testCaHostname, OU=NIFI Subject Public Key Info: Public Key Algorithm: rsaEncryption Public-Key: (2048 bit) Modulus: 00:95:1f:2f:f5:0e:a8:94:27:0e:3e:da:89:eb:e6: 8a:7b:9d:54:43:03:eb:5b:dd:fc:3a:39:a3:8b:f5: e3:1f:f7:00:32:d5:4c:f9:55:e6:4c:04:80:97:c5: 80:3b:92:22:a4:34:a9:3c:72:18:09:03:56:8f:18: 74:f9:f7:5d:0a:7f:37:32:16:6b:8a:84:f3:c8:71: ce:1d:92:9f:e2:06:7d:bf:92:73:c8:11:d9:54:46: e6:3a:4f:4e:6d:90:e3:f6:ee:91:11:6a:66:0c:4c: 1f:91:76:96:76:2e:c6:ff:35:e9:c5:1f:51:0c:cb: ba:5d:39:24:b6:dd:67:75:84:35:c2:a5:5e:a0:ad: 53:13:ca:ba:67:8f:07:ef:e7:b0:63:65:09:48:d6: c0:77:61:c2:77:8a:b8:f1:f8:2e:1f:41:db:4f:49: 55:ca:01:ab:4c:a7:8a:3f:2f:89:23:7c:89:01:e1: 56:3b:a9:3a:2b:fe:e2:66:85:2a:4e:8b:9c:5f:ac: 7c:45:d3:9b:92:3c:b5:5c:36:83:7c:71:5c:33:83: 7d:20:e4:b5:1a:62:94:93:6a:36:5c:cc:38:63:4e: f6:70:58:04:04:62:bd:a5:27:a8:33:1c:c4:a6:50: bd:7b:a5:de:01:6d:8e:70:1b:51:ed:b3:d2:6f:e0: 4f:f1 Exponent: 65537 (0x10001) Attributes: Requested Extensions: X509v3 Subject Alternative Name: DNS:127.0.0.1, DNS:nifi.nifi.apache.org Signature Algorithm: sha256WithRSAEncryption 2b:aa:b5:d3:a6:97:44:e2:cb:28:26:5e:6d:f6:3b:cc:66:a1: 5b:c7:46:6d:52:30:da:99:12:a5:9e:04:d9:9c:26:17:a0:07: 75:e6:53:80:ae:93:fc:9b:3b:f4:e9:b2:94:4e:7b:d2:89:d0: ab:c3:9d:03:39:c6:c9:e1:ea:0d:c6:14:72:0d:06:43:4d:64: a0:cb:e0:ef:58:d7:d6:69:32:7f:6b:30:1a:03:54:f6:e4:49: 5e:29:58:d5:e3:e8:17:c2:cc:30:28:e0:4a:85:59:fe:d6:ad: e1:4d:62:99:52:99:49:b5:f7:54:b8:7f:eb:b6:50:c8:0d:5c: 2f:d6:26:28:33:5c:53:b5:50:13:7f:08:5b:35:fb:ef:9a:48: b1:fa:fd:39:c6:9f:96:ef:99:37:bc:a8:60:13:09:1f:27:3d: 67:41:33:dc:5d:48:b4:43:dc:69:9b:0b:93:14:6e:40:07:84: 22:27:ee:be:6b:07:91:99:e2:20:c5:94:bd:49:d3:3b:3d:56: 75:b8:bf:1c:bf:56:ff:42:64:04:c0:68:ed:1b:f6:fd:4f:ab: 89:e1:4e:e0:d8:6b:f1:a2:2b:81:1a:c1:9e:41:18:b9:2c:6d: 3f:31:c1:bc:70:2a:2a:9a:29:91:3f:d4:94:a5:65:54:2e:03: 1d:f5:96:83 ``` All tests pass, contrib-check passes, +1 and merging.
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 6fc30900b94308b8651eee368c39e272fe8e40de in nifi's branch refs/heads/master from Andy LoPresto
          [ https://git-wip-us.apache.org/repos/asf?p=nifi.git;h=6fc3090 ]

          NIFI-3331 TLS Toolkit - add the possibility to define SAN in issued certificates.
          Added unit tests for SAN inclusion in CertificateUtils#generateIssuedCertificate() and TlsHelper#generateCertificationRequest().
          Fixed typos.

          This closes #1491.

          Signed-off-by: Andy LoPresto <alopresto@apache.org>

          Show
          jira-bot ASF subversion and git services added a comment - Commit 6fc30900b94308b8651eee368c39e272fe8e40de in nifi's branch refs/heads/master from Andy LoPresto [ https://git-wip-us.apache.org/repos/asf?p=nifi.git;h=6fc3090 ] NIFI-3331 TLS Toolkit - add the possibility to define SAN in issued certificates. Added unit tests for SAN inclusion in CertificateUtils#generateIssuedCertificate() and TlsHelper#generateCertificationRequest(). Fixed typos. This closes #1491. Signed-off-by: Andy LoPresto <alopresto@apache.org>
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on the issue:

          https://github.com/apache/nifi/pull/1491

          Reviewing...

          (fixing typos and adding tests).

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on the issue: https://github.com/apache/nifi/pull/1491 Reviewing... (fixing typos and adding tests).
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/1491#discussion_r101138569

          — Diff: nifi-toolkit/nifi-toolkit-tls/src/test/java/org/apache/nifi/toolkit/tls/service/server/TlsCertificateAuthorityServiceHandlerTest.java —
          @@ -122,7 +122,7 @@ public void setup() throws Exception {
          caCert = CertificateUtils.generateSelfSignedX509Certificate(keyPair, "CN=fakeCa", TlsConfig.DEFAULT_SIGNING_ALGORITHM, TlsConfig.DEFAULT_DAYS);
          requestedDn = new TlsConfig().calcDefaultDn(TlsConfig.DEFAULT_HOSTNAME);
          certificateKeyPair = TlsHelper.generateKeyPair(TlsConfig.DEFAULT_KEY_PAIR_ALGORITHM, TlsConfig.DEFAULT_KEY_SIZE);

          • jcaPKCS10CertificationRequest = TlsHelper.generateCertificationRequest(requestedDn, certificateKeyPair, TlsConfig.DEFAULT_SIGNING_ALGORITHM);
            + jcaPKCS10CertificationRequest = TlsHelper.generateCertificationRequest(requestedDn, null, certificateKeyPair, TlsConfig.DEFAULT_SIGNING_ALGORITHM);
              • End diff –

          There should be new tests to cover the added functionality.

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on a diff in the pull request: https://github.com/apache/nifi/pull/1491#discussion_r101138569 — Diff: nifi-toolkit/nifi-toolkit-tls/src/test/java/org/apache/nifi/toolkit/tls/service/server/TlsCertificateAuthorityServiceHandlerTest.java — @@ -122,7 +122,7 @@ public void setup() throws Exception { caCert = CertificateUtils.generateSelfSignedX509Certificate(keyPair, "CN=fakeCa", TlsConfig.DEFAULT_SIGNING_ALGORITHM, TlsConfig.DEFAULT_DAYS); requestedDn = new TlsConfig().calcDefaultDn(TlsConfig.DEFAULT_HOSTNAME); certificateKeyPair = TlsHelper.generateKeyPair(TlsConfig.DEFAULT_KEY_PAIR_ALGORITHM, TlsConfig.DEFAULT_KEY_SIZE); jcaPKCS10CertificationRequest = TlsHelper.generateCertificationRequest(requestedDn, certificateKeyPair, TlsConfig.DEFAULT_SIGNING_ALGORITHM); + jcaPKCS10CertificationRequest = TlsHelper.generateCertificationRequest(requestedDn, null, certificateKeyPair, TlsConfig.DEFAULT_SIGNING_ALGORITHM); End diff – There should be new tests to cover the added functionality.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/1491#discussion_r101138547

          — Diff: nifi-toolkit/nifi-toolkit-tls/src/test/java/org/apache/nifi/toolkit/tls/service/client/TlsCertificateSigningRequestPerformerTest.java —
          @@ -106,7 +106,7 @@ public void setup() throws GeneralSecurityException, OperatorCreationException,
          when(tlsClientConfig.getPort()).thenReturn(testPort);
          when(tlsClientConfig.createCertificateSigningRequestPerformer()).thenReturn(tlsCertificateSigningRequestPerformer);
          when(tlsClientConfig.getSigningAlgorithm()).thenReturn(TlsConfig.DEFAULT_SIGNING_ALGORITHM);

          • JcaPKCS10CertificationRequest jcaPKCS10CertificationRequest = TlsHelper.generateCertificationRequest(tlsClientConfig.getDn(), keyPair, TlsConfig.DEFAULT_SIGNING_ALGORITHM);
            + JcaPKCS10CertificationRequest jcaPKCS10CertificationRequest = TlsHelper.generateCertificationRequest(tlsClientConfig.getDn(), null, keyPair, TlsConfig.DEFAULT_SIGNING_ALGORITHM);
              • End diff –

          There should be new tests to cover the added functionality.

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on a diff in the pull request: https://github.com/apache/nifi/pull/1491#discussion_r101138547 — Diff: nifi-toolkit/nifi-toolkit-tls/src/test/java/org/apache/nifi/toolkit/tls/service/client/TlsCertificateSigningRequestPerformerTest.java — @@ -106,7 +106,7 @@ public void setup() throws GeneralSecurityException, OperatorCreationException, when(tlsClientConfig.getPort()).thenReturn(testPort); when(tlsClientConfig.createCertificateSigningRequestPerformer()).thenReturn(tlsCertificateSigningRequestPerformer); when(tlsClientConfig.getSigningAlgorithm()).thenReturn(TlsConfig.DEFAULT_SIGNING_ALGORITHM); JcaPKCS10CertificationRequest jcaPKCS10CertificationRequest = TlsHelper.generateCertificationRequest(tlsClientConfig.getDn(), keyPair, TlsConfig.DEFAULT_SIGNING_ALGORITHM); + JcaPKCS10CertificationRequest jcaPKCS10CertificationRequest = TlsHelper.generateCertificationRequest(tlsClientConfig.getDn(), null, keyPair, TlsConfig.DEFAULT_SIGNING_ALGORITHM); End diff – There should be new tests to cover the added functionality.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/1491#discussion_r101138360

          — Diff: nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/util/TlsHelper.java —
          @@ -184,8 +192,27 @@ public static KeyPair generateKeyPair(String algorithm, int keySize) throws NoSu
          return createKeyPairGenerator(algorithm, keySize).generateKeyPair();
          }

          • public static JcaPKCS10CertificationRequest generateCertificationRequest(String requestedDn, KeyPair keyPair, String signingAlgorithm) throws OperatorCreationException {
            + public static JcaPKCS10CertificationRequest generateCertificationRequest(String requestedDn, String domainAlternativeName,
            + KeyPair keyPair, String signingAlgorithm) throws OperatorCreationException {
            JcaPKCS10CertificationRequestBuilder jcaPKCS10CertificationRequestBuilder = new JcaPKCS10CertificationRequestBuilder(new X500Name(requestedDn), keyPair.getPublic());
            +
            + // add Subject Alternative Name
            + if(StringUtils.isNotBlank(domainAlternativeName)) {
              • End diff –

          Variable should be plural as it can contain multiple entries.

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on a diff in the pull request: https://github.com/apache/nifi/pull/1491#discussion_r101138360 — Diff: nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/util/TlsHelper.java — @@ -184,8 +192,27 @@ public static KeyPair generateKeyPair(String algorithm, int keySize) throws NoSu return createKeyPairGenerator(algorithm, keySize).generateKeyPair(); } public static JcaPKCS10CertificationRequest generateCertificationRequest(String requestedDn, KeyPair keyPair, String signingAlgorithm) throws OperatorCreationException { + public static JcaPKCS10CertificationRequest generateCertificationRequest(String requestedDn, String domainAlternativeName, + KeyPair keyPair, String signingAlgorithm) throws OperatorCreationException { JcaPKCS10CertificationRequestBuilder jcaPKCS10CertificationRequestBuilder = new JcaPKCS10CertificationRequestBuilder(new X500Name(requestedDn), keyPair.getPublic()); + + // add Subject Alternative Name + if(StringUtils.isNotBlank(domainAlternativeName)) { End diff – Variable should be plural as it can contain multiple entries.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/1491#discussion_r101138252

          — Diff: nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/standalone/TlsToolkitStandalone.java —
          @@ -180,7 +180,7 @@ public void createNifiKeystoresAndTrustStores(StandaloneConfig standaloneConfig)
          TlsClientManager tlsClientManager = new TlsClientManager(tlsClientConfig);
          KeyPair keyPair = TlsHelper.generateKeyPair(keyPairAlgorithm, keySize);
          tlsClientManager.addPrivateKeyToKeyStore(keyPair, NIFI_KEY, CertificateUtils.generateIssuedCertificate(tlsClientConfig.calcDefaultDn(hostname),

          • keyPair.getPublic(), certificate, caKeyPair, signingAlgorithm, days), certificate);
            + keyPair.getPublic(), null, certificate, caKeyPair, signingAlgorithm, days), certificate);
              • End diff –

          Why not allow SAN population from the standalone tool also?

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on a diff in the pull request: https://github.com/apache/nifi/pull/1491#discussion_r101138252 — Diff: nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/standalone/TlsToolkitStandalone.java — @@ -180,7 +180,7 @@ public void createNifiKeystoresAndTrustStores(StandaloneConfig standaloneConfig) TlsClientManager tlsClientManager = new TlsClientManager(tlsClientConfig); KeyPair keyPair = TlsHelper.generateKeyPair(keyPairAlgorithm, keySize); tlsClientManager.addPrivateKeyToKeyStore(keyPair, NIFI_KEY, CertificateUtils.generateIssuedCertificate(tlsClientConfig.calcDefaultDn(hostname), keyPair.getPublic(), certificate, caKeyPair, signingAlgorithm, days), certificate); + keyPair.getPublic(), null, certificate, caKeyPair, signingAlgorithm, days), certificate); End diff – Why not allow SAN population from the standalone tool also?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/1491#discussion_r101138034

          — Diff: nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/service/client/TlsCertificateAuthorityClientCommandLine.java —
          @@ -110,13 +113,18 @@ protected String getDnHostname() {
          protected CommandLine doParse(String[] args) throws CommandLineParseException

          { CommandLine commandLine = super.doParse(args); certificateDirectory = commandLine.getOptionValue(CERTIFICATE_DIRECTORY, DEFAULT_CERTIFICATE_DIRECTORY); + domaineAlternativeName = commandLine.getOptionValue(SUBJECT_ALTERNATIVE_NAME); return commandLine; }

          public String getCertificateDirectory()

          { return certificateDirectory; }

          + public String getDomaineAlternativeName() {
          — End diff –

          Same comment about typo.

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on a diff in the pull request: https://github.com/apache/nifi/pull/1491#discussion_r101138034 — Diff: nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/service/client/TlsCertificateAuthorityClientCommandLine.java — @@ -110,13 +113,18 @@ protected String getDnHostname() { protected CommandLine doParse(String[] args) throws CommandLineParseException { CommandLine commandLine = super.doParse(args); certificateDirectory = commandLine.getOptionValue(CERTIFICATE_DIRECTORY, DEFAULT_CERTIFICATE_DIRECTORY); + domaineAlternativeName = commandLine.getOptionValue(SUBJECT_ALTERNATIVE_NAME); return commandLine; } public String getCertificateDirectory() { return certificateDirectory; } + public String getDomaineAlternativeName() { — End diff – Same comment about typo.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/1491#discussion_r101137954

          — Diff: nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/service/client/TlsCertificateAuthorityClientCommandLine.java —
          @@ -41,12 +41,14 @@
          public class TlsCertificateAuthorityClientCommandLine extends BaseCertificateAuthorityCommandLine {
          public static final String DESCRIPTION = "Generates a private key and gets it signed by the certificate authority.";
          public static final String CERTIFICATE_DIRECTORY = "certificateDirectory";
          + public static final String SUBJECT_ALTERNATIVE_NAME = "subjectAlternativeName";
          public static final String DEFAULT_CERTIFICATE_DIRECTORY = ".";

          private final Logger logger = LoggerFactory.getLogger(TlsCertificateAuthorityClientCommandLine.class);
          private final InputStreamFactory inputStreamFactory;

          private String certificateDirectory;
          + private String domaineAlternativeName;
          — End diff –

          Will change with fix in `TlsConfig.java`.

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on a diff in the pull request: https://github.com/apache/nifi/pull/1491#discussion_r101137954 — Diff: nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/service/client/TlsCertificateAuthorityClientCommandLine.java — @@ -41,12 +41,14 @@ public class TlsCertificateAuthorityClientCommandLine extends BaseCertificateAuthorityCommandLine { public static final String DESCRIPTION = "Generates a private key and gets it signed by the certificate authority."; public static final String CERTIFICATE_DIRECTORY = "certificateDirectory"; + public static final String SUBJECT_ALTERNATIVE_NAME = "subjectAlternativeName"; public static final String DEFAULT_CERTIFICATE_DIRECTORY = "."; private final Logger logger = LoggerFactory.getLogger(TlsCertificateAuthorityClientCommandLine.class); private final InputStreamFactory inputStreamFactory; private String certificateDirectory; + private String domaineAlternativeName; — End diff – Will change with fix in `TlsConfig.java`.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/1491#discussion_r101137884

          — Diff: nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/service/client/TlsCertificateAuthorityClientCommandLine.java —
          @@ -41,12 +41,14 @@
          public class TlsCertificateAuthorityClientCommandLine extends BaseCertificateAuthorityCommandLine {
          public static final String DESCRIPTION = "Generates a private key and gets it signed by the certificate authority.";
          public static final String CERTIFICATE_DIRECTORY = "certificateDirectory";
          + public static final String SUBJECT_ALTERNATIVE_NAME = "subjectAlternativeName";
          — End diff –

          This should probably be plural.

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on a diff in the pull request: https://github.com/apache/nifi/pull/1491#discussion_r101137884 — Diff: nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/service/client/TlsCertificateAuthorityClientCommandLine.java — @@ -41,12 +41,14 @@ public class TlsCertificateAuthorityClientCommandLine extends BaseCertificateAuthorityCommandLine { public static final String DESCRIPTION = "Generates a private key and gets it signed by the certificate authority."; public static final String CERTIFICATE_DIRECTORY = "certificateDirectory"; + public static final String SUBJECT_ALTERNATIVE_NAME = "subjectAlternativeName"; — End diff – This should probably be plural.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/1491#discussion_r101137798

          — Diff: nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/configuration/TlsConfig.java —
          @@ -206,4 +207,12 @@ public void initDefaults()

          { dn = calcDefaultDn(caHostname); }

          }
          +
          + public String getDomaineAlternativeName() {
          — End diff –

          Typo in method & field name.

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on a diff in the pull request: https://github.com/apache/nifi/pull/1491#discussion_r101137798 — Diff: nifi-toolkit/nifi-toolkit-tls/src/main/java/org/apache/nifi/toolkit/tls/configuration/TlsConfig.java — @@ -206,4 +207,12 @@ public void initDefaults() { dn = calcDefaultDn(caHostname); } } + + public String getDomaineAlternativeName() { — End diff – Typo in method & field name.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user alopresto commented on a diff in the pull request:

          https://github.com/apache/nifi/pull/1491#discussion_r101137493

          — Diff: nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/CertificateUtils.java —
          @@ -613,6 +644,25 @@ public static boolean compareDNs(String dn1, String dn2) {
          }
          }

          + /**
          + * Extract extensions from CSR object
          + */
          + public static Extensions getExtensionsFromCSR(JcaPKCS10CertificationRequest csr) {
          + Attribute[] attributess = csr.getAttributes(PKCSObjectIdentifiers.pkcs_9_at_extensionRequest);
          — End diff –

          Typo in variable name.

          Show
          githubbot ASF GitHub Bot added a comment - Github user alopresto commented on a diff in the pull request: https://github.com/apache/nifi/pull/1491#discussion_r101137493 — Diff: nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/util/CertificateUtils.java — @@ -613,6 +644,25 @@ public static boolean compareDNs(String dn1, String dn2) { } } + /** + * Extract extensions from CSR object + */ + public static Extensions getExtensionsFromCSR(JcaPKCS10CertificationRequest csr) { + Attribute[] attributess = csr.getAttributes(PKCSObjectIdentifiers.pkcs_9_at_extensionRequest); — End diff – Typo in variable name.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user pvillard31 opened a pull request:

          https://github.com/apache/nifi/pull/1491

          NIFI-3331 - TLS Toolkit - add the possibility to define SAN in issued…

          … certificates

          =================================================================

          Issue behind this improvement is: if you have a load balancer in front of your NiFi cluster, you may have the following issue - when accessing the UI (secured cluster, HTTPS) you might face a certificate issue because the certificate issuer won't match the address of the load balancer. One option to solve this issue is to add, in addition to the node host name, the load balancer address as Subject Alternative Names in the certificate. This should help when deploying tools like Knox, HAProxy, etc, in front of NiFi with HTTPS access.

          This PR adds an option in the TLS Toolkit when using it in client mode. The option allows user to define a comma separated list of domains to add as SAN (in the example: the node and the load balancer). The domains will be added in the CSR sent to the CA and the CA will issue a signed certificate containing the same SAN.

          =================================================================

          Thank you for submitting a contribution to Apache NiFi.

          In order to streamline the review of the contribution we ask you
          to ensure the following steps have been taken:

              1. For all changes:
          • [X] Is there a JIRA ticket associated with this PR? Is it referenced
            in the commit message?
          • [X] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
          • [X] Has your PR been rebased against the latest commit within the target branch (typically master)?
          • [X] Is your initial contribution a single, squashed commit?
              1. For code changes:
          • [X] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
          • [ ] Have you written or updated unit tests to verify your changes?
          • [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
          • [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
          • [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
          • [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
              1. For documentation related changes:
          • [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
              1. Note:
                Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/pvillard31/nifi NIFI-3331

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/nifi/pull/1491.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #1491


          commit 1a1f8dedc40ea2769eb67c1fdaac06cb2e13e449
          Author: Pierre Villard <pierre.villard.fr@gmail.com>
          Date: 2017-02-09T13:47:39Z

          NIFI-3331 - TLS Toolkit - add the possibility to define SAN in issued certificates


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user pvillard31 opened a pull request: https://github.com/apache/nifi/pull/1491 NIFI-3331 - TLS Toolkit - add the possibility to define SAN in issued… … certificates ================================================================= Issue behind this improvement is: if you have a load balancer in front of your NiFi cluster, you may have the following issue - when accessing the UI (secured cluster, HTTPS) you might face a certificate issue because the certificate issuer won't match the address of the load balancer. One option to solve this issue is to add, in addition to the node host name, the load balancer address as Subject Alternative Names in the certificate. This should help when deploying tools like Knox, HAProxy, etc, in front of NiFi with HTTPS access. This PR adds an option in the TLS Toolkit when using it in client mode. The option allows user to define a comma separated list of domains to add as SAN (in the example: the node and the load balancer). The domains will be added in the CSR sent to the CA and the CA will issue a signed certificate containing the same SAN. ================================================================= Thank you for submitting a contribution to Apache NiFi. In order to streamline the review of the contribution we ask you to ensure the following steps have been taken: For all changes: [X] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message? [X] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character. [X] Has your PR been rebased against the latest commit within the target branch (typically master)? [X] Is your initial contribution a single, squashed commit? For code changes: [X] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder? [ ] Have you written or updated unit tests to verify your changes? [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0] ( http://www.apache.org/legal/resolved.html#category-a)? [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly? [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly? [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties? For documentation related changes: [ ] Have you ensured that format looks appropriate for the output in which it is rendered? Note: Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible. You can merge this pull request into a Git repository by running: $ git pull https://github.com/pvillard31/nifi NIFI-3331 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/nifi/pull/1491.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1491 commit 1a1f8dedc40ea2769eb67c1fdaac06cb2e13e449 Author: Pierre Villard <pierre.villard.fr@gmail.com> Date: 2017-02-09T13:47:39Z NIFI-3331 - TLS Toolkit - add the possibility to define SAN in issued certificates

            People

            • Assignee:
              pvillard Pierre Villard
              Reporter:
              pvillard Pierre Villard
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development