Make tenant keys and certificates readable by 1000 only

11 files changed, 78 insertions, 28 deletions

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
@@ -86,6 +89,11 @@ public class NodeAgentContextImpl implements NodeAgentContext {
     }
 
     @Override
+    public int vespaUserIdOnHost() {
+        return vespaUserIdOnHost;
+    }
+
+    @Override
     public Path pathOnHostFromPathInNode(Path pathInNode) {
         if (! pathInNode.isAbsolute())
             throw new IllegalArgumentException("Expected an absolute path in the container, got: " + pathInNode);
@@ -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<PosixFilePermission> 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<PosixFilePermission> set = getPosixFilePermissionsFromString(permissions);
         FileAttribute<Set<PosixFilePermission>> 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<String, NodeAgent> 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);