Github user ijokarumawak commented on a diff in the pull request:
https://github.com/apache/nifi/pull/3110#discussion_r231011542
--- Diff:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/queue/clustered/server/ClusterLoadBalanceAuthorizer.java
---
@@ -33,14 +42,27 @@
private final ClusterCoordinator clusterCoordinator;
private final EventReporter eventReporter;
+ private final HostnameVerifier hostnameVerifier;
public ClusterLoadBalanceAuthorizer(final ClusterCoordinator
clusterCoordinator, final EventReporter eventReporter) {
this.clusterCoordinator = clusterCoordinator;
this.eventReporter = eventReporter;
+ this.hostnameVerifier = new DefaultHostnameVerifier();
}
@Override
- public String authorize(final Collection<String> clientIdentities)
throws NotAuthorizedException {
+ public String authorize(SSLSocket sslSocket) throws
NotAuthorizedException, IOException {
+ final SSLSession sslSession = sslSocket.getSession();
+
+ final Set<String> clientIdentities;
+ try {
+ clientIdentities = getCertificateIdentities(sslSession);
+ } catch (final CertificateException e) {
+ throw new IOException("Failed to extract Client Certificate",
e);
+ }
+
+ logger.debug("Will perform authorization against Client Identities
'{}'", clientIdentities);
+
if (clientIdentities == null) {
--- End diff --
The existing log message indicating that this block is meant for the case
where NiFi clustering is not secured (not sending data via SSLSocket). This
block contradicts with the other block such as following
getCertificateIdentities method:
```
final Certificate[] certs = sslSession.getPeerCertificates();
if (certs == null || certs.length == 0) {
throw new SSLPeerUnverifiedException("No certificates found");
}
```
If we care about `clientIdentities` being null, then we should throw
`SSLPeerUnverifiedException("No client identities found");` instead of
authorizing it. Having said that, I believe removing this block is safe as
`clientIdentities` are populated using `Collectors.toSet`. If no SAN is found,
the value will be an empty set instead of null.
---
