[CLI] attempt to restore owner and group on new files
For the CLI tools in Shield we create a new temp file and replace the existing file to prevent issues with reloading a half written file. This has a potential side effect of changing the user and group that own the file. Many times the commands are run with root privileges (sudo) and when run with root privileges we can actually reset the owner and group correctly. Closes elastic/elasticsearch#812 Original commit: elastic/x-pack-elasticsearch@1ee3715376
This commit is contained in:
parent
197817e900
commit
39f587a497
6
pom.xml
6
pom.xml
|
@ -158,6 +158,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>
|
||||
|
|
|
@ -11,6 +11,7 @@ import java.io.IOException;
|
|||
import java.io.Writer;
|
||||
import java.nio.file.*;
|
||||
import java.nio.file.attribute.PosixFileAttributeView;
|
||||
import java.nio.file.attribute.PosixFileAttributes;
|
||||
import java.nio.file.attribute.PosixFilePermission;
|
||||
import java.util.Set;
|
||||
|
||||
|
@ -48,24 +49,36 @@ public class ShieldFiles {
|
|||
public void close() throws IOException {
|
||||
writer.close();
|
||||
// get original permissions
|
||||
boolean supportsPosixPermissions = false;
|
||||
Set<PosixFilePermission> posixFilePermissions = null;
|
||||
if (Files.exists(path)) {
|
||||
supportsPosixPermissions = Files.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class);
|
||||
if (supportsPosixPermissions) {
|
||||
posixFilePermissions = Files.getPosixFilePermissions(path);
|
||||
boolean supportsPosixAttributes = Files.getFileStore(path).supportsFileAttributeView(PosixFileAttributeView.class);
|
||||
if (supportsPosixAttributes) {
|
||||
setPosixAttributesOnTempFile(path, tempFile);
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
Files.move(tempFile, path, StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.ATOMIC_MOVE);
|
||||
} catch (AtomicMoveNotSupportedException e) {
|
||||
Files.move(tempFile, path, StandardCopyOption.REPLACE_EXISTING);
|
||||
}
|
||||
// restore original permissions
|
||||
if (supportsPosixPermissions && posixFilePermissions != null) {
|
||||
Files.setPosixFilePermissions(path, posixFilePermissions);
|
||||
}
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
static void setPosixAttributesOnTempFile(Path path, Path tempFile) throws IOException {
|
||||
PosixFileAttributes attributes = Files.getFileAttributeView(path, PosixFileAttributeView.class).readAttributes();
|
||||
PosixFileAttributeView tempFileView = Files.getFileAttributeView(tempFile, PosixFileAttributeView.class);
|
||||
|
||||
tempFileView.setPermissions(attributes.permissions());
|
||||
|
||||
// Make an attempt to set the username and group to match. If it fails, silently ignore the failure as the user
|
||||
// will be notified by the CheckFileCommand that the ownership has changed and needs to be corrected
|
||||
try {
|
||||
tempFileView.setOwner(attributes.owner());
|
||||
} catch (Exception e) {}
|
||||
|
||||
try {
|
||||
tempFileView.setGroup(attributes.group());
|
||||
} catch (Exception e) {}
|
||||
}
|
||||
}
|
||||
|
|
|
@ -6,11 +6,14 @@
|
|||
package org.elasticsearch.shield.support;
|
||||
|
||||
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.test.ElasticsearchTestCase;
|
||||
import org.junit.Test;
|
||||
|
||||
import java.io.PrintWriter;
|
||||
import java.nio.file.FileSystem;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Path;
|
||||
import java.nio.file.attribute.PosixFileAttributeView;
|
||||
|
@ -20,7 +23,9 @@ import java.util.Set;
|
|||
|
||||
import static java.nio.file.attribute.PosixFilePermission.*;
|
||||
import static org.elasticsearch.shield.support.ShieldFiles.openAtomicMoveWriter;
|
||||
import static org.hamcrest.Matchers.equalTo;
|
||||
import static org.hamcrest.Matchers.is;
|
||||
import static org.hamcrest.Matchers.not;
|
||||
|
||||
public class ShieldFilesTests extends ElasticsearchTestCase {
|
||||
|
||||
|
@ -49,4 +54,27 @@ public class ShieldFilesTests extends ElasticsearchTestCase {
|
|||
assertThat(permissionsAfterWrite, is(perms));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testThatOwnerAndGroupAreChanged() throws Exception {
|
||||
Configuration jimFsConfiguration = Configuration.unix().toBuilder().setAttributeViews("basic", "owner", "posix", "unix").build();
|
||||
try (FileSystem fs = Jimfs.newFileSystem(jimFsConfiguration)) {
|
||||
Path path = fs.getPath("foo");
|
||||
Path tempPath = fs.getPath("bar");
|
||||
Files.write(path, "foo".getBytes(Charsets.UTF_8));
|
||||
Files.write(tempPath, "bar".getBytes(Charsets.UTF_8));
|
||||
|
||||
PosixFileAttributeView view = Files.getFileAttributeView(path, PosixFileAttributeView.class);
|
||||
view.setGroup(fs.getUserPrincipalLookupService().lookupPrincipalByGroupName(randomAsciiOfLength(10)));
|
||||
view.setOwner(fs.getUserPrincipalLookupService().lookupPrincipalByName(randomAsciiOfLength(10)));
|
||||
|
||||
PosixFileAttributeView tempPathView = Files.getFileAttributeView(tempPath, PosixFileAttributeView.class);
|
||||
assertThat(tempPathView.getOwner(), not(equalTo(view.getOwner())));
|
||||
assertThat(tempPathView.readAttributes().group(), not(equalTo(view.readAttributes().group())));
|
||||
|
||||
ShieldFiles.setPosixAttributesOnTempFile(path, tempPath);
|
||||
|
||||
assertThat(tempPathView.getOwner(), equalTo(view.getOwner()));
|
||||
assertThat(tempPathView.readAttributes().group(), equalTo(view.readAttributes().group()));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue