Commit ccd812a1 authored by Ard Schrijvers's avatar Ard Schrijvers

REPO-2180 [Backport 11.2] improve the JCR session in memory handling

After a successful or unsuccessful login, zero-out the password so it
isn't present any more in memory.

(cherry picked from commit 866a6a7d)
(cherry picked from commit a155b999)
parent 1812e057
/*
* Copyright 2008-2015 Hippo B.V. (http://www.onehippo.com)
* Copyright 2008-2019 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -18,6 +18,7 @@ package org.hippoecm.repository.security.user;
import java.io.UnsupportedEncodingException;
import java.security.NoSuchAlgorithmException;
import java.util.Arrays;
import java.util.UUID;
import javax.jcr.Node;
import javax.jcr.RepositoryException;
......@@ -44,6 +45,11 @@ public class RepositoryUserManager extends AbstractUserManager {
private static final Logger log = LoggerFactory.getLogger(RepositoryUserManager.class);
private static final String SECURITY_PATH = HippoNodeType.CONFIGURATION_PATH + "/" + HippoNodeType.SECURITY_PATH;
// note we include a random uuid to make sure that *before* a successful login the attribute name is not known
// on the SimpleCredentials object
private static final String LOGIN_RESULT_ATTR = RepositoryUserManager.class.getName()
+ UUID.randomUUID().toString() + ".loginSuccess";
private static final long ONEDAYMS = 1000 * 3600 * 24;
private boolean maintenanceMode = false;
......@@ -112,8 +118,20 @@ public class RepositoryUserManager extends AbstractUserManager {
return true;
}
final Boolean loginResult = (Boolean)creds.getAttribute(LOGIN_RESULT_ATTR);
if (loginResult != null) {
return loginResult;
}
// do regular password check
return PasswordHelper.checkHash(password, getPasswordHash(userinfo));
final boolean success = PasswordHelper.checkHash(password, getPasswordHash(userinfo));
// purge the credentials and store the success outcome
creds.setAttribute(LOGIN_RESULT_ATTR, success);
zeroOutPassword(creds.getPassword());
return success;
} catch (NoSuchAlgorithmException e) {
throw new RepositoryException("Unknown algorithm found when authenticating user: " + creds.getUserID(), e);
} catch (UnsupportedEncodingException e) {
......@@ -121,6 +139,12 @@ public class RepositoryUserManager extends AbstractUserManager {
}
}
private void zeroOutPassword(final char[] password) {
for (int i = 0; i < password.length ; i++) {
password[i] = 0;
}
}
public String getNodeType() {
return HippoNodeType.NT_USER;
}
......
/*
* Copyright 2008-2015 Hippo B.V. (http://www.onehippo.com)
* Copyright 2008-2018 Hippo B.V. (http://www.onehippo.com)
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
......@@ -34,6 +34,8 @@ import static org.hippoecm.repository.api.HippoNodeType.HIPPO_PASSKEY;
import static org.hippoecm.repository.api.HippoNodeType.HIPPO_PASSWORD;
import static org.hippoecm.repository.api.HippoNodeType.NT_USER;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
public class RepositoryLoginTest extends RepositoryTestCase {
......@@ -82,12 +84,30 @@ public class RepositoryLoginTest extends RepositoryTestCase {
super.tearDown();
}
private void loginAssertions(final String userId, final String password) throws RepositoryException {
final SimpleCredentials credentials = new SimpleCredentials(userId, password.toCharArray());
assertFalse(passwordIsZeroedOut(credentials.getPassword()));
Session session = server.login(credentials);
assertEquals(userId, session.getUserID());
assertTrue(passwordIsZeroedOut(credentials.getPassword()));
session.logout();
// with the same credentials object, you should be able to login again
final Session session2 = server.login(credentials);
session2.logout();
final SimpleCredentials clone = new SimpleCredentials(credentials.getUserID(), credentials.getPassword());
try {
server.login(clone);
fail("The cloned credentials should not work since other object will zeroed out password");
} catch (LoginException e) {
// expected
}
}
@Test
public void testLoginPlainSuccess() throws Exception {
try {
Session session = server.login(TESTUSER_ID_PLAIN, TESTUSER_PASS.toCharArray());
assertEquals(TESTUSER_ID_PLAIN, session.getUserID());
session.logout();
loginAssertions(TESTUSER_ID_PLAIN, TESTUSER_PASS);
} catch (LoginException e) {
fail("Plain login failed with valid credentials");
}
......@@ -96,9 +116,7 @@ public class RepositoryLoginTest extends RepositoryTestCase {
@Test
public void testLoginMD5Success() throws Exception {
try {
Session session = server.login(TESTUSER_ID_MD5, TESTUSER_PASS.toCharArray());
assertEquals(TESTUSER_ID_MD5, session.getUserID());
session.logout();
loginAssertions(TESTUSER_ID_MD5, TESTUSER_PASS);
} catch (LoginException e) {
fail("MD5 login failed with valid credentials");
}
......@@ -107,9 +125,7 @@ public class RepositoryLoginTest extends RepositoryTestCase {
@Test
public void testLoginSHA1Success() throws Exception {
try {
Session session = server.login(TESTUSER_ID_SHA1, TESTUSER_PASS.toCharArray());
assertEquals(TESTUSER_ID_SHA1, session.getUserID());
session.logout();
loginAssertions(TESTUSER_ID_SHA1, TESTUSER_PASS);
} catch (LoginException e) {
fail("SHA-1 login failed with valid credentials");
}
......@@ -118,9 +134,7 @@ public class RepositoryLoginTest extends RepositoryTestCase {
@Test
public void testLoginSHA256Success() throws Exception {
try {
Session session = server.login(TESTUSER_ID_SHA256, TESTUSER_PASS.toCharArray());
assertEquals(TESTUSER_ID_SHA256, session.getUserID());
session.logout();
loginAssertions(TESTUSER_ID_SHA256, TESTUSER_PASS);
} catch (LoginException e) {
fail("SHA-256 login failed with valid credentials");
}
......@@ -129,9 +143,12 @@ public class RepositoryLoginTest extends RepositoryTestCase {
@Test
public void testLoginJvmUser() throws Exception {
try {
final Session session = server.login(JvmCredentials.getCredentials(TESTUSER_JVM));
final JvmCredentials credentials = JvmCredentials.getCredentials(TESTUSER_JVM);
final Session session = server.login(credentials);
assertEquals(TESTUSER_JVM, session.getUserID());
session.logout();
// jvm user has random password which does not need to be zeroed out
assertFalse(passwordIsZeroedOut(credentials.getPassword()));
} catch (LoginException e) {
fail("Jvm user login failed");
}
......@@ -155,14 +172,26 @@ public class RepositoryLoginTest extends RepositoryTestCase {
@Test(expected = LoginException.class)
public void testLoginPlainFail() throws Exception {
Session session = server.login(TESTUSER_ID_PLAIN, "wrongpassword".toCharArray());
session.logout();
failingLoginAssertions(TESTUSER_ID_PLAIN, "wrongpassword");
}
private void failingLoginAssertions(final String testuserIdPlain, final String wrongPassword) throws RepositoryException {
final SimpleCredentials credentials = new SimpleCredentials(testuserIdPlain, wrongPassword.toCharArray());
assertFalse(passwordIsZeroedOut(credentials.getPassword()));
try {
server.login(credentials);
fail("Expected login exception");
} catch (LoginException e) {
assertTrue("even for failed login the password gets zeroed out",
passwordIsZeroedOut(credentials.getPassword()));
// new attempt just results again in LoginException
server.login(credentials);
}
}
@Test(expected = LoginException.class)
public void testLoginHashFail() throws Exception {
Session session = server.login(TESTUSER_ID_SHA1, "wrongpassword".toCharArray());
session.logout();
failingLoginAssertions(TESTUSER_ID_SHA1, "wrongpassword");
}
@Test(expected = LoginException.class)
......@@ -251,4 +280,14 @@ public class RepositoryLoginTest extends RepositoryTestCase {
log.info(count + "\t" + (t2 - t1) + "\t" + ((t2 - t1) / count));
}
}
private boolean passwordIsZeroedOut(final char[] password) {
for (char c : password) {
if (c != 0) {
return false;
}
}
return true;
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment