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

    https://github.com/apache/nifi/pull/3110#discussion_r228428588
  
    --- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/queue/clustered/server/ClusterLoadBalanceAuthorizer.java
 ---
    @@ -40,28 +42,23 @@ public ClusterLoadBalanceAuthorizer(final 
ClusterCoordinator clusterCoordinator,
         }
     
         @Override
    -    public void authorize(final Collection<String> clientIdentities) 
throws NotAuthorizedException {
    -        if (clientIdentities == null) {
    -            logger.debug("Client Identities is null, so assuming that Load 
Balancing communications are not secure. Authorizing client to participate in 
Load Balancing");
    -            return;
    -        }
    -
    -        final Set<String> nodeIds = 
clusterCoordinator.getNodeIdentifiers().stream()
    +    public void authorize(final SSLSession sslSession) throws 
NotAuthorizedException {
    +        final List<String> nodeIds = 
clusterCoordinator.getNodeIdentifiers().stream()
                     .map(NodeIdentifier::getApiAddress)
    -                .collect(Collectors.toSet());
    +                .collect(Collectors.toList());
     
    -        for (final String clientId : clientIdentities) {
    -            if (nodeIds.contains(clientId)) {
    -                logger.debug("Client ID '{}' is in the list of Nodes in 
the Cluster. Authorizing Client to Load Balance data", clientId);
    +        for (final String nodeId : nodeIds) {
    +            final HostnameVerifier verifier = new 
DefaultHostnameVerifier();
    +            if (verifier.verify(nodeId, sslSession)) {
    +                logger.debug("Authorizing Client to Load Balance data");
                     return;
    --- End diff --
    
    In my opinion, we just need to use HostnameVerifier to verify and use the 
hostname derived from the socket. The reason is that, anyway, HostnameVerifier 
could simply authorize a node using certs w/ or w/o wildcard, and I think the 
hostname derived from the socket is enough. If there are cases where the 
hostname derived from the socket and the hostname from Certificate Identities 
are different, please ignore my option.
    
    I'd also like to hear comment from @markap14 . Thank you.


---

Reply via email to