opensearch-project / security

🔐 Secure your cluster with TLS, numerous authentication backends, data masking, audit logging as well as role-based access control on indices, documents, and fields
https://opensearch.org/docs/latest/security-plugin/index/
Apache License 2.0
180 stars 264 forks source link

[FEATURE] is it possible to ignore basic auth `id/password` default validation #4410

Open 10000-ki opened 3 weeks ago

10000-ki commented 3 weeks ago

Is your feature request related to a problem?

public RequestContentValidator.ValidationError 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 RequestContentValidator.ValidationError.INVALID_PASSWORD_TOO_SHORT;
        }
        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 RequestContentValidator.ValidationError.INVALID_PASSWORD_TOO_LONG;
        }
        if (Objects.nonNull(passwordRegexpPattern) && !passwordRegexpPattern.matcher(password).matches()) {
            logger.debug("Regex does not match password");
            return RequestContentValidator.ValidationError.INVALID_PASSWORD_INVALID_REGEX;
        }
        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 RequestContentValidator.ValidationError.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 RequestContentValidator.ValidationError.SIMILAR_PASSWORD;
        }
        return RequestContentValidator.ValidationError.NONE;
    }

The opensearch default auth id/password validation criteria is pretty strict.

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 RequestContentValidator.ValidationError.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 RequestContentValidator.ValidationError.SIMILAR_PASSWORD;
        }

For things like scoring and similarity checks, the error messages are also vague compared to regular expression checks. making it difficult for users to know exactly what to do to fix the problem.

What solution would you like?

I'd like to see an option to make the scoring and id/password similarity check validation optional.

like this

private ValidationResult<JsonNode> validatePassword(final RestRequest request, final JsonNode jsonContent, final boolean ignore) {
        if (jsonContent.has("password") && !ignore) {
            final PasswordValidator passwordValidator = PasswordValidator.of(validationContext.settings());
            final String password = jsonContent.get("password").asText();
            if (Strings.isNullOrEmpty(password)) {
                this.validationError = ValidationError.INVALID_PASSWORD_TOO_SHORT;
                return ValidationResult.error(RestStatus.BAD_REQUEST, this);
            }
            final String username = Optional.ofNullable(request.param("name"))
                .orElseGet(() -> validationContext.hasParams() ? (String) validationContext.params()[0] : null);
            if (Strings.isNullOrEmpty(username)) {
                if (LOGGER.isDebugEnabled()) {
                    LOGGER.debug("Unable to validate username because no user is given");
                }
                this.validationError = ValidationError.NO_USERNAME;
                return ValidationResult.error(RestStatus.BAD_REQUEST, this);
            }
            this.validationError = passwordValidator.validate(username, password);
            if (this.validationError != ValidationError.NONE) {
                return ValidationResult.error(RestStatus.BAD_REQUEST, this);
            }
        }
        return ValidationResult.success(jsonContent);
    }

What alternatives have you considered? A clear and concise description of any alternative solutions or features you've considered.

Do you have any additional context? Add any other context or screenshots about the feature request here.

DarshitChanpura commented 3 weeks ago

@10000-ki you can set the setting: plugins.security.restapi.password_score_based_validation_strength to one of the values from this ScoreStrength enum as needed. You can read more about zxcvbn here.

10000-ki commented 3 weeks ago

@DarshitChanpura

I'm not just talking about scoring, but also similarity check final boolean similar = strength.getSequence().stream().anyMatch(USERNAME_SIMILARITY_CHECK)

PasswordValidation checks may not always be necessary in some situations. (test or internal user)

I thought it would be nice to give the user a choice.

scrawfor99 commented 3 weeks ago

[Triage] Hi @10000-ki, thank you for filing this issue. @willyborankin is taking a look to make sure there is no bug here and will update.

willyborankin commented 3 weeks ago

I will take a look on the similarity check. It could be that I misunderstood how the library works.