-
Notifications
You must be signed in to change notification settings - Fork 7.3k
ZOOKEEPER-4849: Option to Provide Custom X509 Implementation of QuorumAuthServer and QuorumAuthLearner #2269
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,6 +77,9 @@ public class QuorumPeerConfig { | |
protected boolean shouldUsePortUnification = false; | ||
protected int observerMasterPort; | ||
protected boolean sslQuorumReloadCertFiles = false; | ||
private String sslAuthServerProvider; | ||
private String sslAuthLearnerProvider; | ||
|
||
protected File dataDir; | ||
protected File dataLogDir; | ||
protected String dynamicConfigFileStr = null; | ||
|
@@ -390,6 +393,10 @@ public void parseProperties(Properties zkProp) throws IOException, ConfigExcepti | |
multiAddressReachabilityCheckEnabled = parseBoolean(key, value); | ||
} else if (key.equals("oraclePath")) { | ||
oraclePath = value; | ||
} else if (key.equals(QuorumAuth.QUORUM_SSL_AUTHPROVIDER)) { | ||
sslAuthServerProvider = value; | ||
} else if (key.equals(QuorumAuth.QUORUM_SSL_LEARNER_AUTHPROVIDER)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No java system properties ? I found that most other ssl config options supports both, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I’ve always wondered why we need to pull the server-side settings from JVM system properties when they’re almost always defined in zoo.cfg. Supporting both adds needless complexity. It makes sense to allow client-side overrides via –D flags, but reading the server properties that way isn’t strictly necessary. In any event, I’ll go ahead and add the system-property support for completeness. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I guess ZooKeeper server was designed to own/occupy jvm. You may have to delete these tow options from |
||
sslAuthLearnerProvider = value; | ||
} else { | ||
System.setProperty("zookeeper." + key, value); | ||
} | ||
|
@@ -875,7 +882,12 @@ public boolean isLocalSessionsUpgradingEnabled() { | |
public boolean isSslQuorum() { | ||
return sslQuorum; | ||
} | ||
|
||
public String getSslAuthServerProvider() { | ||
return sslAuthServerProvider; | ||
} | ||
public String getSslAuthLearnerProvider() { | ||
return sslAuthLearnerProvider; | ||
} | ||
public boolean shouldUsePortUnification() { | ||
return shouldUsePortUnification; | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,171 @@ | ||
/* | ||
* 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.assertTrue; | ||
import java.io.File; | ||
import java.lang.reflect.Method; | ||
import java.util.Properties; | ||
import org.apache.zookeeper.common.QuorumX509Util; | ||
import org.apache.zookeeper.server.quorum.auth.MockSSLQuorumAuthLearner; | ||
import org.apache.zookeeper.server.quorum.auth.MockSslQuorumAuthServer; | ||
import org.apache.zookeeper.server.quorum.auth.NullQuorumAuthLearner; | ||
import org.apache.zookeeper.server.quorum.auth.NullQuorumAuthServer; | ||
import org.apache.zookeeper.server.quorum.auth.QuorumAuth; | ||
import org.apache.zookeeper.server.quorum.auth.QuorumAuthLearner; | ||
import org.apache.zookeeper.server.quorum.auth.QuorumAuthServer; | ||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.Test; | ||
|
||
/** | ||
* Unit tests for pluggable SSL quorum auth providers in {@link QuorumPeer}. | ||
*/ | ||
public class QuorumPeerAuthProviderTest { | ||
|
||
private QuorumX509Util quorumX509Util; | ||
|
||
private static final String SSL_AUTH_PROVIDER_PROPERTY = | ||
"zookeeper." + QuorumAuth.QUORUM_SSL_AUTHPROVIDER; | ||
private static final String SSL_LEARNER_AUTH_PROVIDER_PROPERTY = | ||
"zookeeper." + QuorumAuth.QUORUM_SSL_LEARNER_AUTHPROVIDER; | ||
|
||
@BeforeEach | ||
public void setup() { | ||
quorumX509Util = new QuorumX509Util(); | ||
} | ||
/** | ||
* When sslAuthServerProvider is set to a custom provider, ensure it instantiates correctly. | ||
*/ | ||
@Test | ||
public void testCustomSslAuthServerProvider() throws Exception { | ||
// Prepare config with custom server auth provider | ||
QuorumPeerConfig config = new QuorumPeerConfig(); | ||
Properties zkProp = getDefaultZKProperties(); | ||
zkProp.setProperty("sslQuorum", "true"); | ||
zkProp.setProperty(QuorumAuth.QUORUM_SSL_AUTHPROVIDER, | ||
MockSslQuorumAuthServer.class.getName()); | ||
config.parseProperties(zkProp); | ||
|
||
// Set on peer and invoke private method | ||
QuorumPeer peer = new QuorumPeer(); | ||
peer.setSslAuthServerProvider(config.getSslAuthServerProvider()); | ||
QuorumAuthServer authServer = invokeGetSslQuorumAuthServer(peer); | ||
|
||
assertTrue(authServer instanceof MockSslQuorumAuthServer, | ||
"Expected MockSSLQuorumAuthServer when provider is configured"); | ||
} | ||
|
||
/** | ||
* When sslAuthServerProvider is set via JVM system property | ||
*/ | ||
@Test | ||
public void testSslAuthServerProviderSystemProperty() throws Exception { | ||
System.setProperty(SSL_AUTH_PROVIDER_PROPERTY, | ||
MockSslQuorumAuthServer.class.getName()); | ||
try { | ||
QuorumPeer peer = new QuorumPeer(); | ||
peer.setSslAuthServerProvider("some.invalid.Class"); | ||
QuorumAuthServer authServer = invokeGetSslQuorumAuthServer(peer); | ||
assertTrue(authServer instanceof MockSslQuorumAuthServer, | ||
"Expected system property for server auth provider"); | ||
} finally { | ||
System.clearProperty(SSL_AUTH_PROVIDER_PROPERTY); | ||
} | ||
} | ||
/** | ||
* Without any provider configured, default should be NullQuorumAuthServer. | ||
*/ | ||
@Test | ||
public void testDefaultSslAuthServerProvider() throws Exception { | ||
QuorumPeer peer = new QuorumPeer(); | ||
QuorumAuthServer authServer = invokeGetSslQuorumAuthServer(peer); | ||
assertTrue(authServer instanceof NullQuorumAuthServer, | ||
"Expected NullQuorumAuthServer when no provider is configured"); | ||
} | ||
|
||
/** | ||
* When sslAuthLearnerProvider is set to a custom provider, ensure it instantiates correctly. | ||
*/ | ||
@Test | ||
public void testCustomSslAuthLearnerProvider() throws Exception { | ||
QuorumPeerConfig config = new QuorumPeerConfig(); | ||
Properties zkProp = getDefaultZKProperties(); | ||
zkProp.setProperty("sslQuorum", "true"); | ||
zkProp.setProperty(QuorumAuth.QUORUM_SSL_LEARNER_AUTHPROVIDER, | ||
MockSSLQuorumAuthLearner.class.getName()); | ||
config.parseProperties(zkProp); | ||
|
||
QuorumPeer peer = new QuorumPeer(); | ||
peer.setSslAuthLearnerProvider(config.getSslAuthLearnerProvider()); | ||
QuorumAuthLearner authLearner = invokeGetSslQuorumAuthLearner(peer); | ||
|
||
assertTrue(authLearner instanceof MockSSLQuorumAuthLearner, | ||
"Expected MockSSLQuorumAuthLearner when learner provider is configured"); | ||
} | ||
|
||
/** | ||
* When sslAuthLearnerProvider is set via JVM system property | ||
*/ | ||
@Test | ||
public void testSslAuthLearnerProviderSystemProperty() throws Exception { | ||
System.setProperty(SSL_LEARNER_AUTH_PROVIDER_PROPERTY, | ||
MockSSLQuorumAuthLearner.class.getName()); | ||
try { | ||
QuorumPeer peer = new QuorumPeer(); | ||
QuorumAuthLearner authLearner = invokeGetSslQuorumAuthLearner(peer); | ||
Properties props = System.getProperties(); | ||
props.forEach((key, value) -> System.out.println(key + " = " + value)); | ||
|
||
assertTrue(authLearner instanceof MockSSLQuorumAuthLearner, | ||
"Expected system property for learner auth provider"); | ||
} finally { | ||
System.clearProperty(SSL_LEARNER_AUTH_PROVIDER_PROPERTY); | ||
} | ||
} | ||
/** | ||
* Without any learner provider configured, default should be NullQuorumAuthLearner. | ||
*/ | ||
@Test | ||
public void testDefaultSslAuthLearnerProvider() throws Exception { | ||
QuorumPeer peer = new QuorumPeer(); | ||
QuorumAuthLearner authLearner = invokeGetSslQuorumAuthLearner(peer); | ||
assertTrue(authLearner instanceof NullQuorumAuthLearner, | ||
"Expected NullQuorumAuthLearner when no learner provider is configured"); | ||
} | ||
|
||
// Reflection helpers to access private methods | ||
|
||
private QuorumAuthServer invokeGetSslQuorumAuthServer(QuorumPeer peer) throws Exception { | ||
Method m = QuorumPeer.class.getDeclaredMethod("getSslQuorumAuthServer"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think what you should do here is reading the field |
||
m.setAccessible(true); | ||
return (QuorumAuthServer) m.invoke(peer); | ||
} | ||
|
||
private QuorumAuthLearner invokeGetSslQuorumAuthLearner(QuorumPeer peer) throws Exception { | ||
Method m = QuorumPeer.class.getDeclaredMethod("getSslQuorumAuthLearner"); | ||
m.setAccessible(true); | ||
return (QuorumAuthLearner) m.invoke(peer); | ||
} | ||
private Properties getDefaultZKProperties() { | ||
Properties zkProp = new Properties(); | ||
zkProp.setProperty("dataDir", new File("myDataDir").getAbsolutePath()); | ||
zkProp.setProperty("oraclePath", new File("mastership").getAbsolutePath()); | ||
return zkProp; | ||
} | ||
} |
Uh oh!
There was an error while loading. Please reload this page.