Skip to content

Commit e667dc7

Browse files
AsmodayAsmoday
authored
Merge pull request #3 from arenadata/feature/ZOOKEEPER-3824-3.9.5
feat(ZOOKEEPER-3824): allow SASL allowlist expansion during reconfig
2 parents 93f6326 + ee5ec50 commit e667dc7

7 files changed

Lines changed: 514 additions & 5 deletions

File tree

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

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import java.util.HashSet;
4040
import java.util.LinkedList;
4141
import java.util.List;
42+
import java.util.Locale;
4243
import java.util.Map;
4344
import java.util.Map.Entry;
4445
import java.util.Properties;
@@ -793,6 +794,11 @@ public synchronized void setCurrentVote(Vote v) {
793794
*/
794795
protected int quorumCnxnThreadsSize = QUORUM_CNXN_THREADS_SIZE_DEFAULT_VALUE;
795796

797+
private boolean quorumSaslAuthzZnodeEnabled = false;
798+
private String quorumSaslAuthzZnodePath = "";
799+
private final AtomicReference<Set<String>> manualSaslAuthzHosts =
800+
new AtomicReference<>(Collections.emptySet());
801+
796802
public static final String QUORUM_CNXN_TIMEOUT_MS = "zookeeper.quorumCnxnTimeoutMs";
797803
private static int quorumCnxnTimeoutMs;
798804

@@ -1903,6 +1909,40 @@ private void connectNewPeers(QuorumCnxManager qcm) {
19031909
}
19041910
}
19051911

1912+
public void refreshQuorumSaslAuthzHosts(QuorumVerifier... extraQVs) {
1913+
if (!(authServer instanceof SaslQuorumAuthServer)) {
1914+
return;
1915+
}
1916+
1917+
Set<String> hosts = new HashSet<>();
1918+
synchronized (QV_LOCK) {
1919+
addHostsFromQV(quorumVerifier, hosts);
1920+
addHostsFromQV(lastSeenQuorumVerifier, hosts);
1921+
}
1922+
Set<String> manualHosts = manualSaslAuthzHosts.get();
1923+
if (manualHosts != null) {
1924+
hosts.addAll(manualHosts);
1925+
}
1926+
if (extraQVs != null) {
1927+
for (QuorumVerifier qv : extraQVs) {
1928+
addHostsFromQV(qv, hosts);
1929+
}
1930+
}
1931+
1932+
((SaslQuorumAuthServer) authServer).updateAuthorizedHosts(hosts);
1933+
}
1934+
1935+
private static void addHostsFromQV(QuorumVerifier qv, Set<String> hosts) {
1936+
if (qv == null) {
1937+
return;
1938+
}
1939+
for (QuorumServer qs : qv.getAllMembers().values()) {
1940+
if (qs != null && qs.hostname != null) {
1941+
hosts.add(qs.hostname);
1942+
}
1943+
}
1944+
}
1945+
19061946
public void setLastSeenQuorumVerifier(QuorumVerifier qv, boolean writeToDisk) {
19071947
if (!isReconfigEnabled()) {
19081948
LOG.info("Dynamic reconfig is disabled, we don't store the last seen config.");
@@ -1932,6 +1972,7 @@ public void setLastSeenQuorumVerifier(QuorumVerifier qv, boolean writeToDisk) {
19321972
return;
19331973
}
19341974
lastSeenQuorumVerifier = qv;
1975+
refreshQuorumSaslAuthzHosts();
19351976
if (qcm != null) {
19361977
connectNewPeers(qcm);
19371978
}
@@ -1991,6 +2032,7 @@ public QuorumVerifier setQuorumVerifier(QuorumVerifier qv, boolean writeToDisk)
19912032
setAddrs(qs.addr, qs.electionAddr, qs.clientAddr);
19922033
}
19932034
updateObserverMasterList();
2035+
refreshQuorumSaslAuthzHosts();
19942036
return prevQV;
19952037
}
19962038
}
@@ -2597,6 +2639,55 @@ void setQuorumCnxnThreadsSize(int qCnxnThreadsSize) {
25972639
LOG.info("quorum.cnxn.threads.size set to {}", quorumCnxnThreadsSize);
25982640
}
25992641

