ZOOKEEPER-4030: Optionally canonicalize host names in quorum SASL authentication

Without the option provided by this changeset, it is not convenient to use `CNAME`s to reference servers in `server.x` configuration lines while using quorum Kerberos authentication, as "incorrect" principals are generated by default.

Setting `kerberos.canonicalizeHostNames=true` causes the quorum server to substitute the canonical host name for the one specified in the configuration.  This has an effect on principal generation, and on the filtering of authorization IDs from incoming connections.

Author: Damien Diederen <dd@crosstwine.com>

Reviewers: Enrico Olivelli <eolivelli@apache.org>

Closes #1564 from ztzg/ZOOKEEPER-4030-quorum-sasl-auth-with-cnames

zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md

@@ -1705,6 +1705,14 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp
     (e.g. the zk/myhost@EXAMPLE.COM client principal will be authenticated in ZooKeeper as zk/myhost)
     Default: false
 
+* *kerberos.canonicalizeHostNames*
+    (Java system property: **zookeeper.kerberos.canonicalizeHostNames**)
+    **New in 3.7.0:**
+    Instructs ZooKeeper to canonicalize server host names extracted from *server.x* lines.
+    This allows using e.g. `CNAME` records to reference servers in configuration files, while still enabling SASL Kerberos authentication between quorum members.
+    It is essentially the quorum equivalent of the *zookeeper.sasl.client.canonicalize.hostname* property for clients.
+    The default value is **false** for backwards compatibility.
+
 * *multiAddress.enabled* :
     (Java system property: **zookeeper.multiAddress.enabled**)
     **New in 3.6.0:**

zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java

@@ -46,6 +46,7 @@ import java.util.Set;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Function;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import javax.security.sasl.SaslException;
@@ -115,6 +116,9 @@ public class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider
 
     private static final Logger LOG = LoggerFactory.getLogger(QuorumPeer.class);
 
+    public static final String CONFIG_KEY_KERBEROS_CANONICALIZE_HOST_NAMES = "zookeeper.kerberos.canonicalizeHostNames";
+    public static final String CONFIG_DEFAULT_KERBEROS_CANONICALIZE_HOST_NAMES = "false";
+
     private QuorumBean jmxQuorumBean;
     LocalPeerBean jmxLocalPeerBean;
     private Map<Long, RemotePeerBean> jmxRemotePeerBean;
@@ -265,13 +269,39 @@ public class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider
             }
         }
 
+        public QuorumServer(long sid, String addressStr) throws ConfigException {
+            this(sid, addressStr, QuorumServer::getInetAddress);
+        }
+
+        QuorumServer(long sid, String addressStr, Function<InetSocketAddress, InetAddress> getInetAddress) throws ConfigException {
+            this.id = sid;
+            initializeWithAddressString(addressStr, getInetAddress);
+        }
+
+        public QuorumServer(long id, InetSocketAddress addr, InetSocketAddress electionAddr, LearnerType type) {
+            this(id, addr, electionAddr, null, type);
+        }
+
+        public QuorumServer(long id, InetSocketAddress addr, InetSocketAddress electionAddr, InetSocketAddress clientAddr, LearnerType type) {
+            this.id = id;
+            if (addr != null) {
+                this.addr.addAddress(addr);
+            }
+            if (electionAddr != null) {
+                this.electionAddr.addAddress(electionAddr);
+            }
+            this.type = type;
+            this.clientAddr = clientAddr;
+
+            setMyAddrs();
+        }
+
         private static final String wrongFormat =
             " does not have the form server_config or server_config;client_config"
             + " where server_config is the pipe separated list of host:port:port or host:port:port:type"
             + " and client_config is port or host:port";
 
