security: do not allow built-in user/role names to be defined in the file realm

This change restricts built-in user/role names from passing validation when we are reading or modifying
the files used by this realm.

Closes elastic/elasticsearch#2078

Original commit: elastic/x-pack-elasticsearch@9f6b34f39d
This commit is contained in:
jaymode 2016-08-31 07:39:35 -04:00
parent 74f55bf46e
commit 2e010d52e9
13 changed files with 60 additions and 46 deletions

View File

@ -519,9 +519,6 @@ public abstract class MonitoringIntegTestCase extends ESIntegTestCase {
"\n" +
"admin:\n" +
" cluster: [ 'cluster:monitor/nodes/info', 'cluster:monitor/nodes/liveness' ]\n" +
"transport_client:\n" +
" cluster: [ 'cluster:monitor/nodes/info', 'cluster:monitor/nodes/liveness' ]\n" +
"\n" +
"monitor:\n" +
" cluster: [ 'cluster:monitor/nodes/info', 'cluster:monitor/nodes/liveness' ]\n"
;

View File

@ -30,10 +30,7 @@ public class AuthenticateRequest extends ActionRequest<AuthenticateRequest> impl
@Override
public ActionRequestValidationException validate() {
Validation.Error error = Validation.Users.validateUsername(username);
if (error != null) {
return addValidationError(error.toString(), null);
}
// we cannot apply our validation rules here as an authenticate request could be for an LDAP user that doesn't fit our restrictions
return null;
}

View File

@ -80,9 +80,6 @@ public class FileUserPasswdStore {
}
public boolean verifyPassword(String username, SecuredString password) {
if (users == null) {
return false;
}
char[] hash = users.get(username);
return hash != null && hasher.verify(password, hash);
}

View File

@ -143,7 +143,7 @@ public class FileUserRolesStore {
continue;
}
String role = line.substring(0, i).trim();
Validation.Error validationError = Validation.Roles.validateRoleName(role);
Validation.Error validationError = Validation.Roles.validateRoleName(role, true);
if (validationError != null) {
logger.error("invalid role entry in users_roles file [{}], line [{}] - {}. skipping...", path.toAbsolutePath(), lineNr,
validationError);

View File

@ -446,7 +446,7 @@ public class UsersTool extends MultiCommand {
}
String[] roles = rolesStr.split(",");
for (String role : roles) {
Validation.Error validationError = Validation.Roles.validateRoleName(role);
Validation.Error validationError = Validation.Roles.validateRoleName(role, true);
if (validationError != null) {
throw new UserException(ExitCodes.DATA_ERROR, "Invalid role [" + role + "]... " + validationError);
}

View File

@ -168,7 +168,7 @@ public class RoleDescriptor implements ToXContent {
public static RoleDescriptor parse(String name, XContentParser parser) throws IOException {
// validate name
Validation.Error validationError = Validation.Roles.validateRoleName(name);
Validation.Error validationError = Validation.Roles.validateRoleName(name, true);
if (validationError != null) {
ValidationException ve = new ValidationException();
ve.addValidationError(validationError.toString());

View File

@ -5,6 +5,9 @@
*/
package org.elasticsearch.xpack.security.support;
import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
import org.elasticsearch.xpack.security.authz.store.ReservedRolesStore;
import java.util.regex.Pattern;
/**
@ -19,12 +22,16 @@ public final class Validation {
private static final int MIN_PASSWD_LENGTH = 6;
public static Error validateUsername(String username) {
return COMMON_NAME_PATTERN.matcher(username).matches() ?
null :
new Error("A valid username must be at least 1 character and no longer than 30 characters. " +
"It must begin with a letter (`a-z` or `A-Z`) or an underscore (`_`). Subsequent " +
"characters can be letters, underscores (`_`), digits (`0-9`) or any of the following " +
"symbols `@`, `-`, `.` or `$`");
if (COMMON_NAME_PATTERN.matcher(username).matches() == false) {
return new Error("A valid username must be at least 1 character and no longer than 30 characters. " +
"It must begin with a letter (`a-z` or `A-Z`) or an underscore (`_`). Subsequent " +
"characters can be letters, underscores (`_`), digits (`0-9`) or any of the following " +
"symbols `@`, `-`, `.` or `$`");
}
if (ReservedRealm.isReserved(username)) {
return new Error("Username [" + username + "] is reserved and may not be used.");
}
return null;
}
public static Error validatePassword(char[] password) {
@ -38,12 +45,20 @@ public final class Validation {
public static final class Roles {
public static Error validateRoleName(String roleName) {
return COMMON_NAME_PATTERN.matcher(roleName).matches() ?
null :
new Error("A valid role name must be at least 1 character and no longer than 30 characters. " +
"It must begin with a letter (`a-z` or `A-Z`) or an underscore (`_`). Subsequent " +
"characters can be letters, underscores (`_`), digits (`0-9`) or any of the following " +
"symbols `@`, `-`, `.` or `$`");
return validateRoleName(roleName, false);
}
public static Error validateRoleName(String roleName, boolean allowReserved) {
if (COMMON_NAME_PATTERN.matcher(roleName).matches() == false) {
return new Error("A valid role name must be at least 1 character and no longer than 30 characters. " +
"It must begin with a letter (`a-z` or `A-Z`) or an underscore (`_`). Subsequent " +
"characters can be letters, underscores (`_`), digits (`0-9`) or any of the following " +
"symbols `@`, `-`, `.` or `$`");
}
if (allowReserved == false && ReservedRolesStore.isReserved(roleName)) {
return new Error("Role [" + roleName + "] is reserved and may not be used.");
}
return null;
}
}

View File

@ -43,11 +43,6 @@ public class PermissionPrecedenceTests extends SecurityIntegTestCase {
" - names: '*'\n" +
" privileges: [ all ]" +
"\n" +
"transport_client:\n" +
" cluster:\n" +
" - cluster:monitor/nodes/info\n" +
" - cluster:monitor/state\n" +
"\n" +
"user:\n" +
" indices:\n" +
" - names: 'test_*'\n" +

View File

@ -57,7 +57,7 @@ public class SecuritySettingsSource extends ClusterDiscoveryConfiguration.Unicas
public static final String DEFAULT_PASSWORD_HASHED = new String(Hasher.BCRYPT.hash(new SecuredString(DEFAULT_PASSWORD.toCharArray())));
public static final String DEFAULT_ROLE = "user";
public static final String DEFAULT_TRANSPORT_CLIENT_ROLE = "trans_client_user";
public static final String DEFAULT_TRANSPORT_CLIENT_ROLE = "transport_client";
public static final String DEFAULT_TRANSPORT_CLIENT_USER_NAME = "test_trans_client_user";
public static final String CONFIG_STANDARD_USER =
@ -73,10 +73,7 @@ public class SecuritySettingsSource extends ClusterDiscoveryConfiguration.Unicas
" cluster: [ ALL ]\n" +
" indices:\n" +
" - names: '*'\n" +
" privileges: [ ALL ]\n" +
DEFAULT_TRANSPORT_CLIENT_ROLE + ":\n" +
" cluster:\n" +
" - transport_client";
" privileges: [ ALL ]\n";
private final Path parentFolder;
private final String subfolderPrefix;

View File

@ -39,8 +39,6 @@ public class RunAsIntegTests extends SecurityIntegTestCase {
static final String RUN_AS_USER = "run_as_user";
static final String TRANSPORT_CLIENT_USER = "transport_user";
static final String ROLES =
"transport_client:\n" +
" cluster: [ 'cluster:monitor/nodes/liveness' ]\n" +
"run_as_role:\n" +
" run_as: [ '" + SecuritySettingsSource.DEFAULT_USER_NAME + "', 'idontexist' ]\n";

View File

@ -7,7 +7,6 @@ package org.elasticsearch.xpack.security.authz.store;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.core.LogEvent;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.xpack.XPackPlugin;
@ -361,14 +360,14 @@ public class FileRolesStoreTests extends ESTestCase {
assertThat(roles, hasKey("admin"));
List<String> events = CapturingLogger.output(logger.getName(), Level.WARN);
List<String> events = CapturingLogger.output(logger.getName(), Level.ERROR);
assertThat(events, notNullValue());
assertThat(events, hasSize(4));
// the system role will always be checked first
assertThat(events.get(0), containsString("role [_system] is reserved"));
assertThat(events.get(1), containsString("role [superuser] is reserved"));
assertThat(events.get(2), containsString("role [kibana] is reserved"));
assertThat(events.get(3), containsString("role [transport_client] is reserved"));
assertThat(events.get(0), containsString("Role [_system] is reserved"));
assertThat(events.get(1), containsString("Role [superuser] is reserved"));
assertThat(events.get(2), containsString("Role [kibana] is reserved"));
assertThat(events.get(3), containsString("Role [transport_client] is reserved"));
}
public void testUsageStats() throws Exception {

View File

@ -5,11 +5,16 @@
*/
package org.elasticsearch.xpack.security.support;
import org.elasticsearch.xpack.security.authz.store.ReservedRolesStore;
import org.elasticsearch.xpack.security.support.Validation.Error;
import org.elasticsearch.xpack.security.support.Validation.Users;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.security.user.ElasticUser;
import org.elasticsearch.xpack.security.user.KibanaUser;
import java.util.Arrays;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.nullValue;
@ -53,6 +58,13 @@ public class ValidationTests extends ESTestCase {
assertThat(Users.validateUsername(name), nullValue());
}
public void testReservedUsernames() {
final String username = randomFrom(ElasticUser.NAME, KibanaUser.NAME);
final Error error = Users.validateUsername(username);
assertNotNull(error);
assertThat(error.toString(), containsString("is reserved"));
}
public void testUsersValidateUsernameInvalidLength() throws Exception {
int length = frequently() ? randomIntBetween(31, 200) : 0; // invalid length
char[] name = new char[length];
@ -84,6 +96,16 @@ public class ValidationTests extends ESTestCase {
assertThat(Validation.Roles.validateRoleName(name), nullValue());
}
public void testReservedRoleName() {
final String rolename = randomFrom(ReservedRolesStore.names());
final Error error = Validation.Roles.validateRoleName(rolename);
assertNotNull(error);
assertThat(error.toString(), containsString("is reserved"));
final Error allowed = Validation.Roles.validateRoleName(rolename, true);
assertNull(allowed);
}
public void testRolesValidateRoleNameInvalidLength() throws Exception {
int length = frequently() ? randomIntBetween(31, 200) : 0; // invalid length
char[] name = new char[length];

View File

@ -676,9 +676,6 @@ public abstract class AbstractWatcherIntegrationTestCase extends ESIntegTestCase
"\n" +
"admin:\n" +
" cluster: [ 'manage' ]\n" +
"transport_client:\n" +
" cluster: [ 'transport_client' ]\n" +
"\n" +
"monitor:\n" +
" cluster: [ 'monitor' ]\n"
;