You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@accumulo.apache.org by milleruntime <gi...@git.apache.org> on 2016/09/14 16:48:26 UTC
[GitHub] accumulo pull request #151: ACCUMULO-3626: Reset security should warn and re...
GitHub user milleruntime opened a pull request:
https://github.com/apache/accumulo/pull/151
ACCUMULO-3626: Reset security should warn and require instance secret
Created a prompt when running "accumulo init --reset-security" to warn user and require
entering the instance secret. Added a "-f,--force" option for scripts to bypass the prompt.
Also fixed message in Initialize about running ChangeSecret tool.
Ticket fix version says 2.0.0 but I think 1.8 and other versions would benefit as well.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/milleruntime/accumulo ACCUMULO-3626
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/accumulo/pull/151.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #151
----
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #151: ACCUMULO-3626: Reset security should warn and require i...
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on the issue:
https://github.com/apache/accumulo/pull/151
Because this is a pretty dangerous command to run without user interaction, I'd be fine adding it in 1.7 and 1.8 to add a bit of safety, so long as we clearly document in the release notes that automated systems will need to use the force flag.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #151: ACCUMULO-3626: Reset security should warn and re...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/151#discussion_r78790672
--- Diff: server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java ---
@@ -762,6 +764,17 @@ public void execute(final String[] args) {
if (opts.resetSecurity) {
AccumuloServerContext context = new AccumuloServerContext(new ServerConfigurationFactory(HdfsZooInstance.getInstance()));
if (isInitialized(fs)) {
+ if (!opts.forceResetSecurity) {
+ String secret = acuConf.get(Property.INSTANCE_SECRET);
+ ConsoleReader c = getConsoleReader();
+ String userEnteredSecret = c.readLine("WARNING: This will remove all users from Accumulo! If you wish to proceed enter the instance secret: ", '*');
+ if (userEnteredSecret != null && !secret.equals(userEnteredSecret)) {
+ log.info("Aborted reset security: Instance secret did not match what is in accumulo-site.xml");
--- End diff --
s/log.info/log.error/
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #151: ACCUMULO-3626: Reset security should warn and require i...
Posted by busbey <gi...@git.apache.org>.
Github user busbey commented on the issue:
https://github.com/apache/accumulo/pull/151
+1 for 2.0.0
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #151: ACCUMULO-3626: Reset security should warn and require i...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/accumulo/pull/151
> If there aren't any reservations to have this released in 2.0.0, then can it be merged?
:+1: from me.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #151: ACCUMULO-3626: Reset security should warn and re...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/accumulo/pull/151
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #151: ACCUMULO-3626: Reset security should warn and re...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/151#discussion_r78790078
--- Diff: server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java ---
@@ -238,7 +238,7 @@ static boolean checkInit(Configuration conf, VolumeManager fs, SiteConfiguration
c.println();
c.println();
c.println("You can change the instance secret in accumulo by using:");
- c.println(" bin/accumulo " + org.apache.accumulo.server.util.ChangeSecret.class.getName() + " oldPassword newPassword.");
+ c.println(" bin/accumulo " + org.apache.accumulo.server.util.ChangeSecret.class.getName());
--- End diff --
This change seems unrelated to your description.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #151: ACCUMULO-3626: Reset security should warn and re...
Posted by milleruntime <gi...@git.apache.org>.
Github user milleruntime commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/151#discussion_r78792678
--- Diff: server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java ---
@@ -762,6 +764,17 @@ public void execute(final String[] args) {
if (opts.resetSecurity) {
AccumuloServerContext context = new AccumuloServerContext(new ServerConfigurationFactory(HdfsZooInstance.getInstance()));
if (isInitialized(fs)) {
+ if (!opts.forceResetSecurity) {
+ String secret = acuConf.get(Property.INSTANCE_SECRET);
+ ConsoleReader c = getConsoleReader();
+ String userEnteredSecret = c.readLine("WARNING: This will remove all users from Accumulo! If you wish to proceed enter the instance secret: ", '*');
+ if (userEnteredSecret != null && !secret.equals(userEnteredSecret)) {
+ log.info("Aborted reset security: Instance secret did not match what is in accumulo-site.xml");
+ return;
+ }
+ log.info("Beginning user initiated reset security on accumulo.");
--- End diff --
I added this log message since it is not clear what is going on in the output...
2016-09-14 12:35:03,251 [conf.AccumuloConfiguration] INFO : Loaded class : org.apache.accumulo.server.security.handler.ZKAuthorizor
2016-09-14 12:35:03,252 [conf.AccumuloConfiguration] INFO : Loaded class : org.apache.accumulo.server.security.handler.ZKAuthenticator
2016-09-14 12:35:03,253 [conf.AccumuloConfiguration] INFO : Loaded class : org.apache.accumulo.server.security.handler.ZKPermHandler
2016-09-14 12:35:03,508 [handler.ZKAuthenticator] INFO : Removed /accumulo/5a96e1b5-452b-4c35-ada7-cfa807cbd994/users/ from zookeeper
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #151: ACCUMULO-3626: Reset security should warn and re...
Posted by ctubbsii <gi...@git.apache.org>.
Github user ctubbsii commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/151#discussion_r78792461
--- Diff: server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java ---
@@ -762,6 +764,17 @@ public void execute(final String[] args) {
if (opts.resetSecurity) {
AccumuloServerContext context = new AccumuloServerContext(new ServerConfigurationFactory(HdfsZooInstance.getInstance()));
if (isInitialized(fs)) {
+ if (!opts.forceResetSecurity) {
+ String secret = acuConf.get(Property.INSTANCE_SECRET);
+ ConsoleReader c = getConsoleReader();
+ String userEnteredSecret = c.readLine("WARNING: This will remove all users from Accumulo! If you wish to proceed enter the instance secret: ", '*');
--- End diff --
The prompt for the secret is similar to how GitHub asks you to type the name of the repo, to be sure you really want to delete it. It's not a security check. The user running this command already needs access to the server configuration file containing the secret. It's just a sanity check. Instead of the secret, it could prompt for the instance name or ID.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #151: ACCUMULO-3626: Reset security should warn and re...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/151#discussion_r78790611
--- Diff: server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java ---
@@ -762,6 +764,17 @@ public void execute(final String[] args) {
if (opts.resetSecurity) {
AccumuloServerContext context = new AccumuloServerContext(new ServerConfigurationFactory(HdfsZooInstance.getInstance()));
if (isInitialized(fs)) {
+ if (!opts.forceResetSecurity) {
+ String secret = acuConf.get(Property.INSTANCE_SECRET);
+ ConsoleReader c = getConsoleReader();
+ String userEnteredSecret = c.readLine("WARNING: This will remove all users from Accumulo! If you wish to proceed enter the instance secret: ", '*');
--- End diff --
> If you wish to proceed enter the instance secret
Do we need to prompt for instance.secret instead of just a 'Y/N'? It's not apparent if this is buying us anything. If someone can successfully re-init (permission wise), they have the ability to get this value.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #151: ACCUMULO-3626: Reset security should warn and require i...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/accumulo/pull/151
> Because this is a pretty dangerous command to run without user interaction
I would also say that I think the risk is rather low to "stumble" onto this. I would guess the risk of an user/admin accidentally running this command is low. I think our docs do a good job already alerting that this is a hammer, not a scalpel (although this is off of memory)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #151: ACCUMULO-3626: Reset security should warn and re...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/151#discussion_r78790750
--- Diff: server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java ---
@@ -762,6 +764,17 @@ public void execute(final String[] args) {
if (opts.resetSecurity) {
AccumuloServerContext context = new AccumuloServerContext(new ServerConfigurationFactory(HdfsZooInstance.getInstance()));
if (isInitialized(fs)) {
+ if (!opts.forceResetSecurity) {
+ String secret = acuConf.get(Property.INSTANCE_SECRET);
+ ConsoleReader c = getConsoleReader();
+ String userEnteredSecret = c.readLine("WARNING: This will remove all users from Accumulo! If you wish to proceed enter the instance secret: ", '*');
+ if (userEnteredSecret != null && !secret.equals(userEnteredSecret)) {
+ log.info("Aborted reset security: Instance secret did not match what is in accumulo-site.xml");
+ return;
+ }
+ log.info("Beginning user initiated reset security on accumulo.");
--- End diff --
Maybe implied?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #151: ACCUMULO-3626: Reset security should warn and re...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/151#discussion_r78790240
--- Diff: server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java ---
@@ -762,6 +764,17 @@ public void execute(final String[] args) {
if (opts.resetSecurity) {
AccumuloServerContext context = new AccumuloServerContext(new ServerConfigurationFactory(HdfsZooInstance.getInstance()));
if (isInitialized(fs)) {
+ if (!opts.forceResetSecurity) {
+ String secret = acuConf.get(Property.INSTANCE_SECRET);
+ ConsoleReader c = getConsoleReader();
+ String userEnteredSecret = c.readLine("WARNING: This will remove all users from Accumulo! If you wish to proceed enter the instance secret: ", '*');
--- End diff --
> This will remove all users from Accumulo
Really, this is removing all ZooKeeper data. The "user database" just happens to be in ZooKeeper. Maybe there's a better way to word this?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #151: ACCUMULO-3626: Reset security should warn and require i...
Posted by busbey <gi...@git.apache.org>.
Github user busbey commented on the issue:
https://github.com/apache/accumulo/pull/151
I don't think this would break Cloudera Manager specifically, since this is the kind of thing we'd document in Knowledge Base articles for support / customers rather than try to automate (as mentioned, it's dangerous and a blunt tool).
But I'd also rather see this in a minor or major release and not a maintenance release since it changes how an operator facing tool works. I've already documented / explained / conditioned folks to expect this tool to be useful in a set of circumstances and to act a certain way (again, since it's dangerous we train folks to use it by saying what to expect and not expect as they use it). I won't be able to grab maintenance releases in a straight-forward way if I have to update internal training things about this tool to specify different expectations for e.g. 1.7.2 vs 1.7.3.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #151: ACCUMULO-3626: Reset security should warn and require i...
Posted by milleruntime <gi...@git.apache.org>.
Github user milleruntime commented on the issue:
https://github.com/apache/accumulo/pull/151
I changed this to merge into master. If there aren't any reservations to have this released in 2.0.0, then can it be merged?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #151: ACCUMULO-3626: Reset security should warn and require i...
Posted by joshelser <gi...@git.apache.org>.
Github user joshelser commented on the issue:
https://github.com/apache/accumulo/pull/151
> Ticket fix version says 2.0.0 but I think 1.8 and other versions would benefit as well.
I am hesitant to include this on patch-releases as this is a very user-facing change. This could break automatic provisioning tools (e.g. Ambari, Cloudera Manager, etc).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #151: ACCUMULO-3626: Reset security should warn and re...
Posted by milleruntime <gi...@git.apache.org>.
Github user milleruntime commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/151#discussion_r78791735
--- Diff: server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java ---
@@ -238,7 +238,7 @@ static boolean checkInit(Configuration conf, VolumeManager fs, SiteConfiguration
c.println();
c.println();
c.println("You can change the instance secret in accumulo by using:");
- c.println(" bin/accumulo " + org.apache.accumulo.server.util.ChangeSecret.class.getName() + " oldPassword newPassword.");
+ c.println(" bin/accumulo " + org.apache.accumulo.server.util.ChangeSecret.class.getName());
--- End diff --
Correct. I noticed the message was wrong and fixed it while editing the file.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #151: ACCUMULO-3626: Reset security should warn and re...
Posted by milleruntime <gi...@git.apache.org>.
Github user milleruntime commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/151#discussion_r78793577
--- Diff: server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java ---
@@ -762,6 +764,17 @@ public void execute(final String[] args) {
if (opts.resetSecurity) {
AccumuloServerContext context = new AccumuloServerContext(new ServerConfigurationFactory(HdfsZooInstance.getInstance()));
if (isInitialized(fs)) {
+ if (!opts.forceResetSecurity) {
+ String secret = acuConf.get(Property.INSTANCE_SECRET);
+ ConsoleReader c = getConsoleReader();
+ String userEnteredSecret = c.readLine("WARNING: This will remove all users from Accumulo! If you wish to proceed enter the instance secret: ", '*');
+ if (userEnteredSecret != null && !secret.equals(userEnteredSecret)) {
+ log.info("Aborted reset security: Instance secret did not match what is in accumulo-site.xml");
+ return;
+ }
+ log.info("Beginning user initiated reset security on accumulo.");
--- End diff --
Should be outside the if though... I can remove or move to the beginning of if (opts.resetSecurity) block?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo issue #151: ACCUMULO-3626: Reset security should warn and require i...
Posted by milleruntime <gi...@git.apache.org>.
Github user milleruntime commented on the issue:
https://github.com/apache/accumulo/pull/151
@busbey You OK with this change going out in 2.0.0?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] accumulo pull request #151: ACCUMULO-3626: Reset security should warn and re...
Posted by milleruntime <gi...@git.apache.org>.
Github user milleruntime commented on a diff in the pull request:
https://github.com/apache/accumulo/pull/151#discussion_r78792143
--- Diff: server/base/src/main/java/org/apache/accumulo/server/init/Initialize.java ---
@@ -762,6 +764,17 @@ public void execute(final String[] args) {
if (opts.resetSecurity) {
AccumuloServerContext context = new AccumuloServerContext(new ServerConfigurationFactory(HdfsZooInstance.getInstance()));
if (isInitialized(fs)) {
+ if (!opts.forceResetSecurity) {
+ String secret = acuConf.get(Property.INSTANCE_SECRET);
+ ConsoleReader c = getConsoleReader();
+ String userEnteredSecret = c.readLine("WARNING: This will remove all users from Accumulo! If you wish to proceed enter the instance secret: ", '*');
--- End diff --
I am open to suggestions for the wording of the message, I was going by what was requested in the ticket. I believe having the user enter something other than Y/N was to require additional thought as to whether they really want perform this action.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---