Skip to content

Commit

Permalink
Add score based password verification (opensearch-project#2557)
Browse files Browse the repository at this point in the history
Signed-off-by: Andrey Pleskach <[email protected]>
  • Loading branch information
willyborankin authored May 15, 2023
1 parent 94db5db commit 15860b6
Show file tree
Hide file tree
Showing 15 changed files with 661 additions and 178 deletions.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@ dependencies {
implementation ('org.opensaml:opensaml-saml-impl:3.4.5') {
exclude(group: 'org.apache.velocity', module: 'velocity')
}
implementation "com.nulab-inc:zxcvbn:1.7.0"
testImplementation 'org.opensaml:opensaml-messaging-impl:3.4.5'
implementation 'org.opensaml:opensaml-messaging-api:3.4.5'
runtimeOnly 'org.opensaml:opensaml-profile-api:3.4.5'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,10 @@ public class SecurityConfigurationTests {
.roles(new Role("limited-role").indexPermissions("indices:data/read/search", "indices:data/read/get").on("user-${user.name}"));
public static final String LIMITED_USER_INDEX = "user-" + LIMITED_USER.getName();
public static final String ADDITIONAL_USER_1 = "additional00001";
public static final String ADDITIONAL_PASSWORD_1 = ADDITIONAL_USER_1;
public static final String ADDITIONAL_PASSWORD_1 = "user 1 fair password";

public static final String ADDITIONAL_USER_2 = "additional2";
public static final String ADDITIONAL_PASSWORD_2 = ADDITIONAL_USER_2;
public static final String ADDITIONAL_PASSWORD_2 = "user 2 fair password";
public static final String CREATE_USER_BODY = "{\"password\": \"%s\",\"opendistro_security_roles\": []}";
public static final String INTERNAL_USERS_RESOURCE = "_plugins/_security/api/internalusers/";
public static final String ID_1 = "one";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@
import org.opensearch.security.configuration.Salt;
import org.opensearch.security.configuration.SecurityFlsDlsIndexSearcherWrapper;
import org.opensearch.security.dlic.rest.api.SecurityRestApiActions;
import org.opensearch.security.dlic.rest.validation.PasswordValidator;
import org.opensearch.security.filter.SecurityFilter;
import org.opensearch.security.filter.SecurityRestFilter;
import org.opensearch.security.http.SecurityHttpServerTransport;
Expand Down Expand Up @@ -1043,6 +1044,19 @@ public List<Setting<?>> getSettings() {
settings.add(Setting.simpleString(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, Property.NodeScope, Property.Filtered));
settings.add(Setting.simpleString(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE, Property.NodeScope, Property.Filtered));

settings.add(
Setting.intSetting(
ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH,
-1, -1, Property.NodeScope, Property.Filtered)
);
settings.add(
Setting.simpleString(
ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH,
PasswordValidator.ScoreStrength.STRONG.name(),
PasswordValidator.ScoreStrength::fromConfiguration,
Property.NodeScope, Property.Filtered
)
);

// Compliance
settings.add(Setting.listSetting(ConfigConstants.OPENDISTRO_SECURITY_COMPLIANCE_HISTORY_WRITE_WATCHED_INDICES, Collections.emptyList(), Function.identity(), Property.NodeScope)); //not filtered here
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,9 +249,19 @@ public XContentBuilder errorsAsXContent(RestChannel channel) {
break;
case INVALID_PASSWORD:
builder.field("status", "error");
builder.field("reason", opensearchSettings.get(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE,
builder.field("reason", opensearchSettings.get(
ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE,
"Password does not match minimum criteria"));
break;
case WEAK_PASSWORD:
case SIMILAR_PASSWORD:
builder.field("status", "error");
builder.field(
"reason",
opensearchSettings.get(
ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE,
errorType.message));
break;
case WRONG_DATATYPE:
builder.field("status", "error");
builder.field("reason", ErrorType.WRONG_DATATYPE.getMessage());
Expand Down Expand Up @@ -289,8 +299,14 @@ public static enum DataType {
}

public static enum ErrorType {
NONE("ok"), INVALID_CONFIGURATION("Invalid configuration"), INVALID_PASSWORD("Invalid password"), WRONG_DATATYPE("Wrong datatype"),
BODY_NOT_PARSEABLE("Could not parse content of request."), PAYLOAD_NOT_ALLOWED("Request body not allowed for this action."),
NONE("ok"),
INVALID_CONFIGURATION("Invalid configuration"),
INVALID_PASSWORD("Invalid password"),
WEAK_PASSWORD("Weak password"),
SIMILAR_PASSWORD("Password is similar to user name"),
WRONG_DATATYPE("Wrong datatype"),
BODY_NOT_PARSEABLE("Could not parse content of request."),
PAYLOAD_NOT_ALLOWED("Request body not allowed for this action."),
PAYLOAD_MANDATORY("Request body required for this action."), SECURITY_NOT_INITIALIZED("Security index not initialized"),
NULL_ARRAY_ELEMENT("`null` is not allowed as json array element");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
package org.opensearch.security.dlic.rest.validation;

import java.util.Map;
import java.util.regex.Pattern;

import org.opensearch.common.Strings;
import org.opensearch.common.bytes.BytesReference;
Expand All @@ -22,17 +21,21 @@
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.rest.RestRequest;
import org.opensearch.security.ssl.util.Utils;
import org.opensearch.security.support.ConfigConstants;

/**
* Validator for validating password and hash present in the payload
*/
public class CredentialsValidator extends AbstractConfigurationValidator {

public CredentialsValidator(final RestRequest request, BytesReference ref, final Settings opensearchSettings,
private final PasswordValidator passwordValidator;

public CredentialsValidator(final RestRequest request,
final BytesReference ref,
final Settings opensearchSettings,
Object... param) {
super(request, ref, opensearchSettings, param);
this.payloadMandatory = true;
this.passwordValidator = PasswordValidator.of(opensearchSettings);
allowedKeys.put("hash", DataType.STRING);
allowedKeys.put("password", DataType.STRING);
}
Expand All @@ -46,49 +49,29 @@ public boolean validate() {
if (!super.validate()) {
return false;
}

final String regex = this.opensearchSettings.get(ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, null);

if ((request.method() == RestRequest.Method.PUT || request.method() == RestRequest.Method.PATCH)
&& this.content != null
&& this.content.length() > 1) {
try {
final Map<String, Object> contentAsMap = XContentHelper.convertToMap(this.content, false, XContentType.JSON).v2();
String password = (String) contentAsMap.get("password");
final String password = (String) contentAsMap.get("password");
if (password != null) {
// Password is not allowed to be empty if present.
if (password.isEmpty()) {
this.errorType = ErrorType.INVALID_PASSWORD;
return false;
}

if (!Strings.isNullOrEmpty(regex)) {
// Password can be null for an existing user. Regex will validate password if present
if (!Pattern.compile("^"+regex+"$").matcher(password).matches()) {
if(log.isDebugEnabled()) {
log.debug("Regex does not match password");
}
this.errorType = ErrorType.INVALID_PASSWORD;
return false;
}

final String username = Utils.coalesce(request.param("name"), hasParams() ? (String) param[0] : null);
final boolean isDebugEnabled = log.isDebugEnabled();

if (username == null || username.isEmpty()) {
if (isDebugEnabled) {
log.debug("Unable to validate username because no user is given");
}
return false;
}

if (username.toLowerCase().equals(password.toLowerCase())) {
if (isDebugEnabled) {
log.debug("Username must not match password");
}
this.errorType = ErrorType.INVALID_PASSWORD;
return false;
final String username = Utils.coalesce(request.param("name"), hasParams() ? (String) param[0] : null);
if (Strings.isNullOrEmpty(username)) {
if (log.isDebugEnabled()) {
log.debug("Unable to validate username because no user is given");
}
return false;
}
final ErrorType passwordValidationResult = passwordValidator.validate(username, password);
if (passwordValidationResult != ErrorType.NONE) {
this.errorType = passwordValidationResult;
return false;
}
}
} catch (NotXContentException e) {
Expand All @@ -99,4 +82,5 @@ public boolean validate() {
}
return true;
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

package org.opensearch.security.dlic.rest.validation;

import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.StringJoiner;
import java.util.function.Predicate;
import java.util.regex.Pattern;

import com.google.common.collect.ImmutableList;
import com.nulabinc.zxcvbn.Strength;
import com.nulabinc.zxcvbn.Zxcvbn;
import com.nulabinc.zxcvbn.matchers.Match;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import org.opensearch.common.Strings;
import org.opensearch.common.settings.Settings;
import org.opensearch.security.dlic.rest.validation.AbstractConfigurationValidator.ErrorType;

import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_MIN_LENGTH;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX;

public class PasswordValidator {

private static final int MAX_LENGTH = 100;

/**
* Checks a username similarity and a password
* names and passwords like:
* - some_user_name/456Some_uSer_Name_1234
* - some_user_name/some_user_name_Ydfge
* - some_user_name/eman_resu_emos
* are similar
* "user_inputs" - is a default dictionary zxcvbn creates for checking similarity
*/
private final static Predicate<Match> USERNAME_SIMILARITY_CHECK = m ->
m.pattern == com.nulabinc.zxcvbn.Pattern.Dictionary && "user_inputs".equals(m.dictionaryName);

private final Logger logger = LogManager.getLogger(this.getClass());

private final int minPasswordLength;

private final Pattern passwordRegexpPattern;

private final ScoreStrength scoreStrength;

private final Zxcvbn zxcvbn;

private PasswordValidator(final int minPasswordLength,
final Pattern passwordRegexpPattern,
final ScoreStrength scoreStrength) {
this.minPasswordLength = minPasswordLength;
this.passwordRegexpPattern = passwordRegexpPattern;
this.scoreStrength = scoreStrength;
this.zxcvbn = new Zxcvbn();
}

public static PasswordValidator of(final Settings settings) {
final String passwordRegex = settings.get(SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX, null);
final ScoreStrength scoreStrength = ScoreStrength.fromConfiguration(
settings.get(SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH, ScoreStrength.STRONG.name())
);
final int minPasswordLength = settings.getAsInt(SECURITY_RESTAPI_PASSWORD_MIN_LENGTH, -1);
return new PasswordValidator(
minPasswordLength,
!Strings.isNullOrEmpty(passwordRegex) ? Pattern.compile(String.format("^%s$", passwordRegex)) : null,
scoreStrength);
}

ErrorType validate(final String username, final String password) {
if (minPasswordLength > 0 && password.length() < minPasswordLength) {
logger.debug(
"Password is too short, the minimum required length is {}, but current length is {}",
minPasswordLength,
password.length()
);
return ErrorType.INVALID_PASSWORD;
}
if (password.length() > MAX_LENGTH) {
logger.debug(
"Password is too long, the maximum required length is {}, but current length is {}",
MAX_LENGTH,
password.length()
);
return ErrorType.INVALID_PASSWORD;
}
if (Objects.nonNull(passwordRegexpPattern)
&& !passwordRegexpPattern.matcher(password).matches()) {
logger.debug("Regex does not match password");
return ErrorType.INVALID_PASSWORD;
}
final Strength strength = zxcvbn.measure(password, ImmutableList.of(username));
if (strength.getScore() < scoreStrength.score()) {
logger.debug(
"Password is weak the required score is {}, but current is {}",
scoreStrength,
ScoreStrength.fromScore(strength.getScore())
);
return ErrorType.WEAK_PASSWORD;
}
final boolean similar = strength.getSequence()
.stream()
.anyMatch(USERNAME_SIMILARITY_CHECK);
if (similar) {
logger.debug("Password is too similar to the user name {}", username);
return ErrorType.SIMILAR_PASSWORD;
}
return ErrorType.NONE;
}

public enum ScoreStrength {

// The weak score defines here only for debugging information
// and doesn't use as a configuration setting value.
WEAK(0, "too guessable: risky password"),
FAIR(1, "very guessable: protection from throttled online attacks"),
GOOD(2, "somewhat guessable: protection from unthrottled online attacks"),
STRONG(3, "safely unguessable: moderate protection from offline slow-hash scenario"),
VERY_STRONG(4, "very unguessable: strong protection from offline slow-hash scenario");

private final int score;

private final String description;

static final List<ScoreStrength> CONFIGURATION_VALUES = ImmutableList.of(FAIR, STRONG, VERY_STRONG);

static final String EXPECTED_CONFIGURATION_VALUES =
new StringJoiner(",")
.add(FAIR.name().toLowerCase(Locale.ROOT))
.add(STRONG.name().toLowerCase(Locale.ROOT))
.add(VERY_STRONG.name().toLowerCase(Locale.ROOT))
.toString();

private ScoreStrength(final int score, final String description) {
this.score = score;
this.description = description;
}

public static ScoreStrength fromScore(final int score) {
for (final ScoreStrength strength : values()) {
if (strength.score == score)
return strength;
}
throw new IllegalArgumentException("Unknown score " + score);
}

public static ScoreStrength fromConfiguration(final String value) {
for (final ScoreStrength strength : CONFIGURATION_VALUES) {
if (strength.name().equalsIgnoreCase(value))
return strength;
}
throw new IllegalArgumentException(
String.format(
"Setting [%s] cannot be used with the configured: %s. Expected one of [%s]",
SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH,
value,
EXPECTED_CONFIGURATION_VALUES
)
);
}

@Override
public String toString() {
return String.format("Password strength score %s. %s", score, description);
}

public int score() {
return this.score;
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,9 @@ public enum RolesMappingResolution {
public static final String SECURITY_RESTAPI_ENDPOINTS_DISABLED = "plugins.security.restapi.endpoints_disabled";
public static final String SECURITY_RESTAPI_PASSWORD_VALIDATION_REGEX = "plugins.security.restapi.password_validation_regex";
public static final String SECURITY_RESTAPI_PASSWORD_VALIDATION_ERROR_MESSAGE = "plugins.security.restapi.password_validation_error_message";

public static final String SECURITY_RESTAPI_PASSWORD_MIN_LENGTH = "plugins.security.restapi.password_min_length";
public static final String SECURITY_RESTAPI_PASSWORD_SCORE_BASED_VALIDATION_STRENGTH = "plugins.security.restapi.password_score_based_validation_strength";
// Illegal Opcodes from here on
public static final String SECURITY_UNSUPPORTED_DISABLE_REST_AUTH_INITIALLY = "plugins.security.unsupported.disable_rest_auth_initially";
public static final String SECURITY_UNSUPPORTED_DISABLE_INTERTRANSPORT_AUTH_INITIALLY = "plugins.security.unsupported.disable_intertransport_auth_initially";
public static final String SECURITY_UNSUPPORTED_PASSIVE_INTERTRANSPORT_AUTH_INITIALLY = "plugins.security.unsupported.passive_intertransport_auth_initially";
Expand Down
Loading

0 comments on commit 15860b6

Please sign in to comment.