CLI Tools: Add command to check for same permissions and owners after run

In case the creation of files changed the owner, group or the permissions, this command
will write an error message to the console.

Relates elastic/elasticsearch#517

Original commit: elastic/x-pack-elasticsearch@49aab5f712
This commit is contained in:
Alexander Reelsen 2015-01-22 16:59:34 +01:00
parent c5028f7384
commit 2986502984
6 changed files with 409 additions and 13 deletions

View File

@ -152,6 +152,12 @@
<version>2.3.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>com.google.jimfs</groupId>
<artifactId>jimfs</artifactId>
<version>1.0</version>
<scope>test</scope>
</dependency>
<!-- real dependencies -->
<dependency>

View File

@ -0,0 +1,96 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.common.cli;
import org.elasticsearch.common.collect.Maps;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.attribute.PosixFileAttributes;
import java.nio.file.attribute.PosixFilePermission;
import java.util.Map;
import java.util.Set;
/**
* helper command to check if file permissions or owner got changed by the command being executed
*/
public abstract class CheckFileCommand extends CliTool.Command {
public CheckFileCommand(Terminal terminal) {
super(terminal);
}
/**
* abstract method, which should implement the same logic as CliTool.Command.execute(), but is wrapped
*/
public abstract CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception;
/**
* Returns the array of paths, that should be checked if the permissions, user or groups have changed
* before and after execution of the command
*
*/
protected abstract Path[] pathsForPermissionsCheck(Settings settings, Environment env) throws Exception;
@Override
public CliTool.ExitStatus execute(Settings settings, Environment env) throws Exception {
Path[] paths = pathsForPermissionsCheck(settings, env);
Map<Path, Set<PosixFilePermission>> permissions = Maps.newHashMapWithExpectedSize(paths.length);
Map<Path, String> owners = Maps.newHashMapWithExpectedSize(paths.length);
Map<Path, String> groups = Maps.newHashMapWithExpectedSize(paths.length);
if (paths != null && paths.length > 0) {
for (Path path : paths) {
try {
boolean supportsPosixPermissions = Files.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class);
if (supportsPosixPermissions) {
permissions.put(path, Files.getPosixFilePermissions(path));
owners.put(path, Files.getOwner(path).getName());
groups.put(path, Files.readAttributes(path, PosixFileAttributes.class).group().getName());
}
} catch (IOException e) {
// silently swallow if not supported, no need to log things
}
}
}
CliTool.ExitStatus status = doExecute(settings, env);
// check if permissions differ
for (Map.Entry<Path, Set<PosixFilePermission>> entry : permissions.entrySet()) {
Set<PosixFilePermission> permissionsBeforeWrite = entry.getValue();
Set<PosixFilePermission> permissionsAfterWrite = Files.getPosixFilePermissions(entry.getKey());
if (!permissionsBeforeWrite.equals(permissionsAfterWrite)) {
terminal.printError("Permissions of [%s] differ after write. Please ensure correct permissions before going on!", entry.getKey());
}
}
// check if owner differs
for (Map.Entry<Path, String> entry : owners.entrySet()) {
String ownerBeforeWrite = entry.getValue();
String ownerAfterWrite = Files.getOwner(entry.getKey()).getName();
if (!ownerAfterWrite.equals(ownerBeforeWrite)) {
terminal.printError("Owner of file [%s] used to be [%s], but now is [%s]", entry.getKey(), ownerBeforeWrite, ownerAfterWrite);
}
}
// check if group differs
for (Map.Entry<Path, String> entry : groups.entrySet()) {
String groupBeforeWrite = entry.getValue();
String groupAfterWrite = Files.readAttributes(entry.getKey(), PosixFileAttributes.class).group().getName();
if (!groupAfterWrite.equals(groupBeforeWrite)) {
terminal.printError("Group of file [%s] used to be [%s], but now is [%s]", entry.getKey(), groupBeforeWrite, groupAfterWrite);
}
}
return status;
}
}

View File

@ -7,6 +7,7 @@ package org.elasticsearch.shield.authc.esusers.tool;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.base.Joiner;
import org.elasticsearch.common.cli.CheckFileCommand;
import org.elasticsearch.common.cli.CliTool;
import org.elasticsearch.common.cli.CliToolConfig;
import org.elasticsearch.common.cli.Terminal;
@ -73,7 +74,7 @@ public class ESUsersTool extends CliTool {
}
}
static class Useradd extends CliTool.Command {
static class Useradd extends CheckFileCommand {
private static final String NAME = "useradd";
@ -137,7 +138,7 @@ public class ESUsersTool extends CliTool {
}
@Override
public ExitStatus execute(Settings settings, Environment env) throws Exception {
public ExitStatus doExecute(Settings settings, Environment env) throws Exception {
Settings esusersSettings = Realms.internalRealmSettings(settings, ESUsersRealm.TYPE);
verifyRoles(terminal, settings, env, roles);
Path file = FileUserPasswdStore.resolveFile(esusersSettings, env);
@ -158,9 +159,17 @@ public class ESUsersTool extends CliTool {
}
return ExitStatus.OK;
}
@Override
protected Path[] pathsForPermissionsCheck(Settings settings, Environment env) {
Settings esusersSettings = Realms.internalRealmSettings(settings, ESUsersRealm.TYPE);
Path userPath = FileUserPasswdStore.resolveFile(esusersSettings, env);
Path userRolesPath = FileUserRolesStore.resolveFile(esusersSettings, env);
return new Path[] {userPath, userRolesPath};
}
}
static class Userdel extends CliTool.Command {
static class Userdel extends CheckFileCommand {
private static final String NAME = "userdel";
@ -183,7 +192,19 @@ public class ESUsersTool extends CliTool {
}
@Override
public ExitStatus execute(Settings settings, Environment env) throws Exception {
protected Path[] pathsForPermissionsCheck(Settings settings, Environment env) {
Settings esusersSettings = Realms.internalRealmSettings(settings, ESUsersRealm.TYPE);
Path userPath = FileUserPasswdStore.resolveFile(esusersSettings, env);
Path userRolesPath = FileUserRolesStore.resolveFile(esusersSettings, env);
if (Files.exists(userRolesPath)) {
return new Path[] { userPath, userRolesPath };
}
return new Path[] { userPath };
}
@Override
public ExitStatus doExecute(Settings settings, Environment env) throws Exception {
Settings esusersSettings = Realms.internalRealmSettings(settings, ESUsersRealm.TYPE);
Path file = FileUserPasswdStore.resolveFile(esusersSettings, env);
Map<String, char[]> users = new HashMap<>(FileUserPasswdStore.parseFile(file, null));
@ -212,7 +233,7 @@ public class ESUsersTool extends CliTool {
}
}
static class Passwd extends CliTool.Command {
static class Passwd extends CheckFileCommand {
private static final String NAME = "passwd";
@ -251,8 +272,16 @@ public class ESUsersTool extends CliTool {
Arrays.fill(passwd, (char) 0);
}
@Override
public ExitStatus execute(Settings settings, Environment env) throws Exception {
protected Path[] pathsForPermissionsCheck(Settings settings, Environment env) {
Settings esusersSettings = Realms.internalRealmSettings(settings, ESUsersRealm.TYPE);
Path path = FileUserPasswdStore.resolveFile(esusersSettings, env);
return new Path[] { path };
}
@Override
public ExitStatus doExecute(Settings settings, Environment env) throws Exception {
Settings esusersSettings = Realms.internalRealmSettings(settings, ESUsersRealm.TYPE);
Path file = FileUserPasswdStore.resolveFile(esusersSettings, env);
Map<String, char[]> users = new HashMap<>(FileUserPasswdStore.parseFile(file, null));
@ -268,7 +297,7 @@ public class ESUsersTool extends CliTool {
}
static class Roles extends CliTool.Command {
static class Roles extends CheckFileCommand {
private static final String NAME = "roles";
@ -305,7 +334,14 @@ public class ESUsersTool extends CliTool {
}
@Override
public ExitStatus execute(Settings settings, Environment env) throws Exception {
protected Path[] pathsForPermissionsCheck(Settings settings, Environment env) {
Settings esusersSettings = Realms.internalRealmSettings(settings, ESUsersRealm.TYPE);
Path path = FileUserPasswdStore.resolveFile(esusersSettings, env);
return new Path[] { path } ;
}
@Override
public ExitStatus doExecute(Settings settings, Environment env) throws Exception {
// check if just need to return data as no write operation happens
// Nothing to add, just list the data for a username
boolean readOnlyUserListing = removeRoles.length == 0 && addRoles.length == 0;
@ -477,4 +513,4 @@ public class ESUsersTool extends CliTool {
Strings.collectionToCommaDelimitedString(unknownRoles), rolesFile.toAbsolutePath());
}
}
}
}

View File

@ -5,6 +5,7 @@
*/
package org.elasticsearch.shield.signature.tool;
import org.elasticsearch.common.cli.CheckFileCommand;
import org.elasticsearch.common.cli.CliTool;
import org.elasticsearch.common.cli.CliToolConfig;
import org.elasticsearch.common.cli.Terminal;
@ -53,7 +54,7 @@ public class SystemKeyTool extends CliTool {
return Generate.parse(terminal, commandLine);
}
static class Generate extends Command {
static class Generate extends CheckFileCommand {
private static final CliToolConfig.Cmd CMD = cmd("generate", Generate.class).build();
@ -74,7 +75,16 @@ public class SystemKeyTool extends CliTool {
}
@Override
public ExitStatus execute(Settings settings, Environment env) throws Exception {
protected Path[] pathsForPermissionsCheck(Settings settings, Environment env) {
Path path = this.path;
if (path == null) {
path = InternalSignatureService.resolveFile(settings, env);
}
return new Path[] { path };
}
@Override
public ExitStatus doExecute(Settings settings, Environment env) throws Exception {
Path path = this.path;
try {
if (path == null) {

View File

@ -0,0 +1,187 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.common.cli;
import com.google.common.collect.Sets;
import com.google.common.jimfs.Configuration;
import com.google.common.jimfs.Jimfs;
import org.elasticsearch.common.base.Charsets;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
import org.elasticsearch.test.ElasticsearchTestCase;
import org.junit.Test;
import java.io.IOException;
import java.nio.file.FileSystem;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.attribute.GroupPrincipal;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.attribute.PosixFilePermission;
import java.nio.file.attribute.UserPrincipal;
import static org.hamcrest.Matchers.*;
/**
*
*/
public class CheckFileCommandTests extends ElasticsearchTestCase {
private CliToolTestCase.CaptureOutputTerminal captureOutputTerminal = new CliToolTestCase.CaptureOutputTerminal();
private Configuration jimFsConfiguration = Configuration.unix().toBuilder().setAttributeViews("basic", "owner", "posix", "unix").build();
private Configuration jimFsConfigurationWithoutPermissions = randomBoolean() ? Configuration.unix().toBuilder().setAttributeViews("basic").build() : Configuration.windows();
@Test
public void testThatCommandLogsErrorMessageOnFail() throws Exception {
executeCommand(jimFsConfiguration, new PermissionCheckFileCommand(captureOutputTerminal, true));
assertThat(captureOutputTerminal.getTerminalOutput(), hasItem(containsString("Please ensure correct permissions")));
}
@Test
public void testThatCommandLogsNothingOnSuccess() throws Exception {
executeCommand(jimFsConfiguration, new PermissionCheckFileCommand(captureOutputTerminal, false));
assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0));
}
@Test
public void testThatCommandLogsNothingIfFilesystemDoesNotSupportPermissions() throws Exception {
executeCommand(jimFsConfigurationWithoutPermissions, new PermissionCheckFileCommand(captureOutputTerminal, false));
assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0));
}
@Test
public void testThatCommandLogsOwnerChange() throws Exception {
executeCommand(jimFsConfiguration, new OwnerCheckFileCommand(captureOutputTerminal, true));
assertThat(captureOutputTerminal.getTerminalOutput(), hasItem(allOf(containsString("Owner of file ["), containsString("] used to be ["), containsString("], but now is ["))));
}
@Test
public void testThatCommandLogsNothingIfOwnerIsNotChanged() throws Exception {
executeCommand(jimFsConfiguration, new OwnerCheckFileCommand(captureOutputTerminal, false));
assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0));
}
@Test
public void testThatCommandLogsNothingIfFileSystemDoesNotSupportOwners() throws Exception {
executeCommand(jimFsConfigurationWithoutPermissions, new OwnerCheckFileCommand(captureOutputTerminal, false));
assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0));
}
@Test
public void testThatCommandLogsIfGroupChanges() throws Exception {
executeCommand(jimFsConfiguration, new GroupCheckFileCommand(captureOutputTerminal, true));
assertThat(captureOutputTerminal.getTerminalOutput(), hasItem(allOf(containsString("Group of file ["), containsString("] used to be ["), containsString("], but now is ["))));
}
@Test
public void testThatCommandLogsNothingIfGroupIsNotChanged() throws Exception {
executeCommand(jimFsConfiguration, new GroupCheckFileCommand(captureOutputTerminal, false));
assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0));
}
@Test
public void testThatCommandLogsNothingIfFileSystemDoesNotSupportGroups() throws Exception {
executeCommand(jimFsConfigurationWithoutPermissions, new GroupCheckFileCommand(captureOutputTerminal, false));
assertThat(captureOutputTerminal.getTerminalOutput(), hasSize(0));
}
private void executeCommand(Configuration configuration, AbstractTestCheckFileCommand command) throws Exception {
try (FileSystem fs = Jimfs.newFileSystem(configuration)) {
command.execute(fs);
}
}
abstract class AbstractTestCheckFileCommand extends CheckFileCommand {
protected final boolean enabled;
protected FileSystem fs;
protected Path[] paths;
public AbstractTestCheckFileCommand(Terminal terminal, boolean enabled) throws IOException {
super(terminal);
this.enabled = enabled;
}
public CliTool.ExitStatus execute(FileSystem fs) throws Exception {
this.fs = fs;
this.paths = new Path[] { writePath(fs, "p1", "anything"), writePath(fs, "p2", "anything"), writePath(fs, "p3", "anything") };
return super.execute(ImmutableSettings.EMPTY, new Environment(ImmutableSettings.EMPTY));
}
private Path writePath(FileSystem fs, String name, String content) throws IOException {
Path path = fs.getPath(name);
Files.write(path, content.getBytes(Charsets.UTF_8));
return path;
}
@Override
protected Path[] pathsForPermissionsCheck(Settings settings, Environment env) {
return paths;
}
}
/**
* command that changes permissions from a file if enabled
*/
class PermissionCheckFileCommand extends AbstractTestCheckFileCommand {
public PermissionCheckFileCommand(Terminal terminal, boolean enabled) throws IOException {
super(terminal, enabled);
}
@Override
public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception {
if (enabled) {
int randomInt = randomInt(paths.length - 1);
Files.setPosixFilePermissions(paths[randomInt], Sets.newHashSet(PosixFilePermission.OWNER_EXECUTE, PosixFilePermission.OTHERS_EXECUTE, PosixFilePermission.GROUP_EXECUTE));
}
return CliTool.ExitStatus.OK;
}
}
/**
* command that changes the owner of a file if enabled
*/
class OwnerCheckFileCommand extends AbstractTestCheckFileCommand {
public OwnerCheckFileCommand(Terminal terminal, boolean enabled) throws IOException {
super(terminal, enabled);
}
@Override
public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception {
if (enabled) {
int randomInt = randomInt(paths.length - 1);
UserPrincipal owner = fs.getUserPrincipalLookupService().lookupPrincipalByName(randomAsciiOfLength(10));
Files.setOwner(paths[randomInt], owner);
}
return CliTool.ExitStatus.OK;
}
}
/**
* command that changes the group of a file if enabled
*/
class GroupCheckFileCommand extends AbstractTestCheckFileCommand {
public GroupCheckFileCommand(Terminal terminal, boolean enabled) throws IOException {
super(terminal, enabled);
}
@Override
public CliTool.ExitStatus doExecute(Settings settings, Environment env) throws Exception {
if (enabled) {
int randomInt = randomInt(paths.length - 1);
GroupPrincipal groupPrincipal = fs.getUserPrincipalLookupService().lookupPrincipalByGroupName(randomAsciiOfLength(10));
Files.getFileAttributeView(paths[randomInt], PosixFileAttributeView.class).setGroup(groupPrincipal);
}
return CliTool.ExitStatus.OK;
}
}
}

View File

@ -11,6 +11,7 @@ import org.elasticsearch.common.base.Charsets;
import org.elasticsearch.common.cli.CliTool;
import org.elasticsearch.common.cli.CliToolTestCase;
import org.elasticsearch.common.cli.Terminal;
import org.elasticsearch.common.collect.Sets;
import org.elasticsearch.common.settings.ImmutableSettings;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.env.Environment;
@ -21,6 +22,9 @@ import org.junit.Test;
import java.io.File;
import java.io.IOException;
import java.nio.file.Path;
import java.nio.file.attribute.PosixFileAttributeView;
import java.nio.file.attribute.PosixFilePermission;
import java.util.List;
import java.util.Locale;
import java.util.Map;
@ -202,6 +206,31 @@ public class ESUsersToolTests extends CliToolTestCase {
assertThat(status, is(CliTool.ExitStatus.CODE_ERROR));
}
@Test
public void testUseradd_Cmd_LogsPermissionChange() throws Exception {
File userFile = writeFile("user1:hash1");
File userRolesFile = newTempFile();
assumePosixPermissionsAreSupported(userFile.toPath(), userRolesFile.toPath());
java.nio.file.Files.setPosixFilePermissions(userFile.toPath(), Sets.newHashSet(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.GROUP_WRITE, PosixFilePermission.OTHERS_WRITE));
java.nio.file.Files.setPosixFilePermissions(userRolesFile.toPath(), Sets.newHashSet(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.GROUP_WRITE, PosixFilePermission.OTHERS_WRITE));
Settings settings = ImmutableSettings.builder()
.put("shield.authc.realms.esusers.type", "esusers")
.put("shield.authc.realms.esusers.files.users", userFile)
.put("shield.authc.realms.esusers.files.users_roles", userRolesFile)
.build();
CaptureOutputTerminal captureOutputTerminal = new CaptureOutputTerminal();
ESUsersTool.Useradd cmd = new ESUsersTool.Useradd(captureOutputTerminal, "user2", SecuredStringTests.build("changeme"));
CliTool.ExitStatus status = execute(cmd, settings);
assertThat(status, is(CliTool.ExitStatus.OK));
assertThat(captureOutputTerminal.getTerminalOutput(), hasItem(containsString("Please ensure correct permissions")));
}
@Test
public void testUserdel_Parse() throws Exception {
ESUsersTool tool = new ESUsersTool();
@ -235,7 +264,7 @@ public class ESUsersToolTests extends CliToolTestCase {
CliTool.ExitStatus status = execute(cmd, settings);
assertThat(status, is(CliTool.ExitStatus.OK));
assertFileExists(userFile);
assertFileExists(userFile);
List<String> lines = Files.readLines(userFile, Charsets.UTF_8);
assertThat(lines.size(), is(0));
@ -264,7 +293,7 @@ public class ESUsersToolTests extends CliToolTestCase {
assertThat(output, hasSize(equalTo(1)));
assertThat(output, hasItem(startsWith("User [user2] doesn't exist")));
assertFileExists(userFile);
assertFileExists(userFile);
List<String> lines = Files.readLines(userFile, Charsets.UTF_8);
assertThat(lines.size(), is(1));
@ -293,6 +322,31 @@ public class ESUsersToolTests extends CliToolTestCase {
assertThat(userRolesFile.exists(), is(false));
}
@Test
public void testUserdel_Cmd_LogsPermissionChange() throws Exception {
File userFile = writeFile("user1:hash1");
File userRolesFile = newTempFile();
assumePosixPermissionsAreSupported(userFile.toPath(), userRolesFile.toPath());
java.nio.file.Files.setPosixFilePermissions(userFile.toPath(), Sets.newHashSet(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.GROUP_WRITE, PosixFilePermission.OTHERS_WRITE));
java.nio.file.Files.setPosixFilePermissions(userRolesFile.toPath(), Sets.newHashSet(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_WRITE, PosixFilePermission.GROUP_WRITE, PosixFilePermission.OTHERS_WRITE));
Settings settings = ImmutableSettings.builder()
.put("shield.authc.realms.esusers.type", "esusers")
.put("shield.authc.realms.esusers.files.users", userFile)
.put("shield.authc.realms.esusers.files.users_roles", userRolesFile)
.build();
CaptureOutputTerminal captureOutputTerminal = new CaptureOutputTerminal();
ESUsersTool.Userdel cmd = new ESUsersTool.Userdel(captureOutputTerminal, "user1");
CliTool.ExitStatus status = execute(cmd, settings);
assertThat(status, is(CliTool.ExitStatus.OK));
assertThat(captureOutputTerminal.getTerminalOutput(), hasItem(containsString("Please ensure correct permissions")));
}
@Test
public void testPasswd_Parse_AllOptions() throws Exception {
ESUsersTool tool = new ESUsersTool();
@ -745,4 +799,11 @@ public class ESUsersToolTests extends CliToolTestCase {
private void assertFileExists(File file) {
assertThat(String.format(Locale.ROOT, "Expected file [%s] to exist", file), file.exists(), is(true));
}
private void assumePosixPermissionsAreSupported(Path ... paths) throws IOException {
for (Path path : paths) {
boolean supportsPosixPermissionsForUserFile = java.nio.file.Files.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class);
assumeTrue("Ignoring because posix file attributes are not supported for file " + path, supportsPosixPermissionsForUserFile);
}
}
}