[
https://issues.apache.org/jira/browse/ZOOKEEPER-2014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15640611#comment-15640611
]
ASF GitHub Bot commented on ZOOKEEPER-2014:
-------------------------------------------
Github user rgs1 commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/96#discussion_r86673398
--- Diff: src/c/tests/TestReconfigServer.cc ---
@@ -324,4 +336,109 @@ testRemoveConnectedFollower() {
zookeeper_close(zk);
}
+/**
+ * ZOOKEEPER-2014: only admin or users who are explicitly granted
permission can do reconfig.
+ */
+void TestReconfigServer::
+testReconfigFailureWithoutAuth() {
+ std::vector<std::string> servers;
+ std::string version;
+ struct Stat stat;
+ int len = 1024;
+ char buf[len];
+
+ // connect to a follower.
+ int32_t leader = getLeader();
+ std::vector<int32_t> followers = getFollowers();
+ CPPUNIT_ASSERT(leader >= 0);
+ CPPUNIT_ASSERT_EQUAL(NUM_SERVERS - 1, (uint32_t)(followers.size()));
+ std::stringstream ss;
+ for (int i = 0; i < followers.size(); i++) {
+ ss << cluster_[followers[i]]->getHostPort() << ",";
+ }
+ ss << cluster_[leader]->getHostPort();
+ std::string hosts = ss.str().c_str();
+ zoo_deterministic_conn_order(true);
+ zhandle_t* zk = zookeeper_init(hosts.c_str(), NULL, 10000, NULL, NULL,
0);
+ CPPUNIT_ASSERT_EQUAL(true, waitForConnected(zk, 10));
+
+ std::string connectedHost(zoo_get_current_server(zk));
+ std::string portString = connectedHost.substr(connectedHost.find(":")
+ 1);
+ uint32_t port;
+ std::istringstream (portString) >> port;
+ CPPUNIT_ASSERT_EQUAL(cluster_[followers[0]]->getClientPort(), port);
+
+ // remove the follower.
+ len = 1024;
+ ss.str("");
+ ss << followers[0];
+ // No auth, should fail.
+ CPPUNIT_ASSERT_EQUAL((int)ZNOAUTH, zoo_reconfig(zk, NULL,
ss.str().c_str(), NULL, -1, buf, &len, &stat));
+ // Wrong auth, should fail.
+ CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_add_auth(zk, "digest",
"super:wrong", 11, NULL,(void*)ZOK));
+ CPPUNIT_ASSERT_EQUAL((int)ZNOAUTH, zoo_reconfig(zk, NULL,
ss.str().c_str(), NULL, -1, buf, &len, &stat));
+ // Right auth, should pass.
+ CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_add_auth(zk, "digest",
"super:test", 10, NULL,(void*)ZOK));
+ CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_reconfig(zk, NULL,
ss.str().c_str(), NULL, -1, buf, &len, &stat));
+ CPPUNIT_ASSERT_EQUAL((int)ZOK, zoo_getconfig(zk, 0, buf, &len, &stat));
+ parseConfig(buf, len, servers, version);
+ CPPUNIT_ASSERT_EQUAL(NUM_SERVERS - 1, (uint32_t)(servers.size()));
+ for (int i = 0; i < cluster_.size(); i++) {
+ if (i == followers[0]) {
+ continue;
+ }
+ CPPUNIT_ASSERT(std::find(servers.begin(), servers.end(),
+ cluster_[i]->getServerString()) != servers.end());
+ }
+ zookeeper_close(zk);
+}
+
+void TestReconfigServer::
+testReconfigFailureWithoutServerSuperuserPasswordConfigured() {
+ std::vector<std::string> servers;
+ std::string version;
+ struct Stat stat;
+ int len = 1024;
+ char buf[len];
+
+ // Create a new quorum with the super user's password not configured.
+ tearDown();
+ ZooKeeperQuorumServer::tConfigPairs configs;
+ configs.push_back(std::make_pair("reconfigEnabled", "true"));
+ cluster_ = ZooKeeperQuorumServer::getCluster(NUM_SERVERS, configs, "");
+
+ // connect to a follower.
+ int32_t leader = getLeader();
+ std::vector<int32_t> followers = getFollowers();
+ CPPUNIT_ASSERT(leader >= 0);
+ CPPUNIT_ASSERT_EQUAL(NUM_SERVERS - 1, (uint32_t)(followers.size()));
+ std::stringstream ss;
+ for (int i = 0; i < followers.size(); i++) {
+ ss << cluster_[followers[i]]->getHostPort() << ",";
+ }
+ ss << cluster_[leader]->getHostPort();
+ std::string hosts = ss.str().c_str();
+ zoo_deterministic_conn_order(true);
+ zhandle_t* zk = zookeeper_init(hosts.c_str(), NULL, 10000, NULL, NULL,
0);
+ CPPUNIT_ASSERT_EQUAL(true, waitForConnected(zk, 10));
+
+ std::string connectedHost(zoo_get_current_server(zk));
+ std::string portString = connectedHost.substr(connectedHost.find(":")
+ 1);
+ uint32_t port;
+ std::istringstream (portString) >> port;
+ CPPUNIT_ASSERT_EQUAL(cluster_[followers[0]]->getClientPort(), port);
--- End diff --
connect to follower seems repeated, can we move it to a private helper
method?
> Only admin should be allowed to reconfig a cluster
> --------------------------------------------------
>
> Key: ZOOKEEPER-2014
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2014
> Project: ZooKeeper
> Issue Type: Bug
> Components: server
> Affects Versions: 3.5.0
> Reporter: Raul Gutierrez Segales
> Assignee: Michael Han
> Priority: Blocker
> Fix For: 3.5.3
>
> Attachments: ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch,
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch,
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch,
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch,
> ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch, ZOOKEEPER-2014.patch
>
>
> ZOOKEEPER-107 introduces reconfiguration support via the reconfig() call. We
> should, at the very least, ensure that only the Admin can reconfigure a
> cluster. Perhaps restricting access to /zookeeper/config as well, though this
> is debatable. Surely one could ensure Admin only access via an ACL, but that
> would leave everyone who doesn't use ACLs unprotected. We could also force a
> default ACL to make it a bit more consistent (maybe).
> Finally, making reconfig() only available to Admins means they have to run
> with zookeeper.DigestAuthenticationProvider.superDigest (which I am not sure
> if everyone does, or how would it work with other authentication providers).
> Review board https://reviews.apache.org/r/51546/
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
