ctubbsii commented on a change in pull request #1798:
URL: https://github.com/apache/accumulo/pull/1798#discussion_r530537337
##########
File path:
server/base/src/main/java/org/apache/accumulo/server/security/handler/ZKAuthenticator.java
##########
@@ -180,18 +222,43 @@ public boolean authenticateUser(String principal,
AuthenticationToken token)
if (!(token instanceof PasswordToken))
throw new AccumuloSecurityException(principal,
SecurityErrorCode.INVALID_TOKEN);
PasswordToken pt = (PasswordToken) token;
- byte[] pass;
+ byte[] zkData;
String zpath = ZKUserPath + "/" + principal;
- pass = zooCache.get(zpath);
- boolean result = ZKSecurityTool.checkPass(pt.getPassword(), pass);
+ zkData = zooCache.get(zpath);
+ boolean result = authenticateUser(principal, pt, zkData);
if (!result) {
zooCache.clear(zpath);
- pass = zooCache.get(zpath);
- result = ZKSecurityTool.checkPass(pt.getPassword(), pass);
+ zkData = zooCache.get(zpath);
+ result = authenticateUser(principal, pt, zkData);
}
return result;
}
+ private boolean authenticateUser(String principal, PasswordToken pt, byte[]
zkData) {
+ if (zkData == null) {
+ return false;
+ }
+
+ // if the hash does not match the outdated format use Crypt to verify it
+ if (!ZKSecurityTool.isOutdatedPass(zkData)) {
+ return ZKSecurityTool.checkCryptPass(pt.getPassword(), zkData);
+ }
+
+ if (!ZKSecurityTool.checkPass(pt.getPassword(), zkData)) {
+ // if password does not match we are done
+ return false;
+ }
+
+ // if the password is correct we have to update the stored hash with new
algorithm
+ try {
+ changePassword(principal, pt);
+ return true;
+ } catch (AccumuloSecurityException e) {
+ log.error("Failed to update hashed user password for user: {}",
principal, e);
+ }
+ return false;
Review comment:
I don't feel very strongly about it. However, I do lean towards forcing
the upgrade and rejecting if it can't be completed (for the reasons @busbey
mentioned, as well as because I want to eventually ensure we can drop the code
that supports reading the old hashes on some future upgrade).
I think it's very unlikely we're going to fail here if we've already
authenticated, unless ZK is having issues (in which case, we'll probably see
other errors in the system). So, it may be moot. Either way would probably be
fine.
----------------------------------------------------------------
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:
[email protected]