2642+
void setQuorumSaslAuthzZnodeEnabled(boolean enabled) {
2643+
quorumSaslAuthzZnodeEnabled = enabled;
2644+
}
2645+
2646+
boolean isQuorumSaslAuthzZnodeEnabled() {
2647+
return quorumSaslAuthzZnodeEnabled;
2648+
}
2649+
2650+
void setQuorumSaslAuthzZnodePath(String path) {
2651+
if (path == null) {
2652+
return;
2653+
}
2654+
quorumSaslAuthzZnodePath = path.trim();
2655+
}
2656+
2657+
String getQuorumSaslAuthzZnodePath() {
2658+
return quorumSaslAuthzZnodePath;
2659+
}
2660+
2661+
void setManualSaslAuthzHosts(String hostsCsv) {
2662+
manualSaslAuthzHosts.set(parseAuthzHosts(hostsCsv));
2663+
}
2664+
2665+
void clearManualSaslAuthzHosts() {
2666+
manualSaslAuthzHosts.set(Collections.emptySet());
2667+
}
2668+
2669+
// VisibleForTesting
2670+
Set<String> getManualSaslAuthzHosts() {
2671+
return manualSaslAuthzHosts.get();
2672+
}
2673+
2674+
private static Set<String> parseAuthzHosts(String hostsCsv) {
2675+
if (hostsCsv == null) {
2676+
return Collections.emptySet();
2677+
}
2678+
String trimmed = hostsCsv.trim();
2679+
if (trimmed.isEmpty()) {
2680+
return Collections.emptySet();
2681+
}
2682+
Set<String> hosts = new HashSet<>();
2683+
for (String token : trimmed.split("[,\\s]+")) {
2684+
if (!token.isEmpty()) {
2685+
hosts.add(token.toLowerCase(Locale.ROOT));
2686+
}
2687+
}
2688+
return Collections.unmodifiableSet(hosts);
2689+
}
2690+
26002691
boolean isQuorumSaslAuthEnabled() {
26012692
return quorumSaslEnableAuth;
26022693
}
@@ -2700,6 +2791,8 @@ public static QuorumPeer createFromConfig(QuorumPeerConfig config) throws IOExce
27002791
quorumPeer.setQuorumLearnerLoginContext(config.quorumLearnerLoginContext);
27012792
}
27022793
quorumPeer.setQuorumCnxnThreadsSize(config.quorumCnxnThreadsSize);
2794+
quorumPeer.setQuorumSaslAuthzZnodeEnabled(config.quorumSaslAuthzZnodeEnabled);
2795+
quorumPeer.setQuorumSaslAuthzZnodePath(config.quorumSaslAuthzZnodePath);
27032796

