summaryrefslogtreecommitdiffstats
path: root/node-admin
diff options
context:
space:
mode:
authorTor Brede Vekterli <vekterli@yahooinc.com>2022-10-31 11:41:27 +0100
committerTor Brede Vekterli <vekterli@yahooinc.com>2022-11-01 10:36:12 +0100
commite189af391f6d450c90b9a481e6f436aaad908de3 (patch)
treef328fe73c7771aaa762010c0bb8602653055b14b /node-admin
parente68a945e40ac91015f31deec1088b8f3edaa016a (diff)
Add encryption capabilities to core dump handler
Once wired in (not currently the case), a Supplier of non-null `SecretSharedKey` instances will trigger: 1. Wrapping the output stream with an encrypting output stream using the secret component of the supplied key. Zstd compression is handled on the input stream, so this should transparently encrypt compressed data. To disambiguate, encrypted core dumps are suffixed with an additional `.enc` file extension. 2. Emitting a public decryption token as part of the metadata using the shared component of the supplied key.
Diffstat (limited to 'node-admin')
-rw-r--r--node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java63
-rw-r--r--node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java104
2 files changed, 132 insertions, 35 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java
index 54aa136d877..3d8669b8782 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java
@@ -1,7 +1,11 @@
// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
+import com.fasterxml.jackson.core.JsonProcessingException;
import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.yahoo.security.SecretSharedKey;
+import com.yahoo.security.SharedKeyGenerator;
import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec;
import com.yahoo.vespa.hosted.node.admin.container.metrics.Dimensions;
import com.yahoo.vespa.hosted.node.admin.container.metrics.Metrics;
@@ -13,6 +17,7 @@ import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath;
import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath;
import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal;
+import javax.crypto.CipherOutputStream;
import java.io.IOException;
import java.io.OutputStream;
import java.io.UncheckedIOException;
@@ -46,6 +51,7 @@ public class CoredumpHandler {
private static final String PROCESSING_DIRECTORY_NAME = "processing";
private static final String METADATA_FILE_NAME = "metadata.json";
private static final String COMPRESSED_EXTENSION = ".zstd";
+ private static final String ENCRYPTED_EXTENSION = ".enc";
public static final String COREDUMP_FILENAME_PREFIX = "dump_";
private final Logger logger = Logger.getLogger(CoredumpHandler.class.getName());
@@ -58,6 +64,7 @@ public class CoredumpHandler {
private final Metrics metrics;
private final Clock clock;
private final Supplier<String> coredumpIdSupplier;
+ private final Supplier<SecretSharedKey> secretSharedKeySupplier;
/**
* @param crashPathInContainer path inside the container where core dump are dumped
@@ -66,12 +73,13 @@ public class CoredumpHandler {
public CoredumpHandler(Terminal terminal, CoreCollector coreCollector, CoredumpReporter coredumpReporter,
String crashPathInContainer, Path doneCoredumpsPath, Metrics metrics) {
this(coreCollector, coredumpReporter, crashPathInContainer, doneCoredumpsPath,
- metrics, Clock.systemUTC(), () -> UUID.randomUUID().toString());
+ metrics, Clock.systemUTC(), () -> UUID.randomUUID().toString(), () -> null /*TODO*/);
}
CoredumpHandler(CoreCollector coreCollector, CoredumpReporter coredumpReporter,
String crashPathInContainer, Path doneCoredumpsPath, Metrics metrics,
- Clock clock, Supplier<String> coredumpIdSupplier) {
+ Clock clock, Supplier<String> coredumpIdSupplier,
+ Supplier<SecretSharedKey> secretSharedKeySupplier) {
this.coreCollector = coreCollector;
this.coredumpReporter = coredumpReporter;
this.crashPatchInContainer = crashPathInContainer;
@@ -79,6 +87,7 @@ public class CoredumpHandler {
this.metrics = metrics;
this.clock = clock;
this.coredumpIdSupplier = coredumpIdSupplier;
+ this.secretSharedKeySupplier = secretSharedKeySupplier;
}
@@ -151,10 +160,12 @@ public class CoredumpHandler {
void processAndReportSingleCoredump(NodeAgentContext context, ContainerPath coredumpDirectory, Supplier<Map<String, Object>> nodeAttributesSupplier) {
try {
- String metadata = getMetadata(context, coredumpDirectory, nodeAttributesSupplier);
+ Optional<SecretSharedKey> sharedCoreKey = Optional.ofNullable(secretSharedKeySupplier.get());
+ Optional<String> decryptionToken = sharedCoreKey.map(k -> k.sealedSharedKey().toTokenString());
+ String metadata = getMetadata(context, coredumpDirectory, nodeAttributesSupplier, decryptionToken);
String coredumpId = coredumpDirectory.getFileName().toString();
coredumpReporter.reportCoredump(coredumpId, metadata);
- finishProcessing(context, coredumpDirectory);
+ finishProcessing(context, coredumpDirectory, sharedCoreKey);
context.log(logger, "Successfully reported coredump " + coredumpId);
} catch (Exception e) {
throw new RuntimeException("Failed to process coredump " + coredumpDirectory, e);
@@ -165,7 +176,7 @@ public class CoredumpHandler {
* @return coredump metadata from metadata.json if present, otherwise attempts to get metadata using
* {@link CoreCollector} and stores it to metadata.json
*/
- String getMetadata(NodeAgentContext context, ContainerPath coredumpDirectory, Supplier<Map<String, Object>> nodeAttributesSupplier) throws IOException {
+ String getMetadata(NodeAgentContext context, ContainerPath coredumpDirectory, Supplier<Map<String, Object>> nodeAttributesSupplier, Optional<String> decryptionToken) throws IOException {
UnixPath metadataPath = new UnixPath(coredumpDirectory.resolve(METADATA_FILE_NAME));
if (!metadataPath.exists()) {
ContainerPath coredumpFile = findCoredumpFileInProcessingDirectory(coredumpDirectory);
@@ -175,25 +186,51 @@ public class CoredumpHandler {
.resolve(context.containerName().asString())
.resolve(coredumpDirectory.getFileName().toString())
.resolve(coredumpFile.getFileName().toString()).toString());
+ decryptionToken.ifPresent(token -> metadata.put("decryption_token", token));
String metadataFields = objectMapper.writeValueAsString(Map.of("fields", metadata));
metadataPath.writeUtf8File(metadataFields);
return metadataFields;
} else {
- return metadataPath.readUtf8File();
+ if (decryptionToken.isPresent()) {
+ // Since encryption keys are single-use and generated for each core dump processing invocation,
+ // we must ensure we store and report the token associated with the _latest_ (i.e. current)
+ // attempt at processing the core dump. Patch and rewrite the file with a new token, if present.
+ String metadataFields = metadataWithPatchedTokenValue(metadataPath, decryptionToken.get());
+ metadataPath.deleteIfExists();
+ metadataPath.writeUtf8File(metadataFields);
+ return metadataFields;
+ } else {
+ return metadataPath.readUtf8File();
+ }
}
}
+ private String metadataWithPatchedTokenValue(UnixPath metadataPath, String decryptionToken) throws JsonProcessingException {
+ var jsonRoot = objectMapper.readTree(metadataPath.readUtf8File());
+ if (jsonRoot.path("fields").isObject()) {
+ ((ObjectNode)jsonRoot.get("fields")).put("decryption_token", decryptionToken);
+ } // else: unit testing case without real metadata
+ return objectMapper.writeValueAsString(jsonRoot);
+ }
+
+ static OutputStream maybeWrapWithEncryption(OutputStream wrappedStream, Optional<SecretSharedKey> sharedCoreKey) {
+ return sharedCoreKey
+ .map(key -> (OutputStream)new CipherOutputStream(wrappedStream, SharedKeyGenerator.makeAesGcmEncryptionCipher(key)))
+ .orElse(wrappedStream);
+ }
+
/**
- * Compresses core file (and deletes the uncompressed core), then moves the entire core dump processing
- * directory to {@link #doneCoredumpsPath} for archive
+ * Compresses and, if a key is provided, encrypts core file (and deletes the uncompressed core), then moves
+ * the entire core dump processing directory to {@link #doneCoredumpsPath} for archive
*/
- private void finishProcessing(NodeAgentContext context, ContainerPath coredumpDirectory) throws IOException {
+ private void finishProcessing(NodeAgentContext context, ContainerPath coredumpDirectory, Optional<SecretSharedKey> sharedCoreKey) throws IOException {
ContainerPath coreFile = findCoredumpFileInProcessingDirectory(coredumpDirectory);
- ContainerPath compressedCoreFile = coreFile.resolveSibling(coreFile.getFileName() + COMPRESSED_EXTENSION);
+ String extension = COMPRESSED_EXTENSION + (sharedCoreKey.isPresent() ? ENCRYPTED_EXTENSION : "");
+ ContainerPath compressedCoreFile = coreFile.resolveSibling(coreFile.getFileName() + extension);
try (ZstdCompressingInputStream zcis = new ZstdCompressingInputStream(Files.newInputStream(coreFile));
- OutputStream fos = Files.newOutputStream(compressedCoreFile)) {
+ OutputStream fos = maybeWrapWithEncryption(Files.newOutputStream(compressedCoreFile), sharedCoreKey)) {
zcis.transferTo(fos);
} catch (IOException e) {
throw new UncheckedIOException(e);
@@ -208,7 +245,8 @@ public class CoredumpHandler {
ContainerPath findCoredumpFileInProcessingDirectory(ContainerPath coredumpProccessingDirectory) {
return (ContainerPath) FileFinder.files(coredumpProccessingDirectory)
- .match(nameStartsWith(COREDUMP_FILENAME_PREFIX).and(nameEndsWith(COMPRESSED_EXTENSION).negate()))
+ .match(nameStartsWith(COREDUMP_FILENAME_PREFIX).and(nameEndsWith(COMPRESSED_EXTENSION).negate())
+ .and(nameEndsWith(ENCRYPTED_EXTENSION).negate()))
.maxDepth(1)
.stream()
.map(FileFinder.FileAttributes::path)
@@ -225,6 +263,7 @@ public class CoredumpHandler {
.match(nameStartsWith(".").negate())
.match(nameMatches(HS_ERR_PATTERN).negate())
.match(nameEndsWith(COMPRESSED_EXTENSION).negate())
+ .match(nameEndsWith(ENCRYPTED_EXTENSION).negate())
.match(nameStartsWith("metadata").negate())
.list().size();
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 082a1f3de58..04cfe0d5d63 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
@@ -1,6 +1,8 @@
// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
+import com.yahoo.security.SealedSharedKey;
+import com.yahoo.security.SecretSharedKey;
import com.yahoo.test.ManualClock;
import com.yahoo.vespa.hosted.node.admin.container.metrics.DimensionMetrics;
import com.yahoo.vespa.hosted.node.admin.container.metrics.Metrics;
@@ -12,7 +14,9 @@ import com.yahoo.vespa.test.file.TestFileSystem;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
+import javax.crypto.spec.SecretKeySpec;
import java.io.IOException;
+import java.nio.charset.StandardCharsets;
import java.nio.file.FileSystem;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -53,8 +57,11 @@ public class CoredumpHandlerTest {
private final ManualClock clock = new ManualClock();
@SuppressWarnings("unchecked")
private final Supplier<String> coredumpIdSupplier = mock(Supplier.class);
+ @SuppressWarnings("unchecked")
+ private final Supplier<SecretSharedKey> secretSharedKeySupplier = mock(Supplier.class);
private final CoredumpHandler coredumpHandler = new CoredumpHandler(coreCollector, coredumpReporter,
- containerCrashPath.pathInContainer(), doneCoredumpsPath, metrics, clock, coredumpIdSupplier);
+ containerCrashPath.pathInContainer(), doneCoredumpsPath, metrics, clock, coredumpIdSupplier,
+ secretSharedKeySupplier);
@Test
@@ -136,8 +143,20 @@ public class CoredumpHandlerTest {
verify(coredumpIdSupplier, times(1)).get();
}
- @Test
- void get_metadata_test() throws IOException {
+ private static String buildExpectedMetadataString(Optional<String> decryptionToken) {
+ return "{\"fields\":{" +
+ "\"hostname\":\"host123.yahoo.com\"," +
+ "\"kernel_version\":\"3.10.0-862.9.1.el7.x86_64\"," +
+ "\"backtrace\":[\"call 1\",\"function 2\",\"something something\"]," +
+ "\"vespa_version\":\"6.48.4\"," +
+ "\"bin_path\":\"/bin/bash\"," +
+ "\"coredump_path\":\"/home/docker/dumps/container-123/id-123/dump_core.456\"," +
+ "\"docker_image\":\"vespa/ci:6.48.4\"" +
+ decryptionToken.map(",\"decryption_token\":\"%s\""::formatted).orElse("") +
+ "}}";
+ }
+
+ void do_get_metadata_test(Optional<String> oldDecryptionToken, Optional<String> newDecryptionToken) throws IOException {
Map<String, Object> metadata = new HashMap<>();
metadata.put("bin_path", "/bin/bash");
metadata.put("backtrace", List.of("call 1", "function 2", "something something"));
@@ -148,16 +167,7 @@ public class CoredumpHandlerTest {
"kernel_version", "3.10.0-862.9.1.el7.x86_64",
"docker_image", "vespa/ci:6.48.4");
- String expectedMetadataStr = "{\"fields\":{" +
- "\"hostname\":\"host123.yahoo.com\"," +
- "\"kernel_version\":\"3.10.0-862.9.1.el7.x86_64\"," +
- "\"backtrace\":[\"call 1\",\"function 2\",\"something something\"]," +
- "\"vespa_version\":\"6.48.4\"," +
- "\"bin_path\":\"/bin/bash\"," +
- "\"coredump_path\":\"/home/docker/dumps/container-123/id-123/dump_core.456\"," +
- "\"docker_image\":\"vespa/ci:6.48.4\"" +
- "}}";
-
+ String expectedMetadataStr = buildExpectedMetadataString(oldDecryptionToken);
ContainerPath coredumpDirectory = context.paths().of("/var/crash/id-123");
Files.createDirectories(coredumpDirectory.pathOnHost());
@@ -165,45 +175,78 @@ public class CoredumpHandlerTest {
when(coreCollector.collect(eq(context), eq(coredumpDirectory.resolve("dump_core.456"))))
.thenReturn(metadata);
- assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, () -> attributes));
+ assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, () -> attributes, oldDecryptionToken));
verify(coreCollector, times(1)).collect(any(), any());
- // Calling it again will simply read the previously generated metadata from disk
- assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, () -> attributes));
+ // Calling it again will read the previously generated metadata from disk and selectively
+ // patch in an updated decryption token value, if one is provided.
+ // This avoids having to re-run a potentially expensive collector step.
+ expectedMetadataStr = buildExpectedMetadataString(newDecryptionToken);
+ assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, () -> attributes, newDecryptionToken));
verify(coreCollector, times(1)).collect(any(), any());
}
@Test
+ void get_metadata_test_without_encryption() throws IOException {
+ do_get_metadata_test(Optional.empty(), Optional.empty()); // No token in metadata
+ }
+
+ @Test
+ void get_metadata_test_with_encryption() throws IOException {
+ when(secretSharedKeySupplier.get()).thenReturn(makeFixedSecretSharedKey());
+ do_get_metadata_test(Optional.of("AVeryCoolToken"), Optional.of("AnEvenCoolerToken"));
+ }
+
+ @Test
+ void get_metadata_test_without_encryption_then_with_encryption() throws IOException {
+ // Edge where encryption was enabled between attempted processing runs.
+ // We don't bother with testing the opposite edge case (encryption -> no encryption), since
+ // in that case the core dump itself won't be encrypted so the token will be a no-op.
+ do_get_metadata_test(Optional.empty(), Optional.of("TheSwaggestToken"));
+ }
+
+ @Test
void cant_get_metadata_if_no_core_file() {
assertThrows(IllegalStateException.class, () -> {
- coredumpHandler.getMetadata(context, context.paths().of("/fake/path"), Map::of);
+ coredumpHandler.getMetadata(context, context.paths().of("/fake/path"), Map::of, Optional.empty());
});
}
@Test
- void fails_to_get_core_file_if_only_compressed() {
+ void fails_to_get_core_file_if_only_compressed_or_encrypted() {
assertThrows(IllegalStateException.class, () -> {
ContainerPath coredumpDirectory = context.paths().of("/path/to/coredump/proccessing/id-123");
Files.createDirectories(coredumpDirectory);
Files.createFile(coredumpDirectory.resolve("dump_bash.core.431.zstd"));
+ Files.createFile(coredumpDirectory.resolve("dump_bash.core.543.zstd.enc"));
coredumpHandler.findCoredumpFileInProcessingDirectory(coredumpDirectory);
});
}
- @Test
- void process_single_coredump_test() throws IOException {
+ void do_process_single_coredump_test(String expectedCoreFileName) throws IOException {
ContainerPath coredumpDirectory = context.paths().of("/path/to/coredump/proccessing/id-123");
Files.createDirectories(coredumpDirectory);
- Files.write(coredumpDirectory.resolve("metadata.json"), "metadata".getBytes());
+ Files.write(coredumpDirectory.resolve("metadata.json"), "{\"test-metadata\":{}}".getBytes());
Files.createFile(coredumpDirectory.resolve("dump_bash.core.431"));
assertFolderContents(coredumpDirectory, "metadata.json", "dump_bash.core.431");
coredumpHandler.processAndReportSingleCoredump(context, coredumpDirectory, Map::of);
verify(coreCollector, never()).collect(any(), any());
- verify(coredumpReporter, times(1)).reportCoredump(eq("id-123"), eq("metadata"));
+ verify(coredumpReporter, times(1)).reportCoredump(eq("id-123"), eq("{\"test-metadata\":{}}"));
assertFalse(Files.exists(coredumpDirectory));
assertFolderContents(doneCoredumpsPath.resolve("container-123"), "id-123");
- assertFolderContents(doneCoredumpsPath.resolve("container-123").resolve("id-123"), "metadata.json", "dump_bash.core.431.zstd");
+ assertFolderContents(doneCoredumpsPath.resolve("container-123").resolve("id-123"), "metadata.json", expectedCoreFileName);
+ }
+
+ @Test
+ void process_single_coredump_test_without_encryption() throws IOException {
+ do_process_single_coredump_test("dump_bash.core.431.zstd");
+ }
+
+ @Test
+ void process_single_coredump_test_with_encryption() throws IOException {
+ when(secretSharedKeySupplier.get()).thenReturn(makeFixedSecretSharedKey());
+ do_process_single_coredump_test("dump_bash.core.431.zstd.enc");
}
@Test
@@ -248,4 +291,19 @@ public class CoredumpHandlerTest {
Files.createFile(path),
FileTime.from(clock.instant().minus(age))));
}
+
+ private static byte[] bytesOf(String str) {
+ return str.getBytes(StandardCharsets.UTF_8);
+ }
+
+ private static SecretSharedKey makeFixedSecretSharedKey() {
+ byte[] keyBytes = bytesOf("very secret yes!"); // 128 bits
+ var secretKey = new SecretKeySpec(keyBytes, "AES");
+ int keyId = 123;
+ // We don't parse any of these fields in the test, so just use dummy contents.
+ byte[] enc = bytesOf("hello world");
+ byte[] ciphertext = bytesOf("imaginary ciphertext");
+ return new SecretSharedKey(secretKey, new SealedSharedKey(keyId, enc, ciphertext));
+ }
+
}