Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make user and role name constraint consistent with max document ID. #86728

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
85fcd04
Make user and role name constraint consistent with max document ID.
slobodanadamovic May 12, 2022
7ef0f1d
Merge branch 'master' of github.com:elastic/elasticsearch into fix-us…
slobodanadamovic May 12, 2022
bc621a6
Update docs/changelog/86728.yaml
slobodanadamovic May 12, 2022
edaa8eb
Merge branch 'master' of github.com:elastic/elasticsearch into fix-us…
slobodanadamovic May 16, 2022
974cb62
Address review comments.
slobodanadamovic May 16, 2022
092c580
Remove already moved FileRealmValidationUtil.
slobodanadamovic May 16, 2022
a23e522
Validate role name not just its presence.
slobodanadamovic May 16, 2022
edcdc6f
Revert changes.
slobodanadamovic May 17, 2022
397211e
Validate PutRoleRequest.
slobodanadamovic May 17, 2022
ca5711f
Merge branch 'master' of github.com:elastic/elasticsearch into fix-us…
slobodanadamovic May 17, 2022
e4583c1
Merge branch 'master' of github.com:elastic/elasticsearch into fix-us…
slobodanadamovic May 17, 2022
b871c7c
Merge branch 'master' of github.com:elastic/elasticsearch into fix-us…
slobodanadamovic May 18, 2022
657d199
Add more validation and tests for roles stored in native realm.
slobodanadamovic May 18, 2022
0e72202
Spotless.
slobodanadamovic May 18, 2022
7316c6e
Fix failing test after refactoring role validation.
slobodanadamovic May 18, 2022
37599d5
Merge branch 'master' of github.com:elastic/elasticsearch into fix-us…
slobodanadamovic May 18, 2022
95f2a4e
Fix failing tests.
slobodanadamovic May 18, 2022
12e060b
Fix failing.
slobodanadamovic May 18, 2022
4e61f2b
Merge branch 'master' of github.com:elastic/elasticsearch into fix-us…
slobodanadamovic May 19, 2022
c539f3a
Test Validation methods which use default max name size.
slobodanadamovic May 19, 2022
ef60679
Consolidate random username generation methods into one.
slobodanadamovic May 19, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/86728.yaml
@@ -0,0 +1,6 @@
pr: 86728
summary: Make user and role name constraint consistent with max document ID
area: Security
type: bug
issues:
- 66020
Expand Up @@ -76,6 +76,11 @@ public class IndexRequest extends ReplicatedWriteRequest<IndexRequest> implement
*/
static final int MAX_SOURCE_LENGTH_IN_TOSTRING = 2048;

/**
* Maximal allowed length (in bytes) of the document ID.
*/
public static final int MAX_DOCUMENT_ID_LENGTH_IN_BYTES = 512;

private static final ShardId NO_SHARD_ID = null;

private String id;
Expand Down Expand Up @@ -218,9 +223,14 @@ public ActionRequestValidationException validate() {

validationException = DocWriteRequest.validateSeqNoBasedCASParams(this, validationException);

if (id != null && id.getBytes(StandardCharsets.UTF_8).length > 512) {
if (id != null && id.getBytes(StandardCharsets.UTF_8).length > MAX_DOCUMENT_ID_LENGTH_IN_BYTES) {
validationException = addValidationError(
"id [" + id + "] is too long, must be no longer than 512 bytes but was: " + id.getBytes(StandardCharsets.UTF_8).length,
"id ["
+ id
+ "] is too long, must be no longer than "
+ MAX_DOCUMENT_ID_LENGTH_IN_BYTES
+ " bytes but was: "
+ id.getBytes(StandardCharsets.UTF_8).length,
validationException
);
}
Expand Down Expand Up @@ -381,7 +391,7 @@ public IndexRequest source(Map<String, ?> source, XContentType contentType) thro
/**
* Sets the document source to index.
*
* Note, its preferable to either set it using {@link #source(org.elasticsearch.common.xcontent.XContentBuilder)}
* Note, its preferable to either set it using {@link #source(org.elasticsearch.xcontent.XContentBuilder)}
* or using the {@link #source(byte[], XContentType)}.
*/
public IndexRequest source(String source, XContentType xContentType) {
Expand Down
2 changes: 1 addition & 1 deletion x-pack/docs/en/rest-api/security/create-users.asciidoc
Expand Up @@ -41,7 +41,7 @@ For more information about the native realm, see
+
--
[[username-validation]]
NOTE: Usernames must be at least 1 and no more than 1024 characters. They can
NOTE: Usernames must be at least 1 and no more than 507 characters. They can
contain alphanumeric characters (`a-z`, `A-Z`, `0-9`), spaces, punctuation, and
printable symbols in the {wikipedia}/Basic_Latin_(Unicode_block)[Basic Latin (ASCII) block]. Leading or trailing whitespace is not allowed.

Expand Down
Expand Up @@ -33,7 +33,7 @@ A role is defined by the following JSON structure:
<5> A list of application privilege entries. This field is optional.

[[valid-role-name]]
NOTE: Role names must be at least 1 and no more than 1024 characters. They can
NOTE: Role names must be at least 1 and no more than 507 characters. They can
contain alphanumeric characters (`a-z`, `A-Z`, `0-9`), spaces,
punctuation, and printable symbols in the {wikipedia}/Basic_Latin_(Unicode_block)[Basic Latin (ASCII) block].
Leading or trailing whitespace is not allowed.
Expand Down
Expand Up @@ -17,6 +17,8 @@
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor;
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege;
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivileges;
import org.elasticsearch.xpack.core.security.support.NativeRealmValidationUtil;
import org.elasticsearch.xpack.core.security.support.Validation;

import java.io.IOException;
import java.util.ArrayList;
Expand All @@ -25,6 +27,8 @@
import java.util.List;
import java.util.Map;

import static org.elasticsearch.action.ValidateActions.addValidationError;

/**
* Request object for adding a role to the security index
*/
Expand Down Expand Up @@ -59,7 +63,12 @@ public PutRoleRequest() {}

@Override
public ActionRequestValidationException validate() {
return RoleDescriptorRequestValidator.validate(roleDescriptor());
ActionRequestValidationException validationException = null;
Validation.Error error = NativeRealmValidationUtil.validateRoleName(this.name, false);
if (error != null) {
validationException = addValidationError(error.toString(), validationException);
}
return RoleDescriptorRequestValidator.validate(roleDescriptor(), validationException);
}

public void name(String name) {
Expand Down
Expand Up @@ -12,8 +12,8 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.xpack.core.security.support.NativeRealmValidationUtil;
import org.elasticsearch.xpack.core.security.support.Validation.Error;
import org.elasticsearch.xpack.core.security.support.Validation.Users;

import java.io.IOException;

Expand All @@ -40,7 +40,7 @@ public SetEnabledRequest(StreamInput in) throws IOException {
@Override
public ActionRequestValidationException validate() {
ActionRequestValidationException validationException = null;
Error error = Users.validateUsername(username, true, Settings.EMPTY);
Error error = NativeRealmValidationUtil.validateUsername(username, true, Settings.EMPTY);
if (error != null) {
validationException = addValidationError(error.toString(), validationException);
}
Expand Down
@@ -0,0 +1,42 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

package org.elasticsearch.xpack.core.security.support;

import org.elasticsearch.common.settings.Settings;

/**
* Provides utility methods for validating user and role names stored in the native realm.
* Note: This class is a wrapper of the {@link Validation} which allow names to have {@link #MAX_NAME_LENGTH}.
*/
public final class NativeRealmValidationUtil {

/**
* In the native realm, the maximal user and role name lenght is influenced by the maximal document ID
* (see {@link org.elasticsearch.action.index.IndexRequest#MAX_DOCUMENT_ID_LENGTH_IN_BYTES}).
* Because the document IDs are prefixed by either {@code user-} or {@code role-},
* the actual possible max length is 507 chars.
*
* Note: If we choose (in the future) to allow more than 507 chars we should
* either consider increasing the max allowed size for document ID or
* consider hashing names longer than 507 in order to fit into document ID.
*/
slobodanadamovic marked this conversation as resolved.
Show resolved Hide resolved
public static final int MAX_NAME_LENGTH = 507;

public static Validation.Error validateUsername(String username, boolean allowReserved, Settings settings) {
return Validation.Users.validateUsername(username, allowReserved, settings, MAX_NAME_LENGTH);
}

public static Validation.Error validateRoleName(String roleName, boolean allowReserved) {
return Validation.Roles.validateRoleName(roleName, allowReserved, MAX_NAME_LENGTH);
}

private NativeRealmValidationUtil() {
throw new IllegalAccessError("Not allowed!");
}

}
Expand Up @@ -20,7 +20,7 @@ public final class Validation {
static final int MIN_NAME_LENGTH = 1;
static final int MAX_NAME_LENGTH = 1024;

static final Set<Character> VALID_NAME_CHARS = Set.of(
public static final Set<Character> VALID_NAME_CHARS = Set.of(
' ',
'!',
'"',
Expand Down Expand Up @@ -120,9 +120,7 @@ public final class Validation {

private static final String INVALID_NAME_MESSAGE = "%1s names must be at least "
+ MIN_NAME_LENGTH
+ " and no more than "
+ MAX_NAME_LENGTH
+ " characters. "
+ " and no more than %s characters. "
+ "They can contain alphanumeric characters (a-z, A-Z, 0-9), spaces, punctuation, and printable symbols in the "
+ "Basic Latin (ASCII) block. Leading or trailing whitespace is not allowed.";

Expand All @@ -132,8 +130,10 @@ public final class Validation {
+ "and at most 256 characters that are alphanumeric (A-Z, a-z, 0-9) or hyphen (-) or underscore (_). "
+ "It must not begin with an underscore (_).";

private static boolean isValidUserOrRoleName(String name) {
if (name.length() < MIN_NAME_LENGTH || name.length() > MAX_NAME_LENGTH) {
private static boolean isValidUserOrRoleName(String name, int maxLength) {
assert maxLength >= MIN_NAME_LENGTH : "maxLength cannot be less than " + MIN_NAME_LENGTH;

if (name.length() < MIN_NAME_LENGTH || name.length() > maxLength) {
return false;
}

Expand Down Expand Up @@ -178,8 +178,15 @@ public static final class Users {
* @return {@code null} if valid
*/
public static Error validateUsername(String username, boolean allowReserved, Settings settings) {
if (isValidUserOrRoleName(username) == false) {
return new Error(String.format(Locale.ROOT, INVALID_NAME_MESSAGE, "User"));
return validateUsername(username, allowReserved, settings, MAX_NAME_LENGTH);
}

static Error validateUsername(String username, boolean allowReserved, Settings settings, int maxLength) {
if (username == null) {
return new Error("username is missing");
}
if (isValidUserOrRoleName(username, maxLength) == false) {
return new Error(String.format(Locale.ROOT, INVALID_NAME_MESSAGE, "User", maxLength));
}
if (allowReserved == false && ClientReservedRealm.isReserved(username, settings)) {
return new Error("Username [" + username + "] is reserved and may not be used.");
Expand All @@ -197,13 +204,16 @@ public static Error validatePassword(SecureString password) {

public static final class Roles {

public static Error validateRoleName(String roleName) {
return validateRoleName(roleName, false);
public static Error validateRoleName(String roleName, boolean allowReserved) {
return validateRoleName(roleName, allowReserved, MAX_NAME_LENGTH);
}

public static Error validateRoleName(String roleName, boolean allowReserved) {
if (isValidUserOrRoleName(roleName) == false) {
return new Error(String.format(Locale.ROOT, INVALID_NAME_MESSAGE, "Role"));
static Error validateRoleName(String roleName, boolean allowReserved, int maxLength) {
if (roleName == null) {
return new Error("role name is missing");
}
if (isValidUserOrRoleName(roleName, maxLength) == false) {
return new Error(String.format(Locale.ROOT, INVALID_NAME_MESSAGE, "Role", maxLength));
}
if (allowReserved == false && ReservedRolesStore.isReserved(roleName)) {
return new Error("Role [" + roleName + "] is reserved and may not be used.");
Expand Down
Expand Up @@ -23,6 +23,7 @@
import org.elasticsearch.xpack.core.security.authz.RoleDescriptor.ApplicationResourcePrivileges;
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege;
import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivileges;
import org.elasticsearch.xpack.core.security.support.NativeRealmValidationUtil;

import java.io.IOException;
import java.util.Arrays;
Expand All @@ -49,6 +50,17 @@ public void testValidationErrorWithUnknownClusterPrivilegeName() {
assertValidationError("unknown cluster privilege [" + unknownClusterPrivilegeName.toLowerCase(Locale.ROOT) + "]", request);
}

public void testValidationErrorWithTooLongRoleName() {
final PutRoleRequest request = new PutRoleRequest();
request.name(
randomAlphaOfLengthBetween(NativeRealmValidationUtil.MAX_NAME_LENGTH + 1, NativeRealmValidationUtil.MAX_NAME_LENGTH * 2)
);
request.cluster("manage_security");

// Fail
assertValidationError("Role names must be at least 1 and no more than " + NativeRealmValidationUtil.MAX_NAME_LENGTH, request);
}

public void testValidationSuccessWithCorrectClusterPrivilegeName() {
final PutRoleRequest request = new PutRoleRequest();
request.name(randomAlphaOfLengthBetween(4, 9));
Expand Down
Expand Up @@ -133,6 +133,8 @@ public class ValidationTests extends ESTestCase {
'\r'
);

private static final int CUSTOM_MAX_NAME_LENGTH = 507;

public void testUsernameValid() throws Exception {
int length = randomIntBetween(Validation.MIN_NAME_LENGTH, Validation.MAX_NAME_LENGTH);
String name = new String(generateValidName(length));
Expand All @@ -147,7 +149,7 @@ public void testUsernameReserved() {
}

public void testUsernameInvalidLength() throws Exception {
int length = frequently() ? randomIntBetween(Validation.MAX_NAME_LENGTH + 1, 2048) : 0;
int length = frequently() ? randomIntBetween(Validation.MAX_NAME_LENGTH + 1, Validation.MAX_NAME_LENGTH * 2) : 0;
char[] name = new char[length];
if (length > 0) {
name = generateValidName(length);
Expand All @@ -167,25 +169,15 @@ public void testUsernameInvalidWhitespace() throws Exception {
assertThat(Users.validateUsername(name, false, Settings.EMPTY), notNullValue());
}

public void testUsersValidatePassword() throws Exception {
SecureString passwd = new SecureString(randomAlphaOfLength(randomIntBetween(0, 20)).toCharArray());
logger.info("{}[{}]", passwd, passwd.length());
if (passwd.length() >= 6) {
assertThat(Users.validatePassword(passwd), nullValue());
} else {
assertThat(Users.validatePassword(passwd), notNullValue());
}
}

public void testRoleNameValid() throws Exception {
int length = randomIntBetween(Validation.MIN_NAME_LENGTH, Validation.MAX_NAME_LENGTH);
String name = new String(generateValidName(length));
assertThat(Roles.validateRoleName(name), nullValue());
assertThat(Roles.validateRoleName(name, false), nullValue());
}

public void testRoleNameReserved() {
final String rolename = randomFrom(ReservedRolesStore.names());
final Error error = Roles.validateRoleName(rolename);
final Error error = Roles.validateRoleName(rolename, false, Validation.MAX_NAME_LENGTH);
assertNotNull(error);
assertThat(error.toString(), containsString("is reserved"));

Expand All @@ -194,7 +186,7 @@ public void testRoleNameReserved() {
}

public void testRoleNameInvalidLength() throws Exception {
int length = frequently() ? randomIntBetween(Validation.MAX_NAME_LENGTH + 1, 2048) : 0;
int length = frequently() ? randomIntBetween(Validation.MAX_NAME_LENGTH + 1, Validation.MAX_NAME_LENGTH * 2) : 0;
char[] name = new char[length];
if (length > 0) {
name = generateValidName(length);
Expand All @@ -214,6 +206,87 @@ public void testRoleNameInvalidWhitespace() throws Exception {
assertThat(Roles.validateRoleName(name, false), notNullValue());
}

public void testUsernameValidWithCustomLength() {
int length = randomIntBetween(Validation.MIN_NAME_LENGTH, CUSTOM_MAX_NAME_LENGTH);
String name = new String(generateValidName(length));
assertThat(Users.validateUsername(name, false, Settings.EMPTY, CUSTOM_MAX_NAME_LENGTH), nullValue());
}

public void testUsernameReservedWithCustomLength() {
final String username = randomFrom(ElasticUser.NAME, KibanaUser.NAME);
final Error error = Users.validateUsername(username, false, Settings.EMPTY, CUSTOM_MAX_NAME_LENGTH);
assertNotNull(error);
assertThat(error.toString(), containsString("is reserved"));
}

public void testUsernameInvalidLengthWithCustomLength() {
int length = frequently() ? randomIntBetween(CUSTOM_MAX_NAME_LENGTH + 1, CUSTOM_MAX_NAME_LENGTH * 2) : 0;
char[] name = new char[length];
if (length > 0) {
name = generateValidName(length);
}
assertThat(Users.validateUsername(new String(name), false, Settings.EMPTY, CUSTOM_MAX_NAME_LENGTH), notNullValue());
}

public void testUsernameInvalidCharactersWithCustomLength() {
int length = randomIntBetween(Validation.MIN_NAME_LENGTH, CUSTOM_MAX_NAME_LENGTH);
String name = new String(generateNameInvalidCharacters(length));
assertThat(Users.validateUsername(name, false, Settings.EMPTY, CUSTOM_MAX_NAME_LENGTH), notNullValue());
}

public void testUsernameInvalidWhitespaceWithCustomLength() {
int length = randomIntBetween(Validation.MIN_NAME_LENGTH, CUSTOM_MAX_NAME_LENGTH);
String name = new String(generateNameInvalidWhitespace(length));
assertThat(Users.validateUsername(name, false, Settings.EMPTY, CUSTOM_MAX_NAME_LENGTH), notNullValue());
}

public void testUsersValidatePassword() {
SecureString passwd = new SecureString(randomAlphaOfLength(randomIntBetween(0, 20)).toCharArray());
logger.info("{}[{}]", passwd, passwd.length());
if (passwd.length() >= 6) {
assertThat(Users.validatePassword(passwd), nullValue());
} else {
assertThat(Users.validatePassword(passwd), notNullValue());
}
}

public void testRoleNameValidWithCustomLength() {
int length = randomIntBetween(Validation.MIN_NAME_LENGTH, CUSTOM_MAX_NAME_LENGTH);
String name = new String(generateValidName(length));
assertThat(Roles.validateRoleName(name, false, CUSTOM_MAX_NAME_LENGTH), nullValue());
}

public void testRoleNameReservedWithCustomLength() {
final String rolename = randomFrom(ReservedRolesStore.names());
final Error error = Roles.validateRoleName(rolename, false, CUSTOM_MAX_NAME_LENGTH);
assertNotNull(error);
assertThat(error.toString(), containsString("is reserved"));

final Error allowed = Roles.validateRoleName(rolename, true, CUSTOM_MAX_NAME_LENGTH);
assertNull(allowed);
}

public void testRoleNameInvalidLengthWithCustomLength() throws Exception {
int length = frequently() ? randomIntBetween(CUSTOM_MAX_NAME_LENGTH + 1, CUSTOM_MAX_NAME_LENGTH * 2) : 0;
char[] name = new char[length];
if (length > 0) {
name = generateValidName(length);
}
assertThat(Roles.validateRoleName(new String(name), false, CUSTOM_MAX_NAME_LENGTH), notNullValue());
}

public void testRoleNameInvalidCharactersWithCustomLength() throws Exception {
int length = randomIntBetween(Validation.MIN_NAME_LENGTH, CUSTOM_MAX_NAME_LENGTH);
String name = new String(generateNameInvalidCharacters(length));
assertThat(Roles.validateRoleName(name, false, CUSTOM_MAX_NAME_LENGTH), notNullValue());
}

public void testRoleNameInvalidWhitespaceWithCustomLength() throws Exception {
int length = randomIntBetween(Validation.MIN_NAME_LENGTH, CUSTOM_MAX_NAME_LENGTH);
String name = new String(generateNameInvalidWhitespace(length));
assertThat(Roles.validateRoleName(name, false, CUSTOM_MAX_NAME_LENGTH), notNullValue());
}

public void testIsValidServiceAccountTokenName() {
final String tokenName1 = ValidationTests.randomTokenName();
assertThat(Validation.isValidServiceAccountTokenName(tokenName1), is(true));
Expand Down