27042797
if (config.jvmPauseMonitorToRun) {
27052798
quorumPeer.setJvmPauseMonitor(new JvmPauseMonitor(config));

‎zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ public class QuorumPeerConfig {
121121
protected String quorumLearnerLoginContext = QuorumAuth.QUORUM_LEARNER_SASL_LOGIN_CONTEXT_DFAULT_VALUE;
122122
protected String quorumServerLoginContext = QuorumAuth.QUORUM_SERVER_SASL_LOGIN_CONTEXT_DFAULT_VALUE;
123123
protected int quorumCnxnThreadsSize;
124+
protected boolean quorumSaslAuthzZnodeEnabled = false;
125+
protected String quorumSaslAuthzZnodePath = "";
124126

125127
// multi address related configs
126128
private boolean multiAddressEnabled = Boolean.parseBoolean(
@@ -362,6 +364,10 @@ public void parseProperties(Properties zkProp) throws IOException, ConfigExcepti
362364
quorumServerLoginContext = value;
363365
} else if (key.equals(QuorumAuth.QUORUM_KERBEROS_SERVICE_PRINCIPAL)) {
364366
quorumServicePrincipal = value;
367+
} else if (key.equals(QuorumAuth.QUORUM_SASL_AUTHZ_ZNODE_ENABLED)) {
368+
quorumSaslAuthzZnodeEnabled = parseBoolean(key, value);
369+
} else if (key.equals(QuorumAuth.QUORUM_SASL_AUTHZ_ZNODE_PATH)) {
370+
quorumSaslAuthzZnodePath = value;
365371
} else if (key.equals("quorum.cnxn.threads.size")) {
366372
quorumCnxnThreadsSize = Integer.parseInt(value);
367373
} else if (key.equals(JvmPauseMonitor.INFO_THRESHOLD_KEY)) {
@@ -390,6 +396,13 @@ public void parseProperties(Properties zkProp) throws IOException, ConfigExcepti
390396
}
391397
}
392398

399+
if (quorumSaslAuthzZnodeEnabled && quorumSaslAuthzZnodePath.trim().isEmpty()) {
400+
quorumSaslAuthzZnodePath = QuorumAuth.QUORUM_SASL_AUTHZ_ZNODE_DEFAULT_PATH;
401+
}
402+
if (!quorumSaslAuthzZnodeEnabled && !quorumSaslAuthzZnodePath.trim().isEmpty()) {
403+
quorumSaslAuthzZnodeEnabled = true;
404+
}
405+
393406
if (!quorumEnableSasl && quorumServerRequireSasl) {
394407
throw new IllegalArgumentException(QuorumAuth.QUORUM_SASL_AUTH_ENABLED
395408
+ " is disabled, so cannot enable "

‎zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,16 +20,23 @@
2020

2121
import java.io.IOException;
2222
import java.io.PrintWriter;
23+
import java.nio.charset.StandardCharsets;
24+
import java.util.List;
2325
import java.util.Objects;
26+
import java.util.concurrent.atomic.AtomicBoolean;
2427
import java.util.function.BiConsumer;
2528
import java.util.stream.Collectors;
2629
import org.apache.zookeeper.CreateMode;
2730
import org.apache.zookeeper.KeeperException;
2831
import org.apache.zookeeper.MultiOperationRecord;
2932
import org.apache.zookeeper.Op;
33+
import org.apache.zookeeper.ZooDefs;
3034
import org.apache.zookeeper.ZooDefs.OpCode;
35+
import org.apache.zookeeper.data.ACL;
36+
import org.apache.zookeeper.data.Stat;
3137
import org.apache.zookeeper.metrics.MetricsContext;
3238
import org.apache.zookeeper.proto.CreateRequest;
39+
import org.apache.zookeeper.server.DataTree.ProcessTxnResult;
3340
import org.apache.zookeeper.server.Request;
3441
import org.apache.zookeeper.server.RequestRecord;
3542
import org.apache.zookeeper.server.ServerMetrics;
@@ -46,6 +53,7 @@ public abstract class QuorumZooKeeperServer extends ZooKeeperServer {
4653

4754
public final QuorumPeer self;
4855
protected UpgradeableSessionTracker upgradeableSessionTracker;
56+
private final AtomicBoolean authzAclWarningLogged = new AtomicBoolean(false);
4957

5058
protected QuorumZooKeeperServer(FileTxnSnapLog logFactory, int tickTime, int minSessionTimeout,
5159
int maxSessionTimeout, int listenBacklog, ZKDatabase zkDb, QuorumPeer self) {
@@ -54,6 +62,12 @@ protected QuorumZooKeeperServer(FileTxnSnapLog logFactory, int tickTime, int min
5462
this.self = self;
5563
}
5664

65+
@Override
66+
public synchronized void startup() {
67+
super.startup();
68+
refreshAuthzHostsFromZnode();
69+
}
70+
5771
@Override
5872
protected void startSessionTracker() {
5973
upgradeableSessionTracker = (UpgradeableSessionTracker) sessionTracker;
@@ -213,4 +227,122 @@ public void dumpMonitorValues(BiConsumer<String, Object> response) {
213227
response.accept("peer_state", self.getDetailedPeerState());
214228
}
215229

230+
@Override
231+
public ProcessTxnResult processTxn(Request request) {
232+
ProcessTxnResult rc = super.processTxn(request);
233+
maybeRefreshAuthzHostsFromTxn(rc);
234+
return rc;
235+
}
236+
237+
private void maybeRefreshAuthzHostsFromTxn(ProcessTxnResult rc) {
238+
if (rc == null) {
239+
return;
240+
}
241+
if (!self.isQuorumSaslAuthEnabled() || !self.isQuorumSaslAuthzZnodeEnabled()) {
242+
return;
243+
}
244+
String znodePath = self.getQuorumSaslAuthzZnodePath();
245+
if (znodePath == null || znodePath.isEmpty()) {
246+
return;
247+
}
248+
249+
if (rc.multiResult != null) {
250+
for (ProcessTxnResult sub : rc.multiResult) {
251+
if (isAuthzZnodeTxn(sub, znodePath)) {
252+
if (sub.type == OpCode.delete) {
253+
clearAuthzHostsFromZnode();
254+
} else {
255+
refreshAuthzHostsFromZnode();
256+
}
257+
return;
258+
}
259+
}
260+
return;
261+
}
262+
263+
if (isAuthzZnodeTxn(rc, znodePath)) {
264+
LOG.info("Authz znode txn applied: type={}, path={}", rc.type, rc.path);
265+
if (rc.type == OpCode.delete) {
266+
clearAuthzHostsFromZnode();
267+
} else {
268+
refreshAuthzHostsFromZnode();
269+
}
270+
}
271+
}
272+
273+
private static boolean isAuthzZnodeTxn(ProcessTxnResult rc, String znodePath) {
274+
if (rc == null || rc.path == null) {
275+
return false;
276+
}
277+
if (!znodePath.equals(rc.path)) {
278+
return false;
279+
}
280+
return rc.type == OpCode.create
281+
|| rc.type == OpCode.create2
282+
|| rc.type == OpCode.createContainer
283+
|| rc.type == OpCode.createTTL
284+
|| rc.type == OpCode.setData
285+
|| rc.type == OpCode.delete;
286+
}
287+
288+
private void refreshAuthzHostsFromZnode() {
289+
if (!self.isQuorumSaslAuthEnabled() || !self.isQuorumSaslAuthzZnodeEnabled()) {
290+
return;
291+
}
292+
String path = self.getQuorumSaslAuthzZnodePath();
293+
if (path == null || path.isEmpty()) {
294+
return;
295+
}
296+
try {
297+
if (hasWorldWritableAcl(path) && authzAclWarningLogged.compareAndSet(false, true)) {
298+
LOG.warn("Authz znode {} has world-writable ACL. "
299+
+ "This is a security risk: any client can modify the quorum authorization host list. "
300+
+ "Set restrictive ACLs on this znode.", path);
301+
}
302+
byte[] data = getZKDatabase().getDataTree().getData(path, new Stat(), null);
303+
if (data == null) {
304+
LOG.info("Authz znode read returned null data for {}", path);
305+
return;
306+
}
307+
if (data.length == 0) {
308+
LOG.info("Authz znode read returned empty data for {}", path);
309+
self.clearManualSaslAuthzHosts();
310+
} else {
311+
LOG.info("Authz znode read {} bytes for {}", data.length, path);
312+
self.setManualSaslAuthzHosts(new String(data, StandardCharsets.UTF_8));
313+
}
314+
} catch (KeeperException.NoNodeException e) {
315+
LOG.info("Authz znode missing at {}", path);
316+
return;
317+
} catch (Exception e) {
318+
LOG.warn("Failed to refresh quorum SASL authz hosts from znode {}", path, e);
319+
return;
320+
}
321+
322+
self.refreshQuorumSaslAuthzHosts();
323+
}
324+
325+
private boolean hasWorldWritableAcl(String path) {
326+
try {
327+
List<ACL> acls = getZKDatabase().getDataTree().getACL(path, new Stat());
328+
for (ACL acl : acls) {
329+
if (ZooDefs.Ids.ANYONE_ID_UNSAFE.equals(acl.getId())
330+
&& (acl.getPerms() & ZooDefs.Perms.WRITE) != 0) {
331+
return true;
332+
}
333+
}
334+
} catch (KeeperException.NoNodeException e) {
335+
// node doesn't exist yet
336+
}
337+
return false;
338+
}
339+
340+
private void clearAuthzHostsFromZnode() {
341+
if (!self.isQuorumSaslAuthEnabled() || !self.isQuorumSaslAuthzZnodeEnabled()) {
342+
return;
343+
}
344+
self.clearManualSaslAuthzHosts();
345+
self.refreshQuorumSaslAuthzHosts();
346+
}
347+
216348
}

‎zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/auth/QuorumAuth.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ public class QuorumAuth {
4242
public static final String QUORUM_SERVER_SASL_LOGIN_CONTEXT = "quorum.auth.server.saslLoginContext";
4343
public static final String QUORUM_SERVER_SASL_LOGIN_CONTEXT_DFAULT_VALUE = "QuorumServer";
4444

45+
public static final String QUORUM_SASL_AUTHZ_ZNODE_ENABLED = "quorum.auth.sasl.authzZnode.enabled";
46+
public static final String QUORUM_SASL_AUTHZ_ZNODE_PATH = "quorum.auth.sasl.authzZnode.path";
47+
public static final String QUORUM_SASL_AUTHZ_ZNODE_DEFAULT_PATH = "/zookeeper/quorumAuthzHosts";
48+
4549
static final String QUORUM_SERVER_PROTOCOL_NAME = "zookeeper-quorum";
4650
static final String QUORUM_SERVER_SASL_DIGEST = "zk-quorum-sasl-md5";
4751
static final String QUORUM_AUTH_MESSAGE_TAG = "qpconnect";

‎zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/auth/SaslQuorumAuthServer.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@
2323
import java.io.DataOutputStream;
2424
import java.io.IOException;
2525
import java.net.Socket;
26+
import java.util.Collections;
2627
import java.util.Set;
28+
import java.util.concurrent.atomic.AtomicReference;
2729
import java.util.function.Supplier;
2830
import javax.security.auth.callback.CallbackHandler;
2931
import javax.security.auth.login.AppConfigurationEntry;
@@ -47,9 +49,11 @@ public class SaslQuorumAuthServer implements QuorumAuthServer {
4749
private static final int MAX_RETRIES = 5;
4850
private final Login serverLogin;
4951
private final boolean quorumRequireSasl;
52+
private final AtomicReference<Set<String>> currentAuthzHosts;
5053

5154
public SaslQuorumAuthServer(boolean quorumRequireSasl, String loginContext, Set<String> authzHosts) throws SaslException {
5255
this.quorumRequireSasl = quorumRequireSasl;
56+
this.currentAuthzHosts = new AtomicReference<>(authzHosts != null ? authzHosts : Collections.emptySet());
5357
try {
5458
AppConfigurationEntry[] entries = Configuration.getConfiguration().getAppConfigurationEntry(loginContext);
5559
if (entries == null || entries.length == 0) {
@@ -58,7 +62,7 @@ public SaslQuorumAuthServer(boolean quorumRequireSasl, String loginContext, Set<
5862
loginContext));
5963
}
6064
Supplier<CallbackHandler> callbackSupplier = () -> {
61-
return new SaslQuorumServerCallbackHandler(entries, authzHosts);
65+
return new SaslQuorumServerCallbackHandler(entries, currentAuthzHosts.get());
6266
};
6367
serverLogin = new Login(loginContext, callbackSupplier, new ZKConfig());
6468
serverLogin.startThreadIfNeeded();
@@ -67,6 +71,12 @@ public SaslQuorumAuthServer(boolean quorumRequireSasl, String loginContext, Set<
6771
}
6872
}
6973

74+
public void updateAuthorizedHosts(Set<String> authzHosts) {
75+
if (authzHosts != null) {
76+
currentAuthzHosts.set(authzHosts);
77+
}
78+
}
79+
7080
@Override
7181
public void authenticate(Socket sock, DataInputStream din) throws SaslException {
7282
DataOutputStream dout = null;

0 commit comments

Comments
 (0)