-        public QuorumServer(long sid, String addressStr) throws ConfigException {
-            this.id = sid;
+        private void initializeWithAddressString(String addressStr, Function<InetSocketAddress, InetAddress> getInetAddress) throws ConfigException {
             LearnerType newType = null;
             String[] serverClientParts = addressStr.split(";");
             String[] serverAddresses = serverClientParts[0].split("\\|");
@@ -283,9 +313,9 @@ public class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider
                 }
 
                 // is client_config a host:port or just a port
-                hostname = (clientParts.length == 2) ? clientParts[0] : "0.0.0.0";
+                String clientHostName = (clientParts.length == 2) ? clientParts[0] : "0.0.0.0";
                 try {
-                    clientAddr = new InetSocketAddress(hostname, Integer.parseInt(clientParts[clientParts.length - 1]));
+                    clientAddr = new InetSocketAddress(clientHostName, Integer.parseInt(clientParts[clientParts.length - 1]));
                 } catch (NumberFormatException e) {
                     throw new ConfigException("Address unresolved: " + hostname + ":" + clientParts[clientParts.length - 1]);
                 }
@@ -294,9 +324,14 @@ public class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider
             boolean multiAddressEnabled = Boolean.parseBoolean(
                 System.getProperty(QuorumPeer.CONFIG_KEY_MULTI_ADDRESS_ENABLED, QuorumPeer.CONFIG_DEFAULT_MULTI_ADDRESS_ENABLED));
             if (!multiAddressEnabled && serverAddresses.length > 1) {
-                throw new ConfigException("Multiple address feature is disabled, but multiple addresses were specified for sid " + sid);
+                throw new ConfigException("Multiple address feature is disabled, but multiple addresses were specified for sid " + this.id);
             }
 
+            boolean canonicalize = Boolean.parseBoolean(
+                System.getProperty(
+                    CONFIG_KEY_KERBEROS_CANONICALIZE_HOST_NAMES,
+                    CONFIG_DEFAULT_KERBEROS_CANONICALIZE_HOST_NAMES));
+
             for (String serverAddress : serverAddresses) {
                 String serverParts[] = ConfigUtils.getHostAndPort(serverAddress);
                 if ((serverClientParts.length > 2) || (serverParts.length < 3)
@@ -304,25 +339,46 @@ public class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider
                     throw new ConfigException(addressStr + wrongFormat);
                 }
 
+                String serverHostName = serverParts[0];
+
                 // server_config should be either host:port:port or host:port:port:type
                 InetSocketAddress tempAddress;
                 InetSocketAddress tempElectionAddress;
                 try {
-                    tempAddress = new InetSocketAddress(serverParts[0], Integer.parseInt(serverParts[1]));
+                    tempAddress = new InetSocketAddress(serverHostName, Integer.parseInt(serverParts[1]));
                     addr.addAddress(tempAddress);
                 } catch (NumberFormatException e) {
-                    throw new ConfigException("Address unresolved: " + serverParts[0] + ":" + serverParts[1]);
+                    throw new ConfigException("Address unresolved: " + serverHostName + ":" + serverParts[1]);
                 }
                 try {
-                    tempElectionAddress = new InetSocketAddress(serverParts[0], Integer.parseInt(serverParts[2]));
+                    tempElectionAddress = new InetSocketAddress(serverHostName, Integer.parseInt(serverParts[2]));
                     electionAddr.addAddress(tempElectionAddress);
                 } catch (NumberFormatException e) {
-                    throw new ConfigException("Address unresolved: " + serverParts[0] + ":" + serverParts[2]);
+                    throw new ConfigException("Address unresolved: " + serverHostName + ":" + serverParts[2]);
                 }
 
                 if (tempAddress.getPort() == tempElectionAddress.getPort()) {
                     throw new ConfigException("Client and election port must be different! Please update the "
-                            + "configuration file on server." + sid);
+                            + "configuration file on server." + this.id);
+                }
+
+                if (canonicalize) {
+                    InetAddress ia = getInetAddress.apply(tempAddress);
+                    if (ia == null) {
+                        throw new ConfigException("Unable to canonicalize address " + serverHostName + " because it's not resolvable");
+                    }
+
+                    String canonicalHostName = ia.getCanonicalHostName();
+
+                    if (!canonicalHostName.equals(serverHostName)
+                        // Avoid using literal IP address when
+                        // security check fails
+                        && !canonicalHostName.equals(ia.getHostAddress())) {
+                        LOG.info("Host name for quorum server {} "
+                            + "canonicalized from {} to {}",
+                            this.id, serverHostName, canonicalHostName);
+                        serverHostName = canonicalHostName;
+                    }
                 }
 
                 if (serverParts.length == 4) {
@@ -336,7 +392,7 @@ public class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider
                     }
                 }
 
-                this.hostname = serverParts[0];
+                this.hostname = serverHostName;
             }
 
             if (newType != null) {
@@ -346,22 +402,8 @@ public class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider
             setMyAddrs();
         }
 
-        public QuorumServer(long id, InetSocketAddress addr, InetSocketAddress electionAddr, LearnerType type) {
-            this(id, addr, electionAddr, null, type);
-        }
-
-        public QuorumServer(long id, InetSocketAddress addr, InetSocketAddress electionAddr, InetSocketAddress clientAddr, LearnerType type) {
-            this.id = id;
-            if (addr != null) {
-                this.addr.addAddress(addr);
-            }
-            if (electionAddr != null) {
-                this.electionAddr.addAddress(electionAddr);
-            }
-            this.type = type;
-            this.clientAddr = clientAddr;
-
-            setMyAddrs();
+        private static InetAddress getInetAddress(InetSocketAddress addr) {
+            return addr.getAddress();
         }
 
         private void setMyAddrs() {

zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumCanonicalizeTest.java

@@ -0,0 +1,102 @@
+/*
+ * 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.
+ */
+
+package org.apache.zookeeper.server.quorum;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+
+public class QuorumCanonicalizeTest extends ZKTestCase {
+
+    private static final InetSocketAddress SA_DONT_CARE = InetSocketAddress.createUnresolved("dont.care.invalid", 80);
+
+    private static final String ZK1_ALIAS = "zookeeper.invalid";
+    private static final String ZK1_FQDN = "zk1.invalid";
+    private static final String ZK1_IP = "169.254.0.42";
+    private static final InetAddress IA_MOCK_ZK1;
+
+    static {
+        InetAddress ia = mock(InetAddress.class);
+
+        when(ia.getCanonicalHostName()).thenReturn(ZK1_FQDN);
+
+        IA_MOCK_ZK1 = ia;
+    }
+
+    private static InetAddress getInetAddress(InetSocketAddress addr) {
+        if (addr.getHostName().equals(ZK1_ALIAS) || addr.getHostName().equals(ZK1_IP)) {
+            return IA_MOCK_ZK1;
+        }
+
+        return addr.getAddress();
+    };
+
+    @AfterEach
+    public void cleanUpEnvironment() {
+        System.clearProperty(QuorumPeer.CONFIG_KEY_KERBEROS_CANONICALIZE_HOST_NAMES);
+    }
+
+    private QuorumPeer.QuorumServer createQuorumServer(String hostName) throws ConfigException {
+        return new QuorumPeer.QuorumServer(0, hostName + ":1234:5678", QuorumCanonicalizeTest::getInetAddress);
+    }
+
+    @Test
+    public void testQuorumDefaultCanonicalization() throws ConfigException {
+        QuorumPeer.QuorumServer qps = createQuorumServer(ZK1_ALIAS);
+
+        assertEquals(ZK1_ALIAS, qps.hostname,
+           "The host name has been \"changed\" (canonicalized?) despite default settings");
+    }
+
+    @Test
+    public void testQuorumNoCanonicalization() throws ConfigException {
+        System.setProperty(QuorumPeer.CONFIG_KEY_KERBEROS_CANONICALIZE_HOST_NAMES, Boolean.FALSE.toString());
+
+        QuorumPeer.QuorumServer qps = createQuorumServer(ZK1_ALIAS);
+
+        assertEquals(ZK1_ALIAS, qps.hostname,
+           "The host name has been \"changed\" (canonicalized?) despite default settings");
+    }
+
+    @Test
+    public void testQuorumCanonicalization() throws ConfigException {
+        System.setProperty(QuorumPeer.CONFIG_KEY_KERBEROS_CANONICALIZE_HOST_NAMES, Boolean.TRUE.toString());
+
+        QuorumPeer.QuorumServer qps = createQuorumServer(ZK1_ALIAS);
+
+        assertEquals(ZK1_FQDN, qps.hostname,
+            "The host name hasn't been correctly canonicalized");
+    }
+
+    @Test
+    public void testQuorumCanonicalizationFromIp() throws ConfigException {
+        System.setProperty(QuorumPeer.CONFIG_KEY_KERBEROS_CANONICALIZE_HOST_NAMES, Boolean.TRUE.toString());
+
+        QuorumPeer.QuorumServer qps = createQuorumServer(ZK1_IP);
+
+        assertEquals(ZK1_FQDN, qps.hostname,
+            "The host name hasn't been correctly canonicalized");
+    }
+}