Revise blacklist password policy provider #8982
- Reduce false positive probability from 1% to 0.01% to avoid rejecting to many actually good passwords. - Make false positive rate configurable via spi config - Revised log messages Supported syntax variant: `passwordBlacklist(wordlistFilename)` Fixes #8982 Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
This commit is contained in:
Thomas Darimont
Marek Posolda
parent
5ba004b447
commit
e38b7adf92
4 changed files with 108 additions and 48 deletions
|
|
@ -1,5 +1,6 @@
|
|||
package org.keycloak.policy;
|
||||
|
||||
import org.keycloak.Config;
|
||||
import org.keycloak.models.KeycloakContext;
|
||||
import org.keycloak.models.RealmModel;
|
||||
import org.keycloak.models.UserModel;
|
||||
|
|
@ -57,6 +58,18 @@ public class BlacklistPasswordPolicyProvider implements PasswordPolicyProvider {
|
|||
return validate(user.getUsername(), password);
|
||||
}
|
||||
|
||||
/**
|
||||
* Parses the allowed configuration for a {@link BlacklistPasswordPolicyProvider}.
|
||||
* Supported syntax is {@¢ode passwordBlacklist(fileName)}
|
||||
*
|
||||
* Example configurations:
|
||||
* <ul>
|
||||
* <li>{@code passwordBlacklist(test-password-blacklist.txt)}</li>
|
||||
* </ul>
|
||||
*
|
||||
* @param blacklistName
|
||||
* @return
|
||||
*/
|
||||
@Override
|
||||
public Object parseConfig(String blacklistName) {
|
||||
|
||||
|
|
|
|||
|
|
@ -39,28 +39,33 @@ import java.util.function.Supplier;
|
|||
/**
|
||||
* Creates {@link BlacklistPasswordPolicyProvider} instances.
|
||||
* <p>
|
||||
* Password blacklists are simple text files where every line is a blacklisted password delimited by {@code \n}.
|
||||
* Blacklist files are discovered and registered at startup.
|
||||
* Password blacklists are simple text files where every line is a blacklisted password delimited by a newline character {@code \n}.
|
||||
* <p>Blacklists can be configured via the <em>Authentication: Password Policy</em> section in the admin-console.
|
||||
* A blacklist-file is referred to by its name in the policy configuration.
|
||||
*
|
||||
* <h1>Blacklist location</h1>
|
||||
* <p>Users can provide custom blacklists by adding a blacklist password file to the configured blacklist folder.
|
||||
* <p>
|
||||
* <p>The location of the password-blacklists folder is derived as follows</p>
|
||||
* <ol>
|
||||
* <li>the value of the System property {@code keycloak.password.blacklists.path} if configured - fails if folder is missing</li>
|
||||
* <li>the value of the SPI config property: {@code blacklistsPath} when explicitly configured - fails if folder is missing</li>
|
||||
* <li>otherwise {@code ${jboss.server.data.dir}/password-blacklists/} if nothing else is configured - the folder is created automatically if not present</li>
|
||||
* <li>otherwise {@code $KC_HOME/data/password-blacklists/} if nothing else is configured</li>
|
||||
* </ol>
|
||||
* <p>Note that the preferred way for configuration is to copy the password file to the {@code ${jboss.server.data.dir}/password-blacklists/} folder</p>
|
||||
* <p>To configure a password blacklist via the SPI configuration, run the following jboss-cli script:</p>
|
||||
* <pre>{@code
|
||||
* /subsystem=keycloak-server/spi=password-policy:add()
|
||||
* /subsystem=keycloak-server/spi=password-policy/provider=passwordBlacklist:add(enabled=true)
|
||||
* /subsystem=keycloak-server/spi=password-policy/provider=passwordBlacklist:write-attribute(name=properties.blacklistsPath, value=/data/keycloak/blacklists/)
|
||||
* }</pre>
|
||||
* <p>A password blacklist with the filename {@code 10_million_password_list_top_1000000-password-blacklist.txt}
|
||||
* that is located beneath {@code /data/keycloak/blacklists/} can be referred to
|
||||
* as {@code 10_million_password_list_top_1000000-password-blacklist.txt} in the <em>Authentication: Password Policy</em> configuration.
|
||||
*
|
||||
* To configure the blacklist folder via CLI use {@code --spi-password-policy-password-blacklist-blacklists-path=/path/to/blacklistsFolder}
|
||||
*
|
||||
* <p>Note that the preferred way for configuration is to copy the password file to the {@code $KC_HOME/data/password-blacklists/} folder</p>
|
||||
* <p>A password blacklist with the filename {@code 10_million_passwords.txt}
|
||||
* that is located beneath {@code $KC_HOME/data/keycloak/blacklists/} can be referred to as {@code 10_million_passwords.txt} in the <em>Authentication: Password Policy</em> configuration.
|
||||
*
|
||||
* <h1>False positives</h1>
|
||||
* <p>
|
||||
* The current implementation uses a probabilistic data-structure called {@link BloomFilter} which allows for fast and memory efficient containment checks, e.g. whether a given password is contained in a blacklist,
|
||||
* with the possibility for false positives. By default a false positive probability {@link #DEFAULT_FALSE_POSITIVE_PROBABILITY} is used.
|
||||
*
|
||||
* To change the false positive probability via CLI configuration use {@code --spi-password-policy-password-blacklist-false-positive-probability=0.00001}
|
||||
* </p>
|
||||
*
|
||||
* @author <a href="mailto:thomas.darimont@gmail.com">Thomas Darimont</a>
|
||||
*/
|
||||
|
|
@ -74,11 +79,15 @@ public class BlacklistPasswordPolicyProviderFactory implements PasswordPolicyPro
|
|||
|
||||
public static final String BLACKLISTS_PATH_PROPERTY = "blacklistsPath";
|
||||
|
||||
public static final String BLACKLISTS_FALSE_POSITIVE_PROBABILITY_PROPERTY = "falsePositiveProbability";
|
||||
|
||||
public static final double DEFAULT_FALSE_POSITIVE_PROBABILITY = 0.0001;
|
||||
|
||||
public static final String JBOSS_SERVER_DATA_DIR = "jboss.server.data.dir";
|
||||
|
||||
public static final String PASSWORD_BLACKLISTS_FOLDER = "password-blacklists" + File.separator;
|
||||
|
||||
private ConcurrentMap<String, FileBasedPasswordBlacklist> blacklistRegistry = new ConcurrentHashMap<>();
|
||||
private final ConcurrentMap<String, FileBasedPasswordBlacklist> blacklistRegistry = new ConcurrentHashMap<>();
|
||||
|
||||
private volatile Path blacklistsBasePath;
|
||||
|
||||
|
|
@ -136,7 +145,7 @@ public class BlacklistPasswordPolicyProviderFactory implements PasswordPolicyPro
|
|||
|
||||
/**
|
||||
* Method to obtain the default location for the list folder. The method
|
||||
* will return the <em>data</em> directory of the installation concatenated
|
||||
* will return the <em>data</em> directory of the Keycloak instance concatenated
|
||||
* with <em>/password-blacklists/</em>.
|
||||
*
|
||||
* @return The default path used by the provider to lookup the lists
|
||||
|
|
@ -156,18 +165,38 @@ public class BlacklistPasswordPolicyProviderFactory implements PasswordPolicyPro
|
|||
|
||||
Objects.requireNonNull(blacklistName, "blacklistName");
|
||||
|
||||
String cleanedBlacklistName = blacklistName.trim();
|
||||
if (cleanedBlacklistName.isEmpty()) {
|
||||
String listName = blacklistName.trim();
|
||||
if (listName.isEmpty()) {
|
||||
throw new IllegalArgumentException("Password blacklist name must not be empty!");
|
||||
}
|
||||
|
||||
return blacklistRegistry.computeIfAbsent(cleanedBlacklistName, (name) -> {
|
||||
FileBasedPasswordBlacklist pbl = new FileBasedPasswordBlacklist(this.blacklistsBasePath, name);
|
||||
return blacklistRegistry.computeIfAbsent(listName, (name) -> {
|
||||
double fpp = getFalsePositiveProbability();
|
||||
FileBasedPasswordBlacklist pbl = new FileBasedPasswordBlacklist(this.blacklistsBasePath, name, fpp);
|
||||
pbl.lazyInit();
|
||||
return pbl;
|
||||
});
|
||||
}
|
||||
|
||||
protected double getFalsePositiveProbability() {
|
||||
|
||||
if (config == null) {
|
||||
return DEFAULT_FALSE_POSITIVE_PROBABILITY;
|
||||
}
|
||||
|
||||
String falsePositiveProbString = config.get(BLACKLISTS_FALSE_POSITIVE_PROBABILITY_PROPERTY);
|
||||
if (falsePositiveProbString == null) {
|
||||
return DEFAULT_FALSE_POSITIVE_PROBABILITY;
|
||||
}
|
||||
|
||||
try {
|
||||
return Double.parseDouble(falsePositiveProbString);
|
||||
} catch (NumberFormatException nfe) {
|
||||
LOG.warnf("Could not parse false positive probability from string %s", falsePositiveProbString);
|
||||
return DEFAULT_FALSE_POSITIVE_PROBABILITY;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A {@link PasswordBlacklist} describes a list of too easy to guess
|
||||
* or potentially leaked passwords that users should not be able to use.
|
||||
|
|
@ -194,14 +223,12 @@ public class BlacklistPasswordPolicyProviderFactory implements PasswordPolicyPro
|
|||
* to construct a {@link PasswordBlacklist}.
|
||||
* <p>
|
||||
* This implementation uses a dynamically sized {@link BloomFilter}
|
||||
* to provide a false positive probability of 1%.
|
||||
* with a provided default false positive probability.
|
||||
*
|
||||
* @see BloomFilter
|
||||
*/
|
||||
public static class FileBasedPasswordBlacklist implements PasswordBlacklist {
|
||||
|
||||
private static final double FALSE_POSITIVE_PROBABILITY = 0.01;
|
||||
|
||||
private static final int BUFFER_SIZE_IN_BYTES = 512 * 1024;
|
||||
|
||||
/**
|
||||
|
|
@ -214,22 +241,34 @@ public class BlacklistPasswordPolicyProviderFactory implements PasswordPolicyPro
|
|||
*/
|
||||
private final Path path;
|
||||
|
||||
private final double falsePositiveProbability;
|
||||
|
||||
/**
|
||||
* Initialized lazily via {@link #lazyInit()}
|
||||
*/
|
||||
private BloomFilter<String> blacklist;
|
||||
|
||||
/**
|
||||
* Creates a new {@link FileBasedPasswordBlacklist} with {@link #DEFAULT_FALSE_POSITIVE_PROBABILITY}.
|
||||
*
|
||||
* @param blacklistBasePath folder containing the blacklists
|
||||
* @param name name of blacklist file
|
||||
*/
|
||||
public FileBasedPasswordBlacklist(Path blacklistBasePath, String name) {
|
||||
this(blacklistBasePath, name, DEFAULT_FALSE_POSITIVE_PROBABILITY);
|
||||
}
|
||||
|
||||
this.name = name;
|
||||
this.path = blacklistBasePath.resolve(name);
|
||||
|
||||
public FileBasedPasswordBlacklist(Path blacklistBasePath, String name, double falsePositiveProbability) {
|
||||
|
||||
if (name.contains("/")) {
|
||||
// disallow '/' to avoid accidental filesystem traversal
|
||||
throw new IllegalArgumentException("" + name + " must not contain slashes!");
|
||||
}
|
||||
|
||||
this.name = name;
|
||||
this.path = blacklistBasePath.resolve(name);
|
||||
this.falsePositiveProbability = falsePositiveProbability;
|
||||
|
||||
if (!Files.exists(this.path)) {
|
||||
throw new IllegalArgumentException("Password blacklist " + name + " not found!");
|
||||
}
|
||||
|
|
@ -239,6 +278,10 @@ public class BlacklistPasswordPolicyProviderFactory implements PasswordPolicyPro
|
|||
return name;
|
||||
}
|
||||
|
||||
public double getFalsePositiveProbability() {
|
||||
return falsePositiveProbability;
|
||||
}
|
||||
|
||||
public boolean contains(String password) {
|
||||
return blacklist != null && blacklist.mightContain(password);
|
||||
}
|
||||
|
|
@ -260,39 +303,46 @@ public class BlacklistPasswordPolicyProviderFactory implements PasswordPolicyPro
|
|||
private BloomFilter<String> load() {
|
||||
|
||||
try {
|
||||
LOG.infof("Loading blacklist with name %s from %s - start", name, path);
|
||||
LOG.infof("Loading blacklist start: name=%s path=%s", name, path);
|
||||
|
||||
long passwordCount = getPasswordCount();
|
||||
long passwordCount = countPasswordsInBlacklistFile();
|
||||
double fpp = getFalsePositiveProbability();
|
||||
|
||||
BloomFilter<String> filter = BloomFilter.create(
|
||||
Funnels.stringFunnel(StandardCharsets.UTF_8),
|
||||
passwordCount,
|
||||
FALSE_POSITIVE_PROBABILITY);
|
||||
fpp);
|
||||
|
||||
try (BufferedReader br = newReader(path)) {
|
||||
br.lines().forEach(filter::put);
|
||||
}
|
||||
insertPasswordsInto(filter);
|
||||
|
||||
LOG.infof("Loading blacklist with name %s from %s - end", name, path);
|
||||
double expectedFfp = filter.expectedFpp();
|
||||
LOG.infof("Loading blacklist finished: name=%s passwords=%s path=%s falsePositiveProbability=%s expectedFalsePositiveProbability=%s",
|
||||
name, passwordCount, path, fpp, expectedFfp);
|
||||
|
||||
return filter;
|
||||
} catch (IOException e) {
|
||||
throw new RuntimeException("Could not load password blacklist from path: " + path, e);
|
||||
throw new RuntimeException("Loading blacklist failed: Could not load password blacklist path=" + path, e);
|
||||
}
|
||||
}
|
||||
|
||||
protected void insertPasswordsInto(BloomFilter<String> filter) throws IOException {
|
||||
try (BufferedReader br = newReader(path)) {
|
||||
br.lines().forEach(filter::put);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Determines password blacklist size to correctly size the {@link BloomFilter} backing this blacklist.
|
||||
*
|
||||
* @return
|
||||
* @return number of passwords found in the blacklist file
|
||||
* @throws IOException
|
||||
*/
|
||||
private long getPasswordCount() throws IOException {
|
||||
private long countPasswordsInBlacklistFile() throws IOException {
|
||||
|
||||
/*
|
||||
* TODO find a more efficient way to determine the password count,
|
||||
* e.g. require a header-line in the password-blacklist file
|
||||
*/
|
||||
/*
|
||||
* TODO find a more efficient way to determine the password count,
|
||||
* e.g. require a header-line in the password-blacklist file
|
||||
*/
|
||||
try (BufferedReader br = newReader(path)) {
|
||||
return br.lines().count();
|
||||
}
|
||||
|
|
@ -305,16 +355,15 @@ public class BlacklistPasswordPolicyProviderFactory implements PasswordPolicyPro
|
|||
/**
|
||||
* Discovers password blacklists location.
|
||||
* <p>
|
||||
* The following discovery options are currently implemented:
|
||||
* <p>
|
||||
* <ol>
|
||||
* <li>
|
||||
* system property {@code keycloak.password.blacklists.path} if present
|
||||
* </li>
|
||||
* <li>SPI config property {@code blacklistsPath}</li>
|
||||
* <li>system property {@code keycloak.password.blacklists.path} if present</li>
|
||||
* <li>SPI config property {@code blacklistsPath}</li>
|
||||
* <li>fallback to the {@code /data/password-blacklists} folder of the currently running Keycloak instance</li>
|
||||
* </ol>
|
||||
* and fallback to the {@code /data/password-blacklists} folder of the currently
|
||||
* running wildfly instance.
|
||||
*
|
||||
* @param config
|
||||
* @param config spi config
|
||||
* @param defaultPathSupplier default path to use if not specified in a system prop or configuration
|
||||
* @return the detected blacklist path
|
||||
* @throws IllegalStateException if no blacklist folder could be detected
|
||||
|
|
|
|||
|
|
@ -40,5 +40,4 @@ public interface PasswordPolicyProvider extends Provider {
|
|||
throw new PasswordPolicyConfigException("Not a valid number");
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
|||
|
|
@ -30,7 +30,6 @@ import org.keycloak.policy.PasswordPolicyProvider;
|
|||
import org.keycloak.provider.ProviderFactory;
|
||||
import org.keycloak.representations.idm.RealmRepresentation;
|
||||
import org.keycloak.testsuite.AbstractKeycloakTest;
|
||||
import org.keycloak.testsuite.util.ContainerAssume;
|
||||
import org.keycloak.testsuite.util.RealmBuilder;
|
||||
|
||||
import java.io.File;
|
||||
|
|
|
|||
Loading…
Reference in a new issue