From 97c58bac8ecc52b4fb1a0b3af82f557f8200878e Mon Sep 17 00:00:00 2001 From: Håkon Hallingstad Date: Fri, 9 Nov 2018 17:59:25 +0100 Subject: Make tenant keys and certificates readable by 1000 only --- .../identity/AthenzCredentialsMaintainer.java | 32 +++++++++++++-------- .../node/admin/nodeagent/NodeAgentContext.java | 2 ++ .../node/admin/nodeagent/NodeAgentContextImpl.java | 19 ++++++++++--- .../hosted/node/admin/task/util/file/UnixPath.java | 33 ++++++++++++++++++++-- .../admin/docker/DockerOperationsImplTest.java | 4 +-- .../node/admin/integrationTests/DockerTester.java | 2 +- .../admin/maintenance/StorageMaintainerTest.java | 6 ++-- .../maintenance/coredump/CoreCollectorTest.java | 2 +- .../maintenance/coredump/CoredumpHandlerTest.java | 2 +- .../admin/nodeagent/NodeAgentContextImplTest.java | 2 +- .../node/admin/nodeagent/NodeAgentImplTest.java | 2 +- 11 files changed, 78 insertions(+), 28 deletions(-) (limited to 'node-admin') diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java index 22093258930..92d3ad14aa7 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java @@ -25,6 +25,7 @@ import com.yahoo.vespa.hosted.dockerapi.ContainerName; import com.yahoo.vespa.hosted.node.admin.component.ConfigServerInfo; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder; +import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; import javax.net.ssl.SSLContext; import java.io.IOException; @@ -166,10 +167,9 @@ public class AthenzCredentialsMaintainer { false, csr); EntityBindingsMapper.writeSignedIdentityDocumentToFile(identityDocumentFile, signedIdentityDocument); - writePrivateKeyAndCertificate(privateKeyFile, keyPair.getPrivate(), certificateFile, instanceIdentity.certificate()); + writePrivateKeyAndCertificate(context.vespaUserIdOnHost(), privateKeyFile, keyPair.getPrivate(), + certificateFile, instanceIdentity.certificate()); context.log(logger, "Instance successfully registered and credentials written to file"); - } catch (IOException e) { - throw new UncheckedIOException(e); } } @@ -192,7 +192,8 @@ public class AthenzCredentialsMaintainer { identityDocument.providerUniqueId().asDottedString(), false, csr); - writePrivateKeyAndCertificate(privateKeyFile, keyPair.getPrivate(), certificateFile, instanceIdentity.certificate()); + writePrivateKeyAndCertificate(context.vespaUserIdOnHost(), privateKeyFile, keyPair.getPrivate(), + certificateFile, instanceIdentity.certificate()); context.log(logger, "Instance successfully refreshed and credentials written to file"); } catch (ZtsClientException e) { if (e.getErrorCode() == 403 && e.getDescription().startsWith("Certificate revoked")) { @@ -208,15 +209,22 @@ public class AthenzCredentialsMaintainer { } - private static void writePrivateKeyAndCertificate( - Path privateKeyFile, PrivateKey privateKey, Path certificateFile, X509Certificate certificate) throws IOException { - Path tempPrivateKeyFile = toTempPath(privateKeyFile); - Files.write(tempPrivateKeyFile, KeyUtils.toPem(privateKey).getBytes()); - Path tempCertificateFile = toTempPath(certificateFile); - Files.write(tempCertificateFile, X509CertificateUtils.toPem(certificate).getBytes()); + private static void writePrivateKeyAndCertificate(int vespaUserIdOnHost, + Path privateKeyFile, + PrivateKey privateKey, + Path certificateFile, + X509Certificate certificate) { + writeFile(privateKeyFile, vespaUserIdOnHost, KeyUtils.toPem(privateKey)); + writeFile(certificateFile, vespaUserIdOnHost, X509CertificateUtils.toPem(certificate)); + } - Files.move(tempPrivateKeyFile, privateKeyFile, StandardCopyOption.ATOMIC_MOVE); - Files.move(tempCertificateFile, certificateFile, StandardCopyOption.ATOMIC_MOVE); + private static void writeFile(Path path, int vespaUserIdOnHost, String utf8Content) { + new UnixPath(path.toString() + ".tmp") + .createNewFile("---------") + .setOwnerId(vespaUserIdOnHost) + .setPermissions("r-----------") + .writeUtf8File(utf8Content) + .atomicMove(path); } private static Path toTempPath(Path file) { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java index 58436edbf20..1f3e02433cb 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java @@ -27,6 +27,8 @@ public interface NodeAgentContext extends TaskContext { String vespaUser(); + int vespaUserIdOnHost(); + /** * This method is the inverse of {@link #pathInNodeFromPathOnHost(Path)}} * diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java index 69947b472f2..04edb033d75 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java @@ -34,10 +34,12 @@ public class NodeAgentContextImpl implements NodeAgentContext { private final Path pathToNodeRootOnHost; private final Path pathToVespaHome; private final String vespaUser; + private final int vespaUserIdOnHost; public NodeAgentContextImpl(String hostname, NodeType nodeType, AthenzService identity, DockerNetworking dockerNetworking, ZoneId zoneId, - Path pathToContainerStorage, Path pathToVespaHome, String vespaUser) { + Path pathToContainerStorage, Path pathToVespaHome, + String vespaUser, int vespaUserIdOnHost) { this.hostName = HostName.from(Objects.requireNonNull(hostname)); this.containerName = ContainerName.fromHostname(hostname); this.nodeType = Objects.requireNonNull(nodeType); @@ -48,6 +50,7 @@ public class NodeAgentContextImpl implements NodeAgentContext { this.pathToVespaHome = Objects.requireNonNull(pathToVespaHome); this.logPrefix = containerName.asString() + ": "; this.vespaUser = vespaUser; + this.vespaUserIdOnHost = vespaUserIdOnHost; } @Override @@ -85,6 +88,11 @@ public class NodeAgentContextImpl implements NodeAgentContext { return vespaUser; } + @Override + public int vespaUserIdOnHost() { + return vespaUserIdOnHost; + } + @Override public Path pathOnHostFromPathInNode(Path pathInNode) { if (! pathInNode.isAbsolute()) @@ -138,9 +146,11 @@ public class NodeAgentContextImpl implements NodeAgentContext { private Path pathToContainerStorage; private Path pathToVespaHome; private String vespaUser; - - public Builder(String hostname) { + private Integer vespaUserIdOnHost; + + public Builder(String hostname, int vespaUserIdOnHost) { this.hostname = hostname; + this.vespaUserIdOnHost = vespaUserIdOnHost; } public Builder nodeType(NodeType nodeType) { @@ -191,7 +201,8 @@ public class NodeAgentContextImpl implements NodeAgentContext { Optional.ofNullable(zoneId).orElseGet(() -> new ZoneId(SystemName.dev, Environment.dev, RegionName.defaultName())), Optional.ofNullable(pathToContainerStorage).orElseGet(() -> Paths.get("/home/docker")), Optional.ofNullable(pathToVespaHome).orElseGet(() -> Paths.get("/opt/vespa")), - Optional.ofNullable(vespaUser).orElseGet(() -> "vespa")); + Optional.ofNullable(vespaUser).orElse("vespa"), + vespaUserIdOnHost); } } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java index ae5c6f57d4c..064354ddc9c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java @@ -9,6 +9,7 @@ import java.nio.file.NoSuchFileException; import java.nio.file.OpenOption; import java.nio.file.Path; import java.nio.file.Paths; +import java.nio.file.StandardCopyOption; import java.nio.file.attribute.FileAttribute; import java.nio.file.attribute.GroupPrincipal; import java.nio.file.attribute.PosixFileAttributeView; @@ -66,8 +67,9 @@ public class UnixPath { return uncheck(() -> Files.readAllBytes(path)); } - public void writeUtf8File(String content, OpenOption... options) { + public UnixPath writeUtf8File(String content, OpenOption... options) { writeBytes(content.getBytes(StandardCharsets.UTF_8), options); + return this; } public void writeBytes(byte[] content, OpenOption... options) { @@ -82,9 +84,19 @@ public class UnixPath { * @param permissions Example: "rwxr-x---" means rwx for owner, rx for group, * and no permissions for others. */ - public void setPermissions(String permissions) { + public UnixPath setPermissions(String permissions) { Set permissionSet = getPosixFilePermissionsFromString(permissions); uncheck(() -> Files.setPosixFilePermissions(path, permissionSet)); + return this; + } + + public int getOwnerId() { + return (Integer) uncheck(() -> Files.getAttribute(path, "unix:uid")); + } + + public UnixPath setOwnerId(int ownerId) { + uncheck(() -> Files.setAttribute(path, "unix:uid", ownerId)); + return this; } public String getOwner() { @@ -125,6 +137,17 @@ public class UnixPath { return IOExceptionUtil.ifExists(this::getAttributes); } + public UnixPath createNewFile() { + uncheck(() -> Files.createFile(path)); + return this; + } + + public UnixPath createNewFile(String permissions) { + FileAttribute attribute = PosixFilePermissions.asFileAttribute(PosixFilePermissions.fromString(permissions)); + uncheck(() -> Files.createFile(path, attribute)); + return this; + } + public void createDirectory(String permissions) { Set set = getPosixFilePermissionsFromString(permissions); FileAttribute> attribute = PosixFilePermissions.asFileAttribute(set); @@ -171,6 +194,12 @@ public class UnixPath { } } + /** This path must be on the same file system as the to-path. */ + public UnixPath atomicMove(Path to) { + uncheck(() -> Files.move(path, to, StandardCopyOption.ATOMIC_MOVE)); + return this; + } + public boolean moveIfExists(Path to) { try { Files.move(path, to); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java index 6e8cfce6c37..fddfd2d875e 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java @@ -46,7 +46,7 @@ public class DockerOperationsImplTest { @Test public void processResultFromNodeProgramWhenSuccess() { - final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld").build(); + final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld", 1000).build(); final ProcessResult actualResult = new ProcessResult(0, "output", "errors"); when(docker.executeInContainerAsUser(any(), any(), any(), any())) @@ -67,7 +67,7 @@ public class DockerOperationsImplTest { @Test(expected = RuntimeException.class) public void processResultFromNodeProgramWhenNonZeroExitCode() { - final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld").build(); + final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld", 1000).build(); final ProcessResult actualResult = new ProcessResult(3, "output", "errors"); when(docker.executeInContainerAsUser(any(), any(), any(), any())) diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java index e22606104f1..d2cc0c8fd7e 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java @@ -94,7 +94,7 @@ public class DockerTester implements AutoCloseable { MetricReceiverWrapper mr = new MetricReceiverWrapper(MetricReceiver.nullImplementation); Function nodeAgentFactory = (hostName) -> new NodeAgentImpl( - new NodeAgentContextImpl.Builder(hostName).fileSystem(fileSystem).build(), nodeRepository, + new NodeAgentContextImpl.Builder(hostName, 1000).fileSystem(fileSystem).build(), nodeRepository, orchestrator, dockerOperations, storageMaintainer, clock, INTERVAL, Optional.empty(), Optional.empty(), Optional.empty()); nodeAdmin = new NodeAdminImpl(nodeAgentFactory, Optional.empty(), mr, Clock.systemUTC()); nodeAdminStateUpdater = new NodeAdminStateUpdater(nodeRepository, orchestrator, diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java index cf5d29d70f1..9e961ff03b4 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java @@ -152,7 +152,7 @@ public class StorageMaintainerTest { } private Path executeAs(NodeType nodeType) { - NodeAgentContext context = new NodeAgentContextImpl.Builder("host123-5.test.domain.tld") + NodeAgentContext context = new NodeAgentContextImpl.Builder("host123-5.test.domain.tld", 1000) .nodeType(nodeType) .fileSystem(TestFileSystem.create()) .zoneId(new ZoneId(SystemName.dev, Environment.prod, RegionName.from("us-north-1"))).build(); @@ -197,7 +197,7 @@ public class StorageMaintainerTest { public void testDiskUsed() throws IOException { StorageMaintainer storageMaintainer = new StorageMaintainer(terminal, docker, null, null); FileSystem fileSystem = TestFileSystem.create(); - NodeAgentContext context = new NodeAgentContextImpl.Builder("host-1.domain.tld").fileSystem(fileSystem).build(); + NodeAgentContext context = new NodeAgentContextImpl.Builder("host-1.domain.tld", 1000).fileSystem(fileSystem).build(); Files.createDirectories(context.pathOnHostFromPathInNode("/")); terminal.expectCommand("du -xsk /home/docker/host-1 2>&1", 0, "321\t/home/docker/host-1/"); @@ -265,7 +265,7 @@ public class StorageMaintainerTest { } private NodeAgentContext createNodeAgentContextAndContainerStorage(FileSystem fileSystem, String containerName) throws IOException { - NodeAgentContext context = new NodeAgentContextImpl.Builder(containerName + ".domain.tld") + NodeAgentContext context = new NodeAgentContextImpl.Builder(containerName + ".domain.tld", 1000) .fileSystem(fileSystem).build(); Path containerVespaHomeOnHost = context.pathOnHostFromPathInNode(context.pathInNodeUnderVespaHome("")); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java index d809d9cbf96..d1e34befa38 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java @@ -27,7 +27,7 @@ public class CoreCollectorTest { private final String GDB_PATH = "/my/path/to/gdb"; private final DockerOperations docker = mock(DockerOperations.class); private final CoreCollector coreCollector = new CoreCollector(docker, Paths.get(GDB_PATH)); - private final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld").build(); + private final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld", 1000).build(); private final Path TEST_CORE_PATH = Paths.get("/tmp/core.1234"); private final Path TEST_BIN_PATH = Paths.get("/usr/bin/program"); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java index 8d599660ace..ec294f1e118 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java @@ -47,7 +47,7 @@ import static org.mockito.Mockito.when; public class CoredumpHandlerTest { private final FileSystem fileSystem = TestFileSystem.create(); private final Path donePath = fileSystem.getPath("/home/docker/dumps"); - private final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld") + private final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-123.domain.tld", 1000) .fileSystem(fileSystem).build(); private final Path crashPathInContainer = Paths.get("/var/crash"); private final Path doneCoredumpsPath = fileSystem.getPath("/home/docker/dumps"); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java index 84f13ed299a..60ac93c41d8 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java @@ -12,7 +12,7 @@ import static org.junit.Assert.assertEquals; */ public class NodeAgentContextImplTest { private final FileSystem fileSystem = TestFileSystem.create(); - private final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-1.domain.tld") + private final NodeAgentContext context = new NodeAgentContextImpl.Builder("container-1.domain.tld", 1000) .fileSystem(fileSystem).build(); @Test diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index 83ee9b57918..af5c17ff34f 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -63,7 +63,7 @@ public class NodeAgentImplTest { private static final String vespaVersion = "1.2.3"; private final String hostName = "host1.test.yahoo.com"; - private final NodeAgentContext context = new NodeAgentContextImpl.Builder(hostName).build(); + private final NodeAgentContext context = new NodeAgentContextImpl.Builder(hostName, 1000).build(); private final DockerImage dockerImage = new DockerImage("dockerImage"); private final DockerOperations dockerOperations = mock(DockerOperations.class); private final NodeRepository nodeRepository = mock(NodeRepository.class); -- cgit v1.2.3