[GitHub] [accumulo] BukrosSzabolcs commented on a change in pull request #1798: Hash upgrade

[GitBox <gi...@apache.org>]

BukrosSzabolcs commented on a change in pull request #1798:
URL: https://github.com/apache/accumulo/pull/1798#discussion_r528868213



##########
File path: server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKAuthenticator.java
##########
@@ -52,6 +52,23 @@ public void initialize(ServerContext context) {
     this.context = context;
     zooCache = new ZooCache(context.getZooReaderWriter(), null);
     ZKUserPath = Constants.ZROOT + "/" + context.getInstanceID() + "/users";
+    checkOutdatedHashes();
+  }
+
+  private void checkOutdatedHashes() {
+    try {
+      listUsers().forEach(user -> {
+        String zpath = ZKUserPath + "/" + user;
+        byte[] zkData = zooCache.get(zpath);
+        if (ZKSecurityTool.isOutdatedPass(zkData)) {
+          log.warn("Found user(s) with outdated password hash. These will be re-hashed"
+              + " on successful authentication.");

Review comment:
       I'm never sure exactly how much detail is too much when it comes to logging security features. I can add an affected user count.
   I'm not sure the operator has to or should do anything, the update is automatic. But with their current tools they could replicate what the code does and could go through the users zNode checking hash lengths. Alternately I could list the affected user principals in the warning but that feels unsafe. Do you have something in mind?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org