From ed3751601856fc136aefd3b091e73d1b879a391b Mon Sep 17 00:00:00 2001 From: Appu Goundan Date: Mon, 4 Nov 2024 11:43:34 -0500 Subject: [PATCH] More tuf updates for conformance - don't persist N.root.json, could clash with a delegation - validate at parsetime the type of meta file - generify metastore a bit more, force updater to clear the right meta - validate any bootstrapped root at update start time Signed-off-by: Appu Goundan --- .../dev/sigstore/tuf/FileSystemTufStore.java | 33 ++------------- .../main/java/dev/sigstore/tuf/MetaStore.java | 23 ++-------- .../tuf/PassthroughCacheMetaStore.java | 20 ++------- .../dev/sigstore/tuf/TrustedMetaStore.java | 5 ++- .../main/java/dev/sigstore/tuf/Updater.java | 20 +++++---- .../java/dev/sigstore/tuf/model/Root.java | 6 +++ .../java/dev/sigstore/tuf/model/Snapshot.java | 6 +++ .../java/dev/sigstore/tuf/model/Targets.java | 6 +++ .../dev/sigstore/tuf/model/Timestamp.java | 6 +++ .../sigstore/tuf/FileSystemTufStoreTest.java | 21 +++------- .../tuf/PassthroughCacheMetaStoreTest.java | 42 ++++--------------- 11 files changed, 63 insertions(+), 125 deletions(-) diff --git a/sigstore-java/src/main/java/dev/sigstore/tuf/FileSystemTufStore.java b/sigstore-java/src/main/java/dev/sigstore/tuf/FileSystemTufStore.java index 5fbbd06f..ef829312 100644 --- a/sigstore-java/src/main/java/dev/sigstore/tuf/FileSystemTufStore.java +++ b/sigstore-java/src/main/java/dev/sigstore/tuf/FileSystemTufStore.java @@ -20,9 +20,7 @@ import com.google.common.annotations.VisibleForTesting; import dev.sigstore.tuf.model.*; import java.io.BufferedWriter; -import java.io.File; import java.io.IOException; -import java.nio.file.FileAlreadyExistsException; import java.nio.file.Files; import java.nio.file.Path; import java.util.Optional; @@ -30,9 +28,6 @@ /** Uses a local file system directory to store the trusted TUF metadata. */ public class FileSystemTufStore implements MetaStore, TargetStore { - private static final String ROOT_FILE_NAME = "root.json"; - private static final String SNAPSHOT_FILE_NAME = "snapshot.json"; - private static final String TIMESTAMP_FILE_NAME = "timestamp.json"; private final Path repoBaseDir; private final Path targetsCache; @@ -101,30 +96,10 @@ > void storeRole(String roleName, T role) throws IOEx } @Override - public void writeRoot(Root root) throws IOException { - Optional trustedRoot = readMeta(RootRole.ROOT, Root.class); - if (trustedRoot.isPresent()) { - try { - Files.move( - repoBaseDir.resolve(ROOT_FILE_NAME), - repoBaseDir.resolve( - trustedRoot.get().getSignedMeta().getVersion() + "." + ROOT_FILE_NAME)); - } catch (FileAlreadyExistsException e) { - // The file is already backed-up. continue. - } - } - storeRole(RootRole.ROOT, root); - } - - @Override - public void clearMetaDueToKeyRotation() throws IOException { - File snapshotMetaFile = repoBaseDir.resolve(SNAPSHOT_FILE_NAME).toFile(); - if (snapshotMetaFile.exists()) { - Files.delete(snapshotMetaFile.toPath()); - } - File timestampMetaFile = repoBaseDir.resolve(TIMESTAMP_FILE_NAME).toFile(); - if (timestampMetaFile.exists()) { - Files.delete(timestampMetaFile.toPath()); + public void clearMeta(String role) throws IOException { + Path metaFile = repoBaseDir.resolve(role + ".json"); + if (Files.isRegularFile(metaFile)) { + Files.delete(metaFile); } } } diff --git a/sigstore-java/src/main/java/dev/sigstore/tuf/MetaStore.java b/sigstore-java/src/main/java/dev/sigstore/tuf/MetaStore.java index a3a4b8be..7a7edafd 100644 --- a/sigstore-java/src/main/java/dev/sigstore/tuf/MetaStore.java +++ b/sigstore-java/src/main/java/dev/sigstore/tuf/MetaStore.java @@ -15,7 +15,6 @@ */ package dev.sigstore.tuf; -import dev.sigstore.tuf.model.Root; import dev.sigstore.tuf.model.SignedTufMeta; import dev.sigstore.tuf.model.TufMeta; import java.io.IOException; @@ -31,8 +30,7 @@ public interface MetaStore extends MetaReader { String getIdentifier(); /** - * Generic method to store one of the {@link SignedTufMeta} resources in the local tuf store. Do - * not use for Root role, use {@link #writeRoot(Root)} instead. + * Generic method to store one of the {@link SignedTufMeta} resources in the local tuf store. * * @param roleName the name of the role * @param meta the metadata to store @@ -41,26 +39,13 @@ public interface MetaStore extends MetaReader { void writeMeta(String roleName, SignedTufMeta meta) throws IOException; /** - * Once you have ascertained that your root is trustworthy use this method to persist it to your - * local store. This will usually only be called with a root loaded statically from a bundled - * trusted root, or after the successful verification of an updated root from a mirror. - * - * @param root a root that has been proven trustworthy by the client - * @throws IOException since some implementations may persist the root to disk or over the network - * we throw {@code IOException} in case of IO error. - * @see 5.3.8 - */ - void writeRoot(Root root) throws IOException; - - /** - * This clears out the snapshot and timestamp metadata from the store, as required when snapshot - * or timestamp verification keys have changed as a result of a root update. + * Generic method to remove meta, useful when keys rotated in root. Deletion is not optional, + * implementers must ensure meta is removed from the storage medium. * * @throws IOException implementations that read/write IO to clear the data may throw {@code * IOException} * @see 5.3.11 */ - void clearMetaDueToKeyRotation() throws IOException; + void clearMeta(String role) throws IOException; } diff --git a/sigstore-java/src/main/java/dev/sigstore/tuf/PassthroughCacheMetaStore.java b/sigstore-java/src/main/java/dev/sigstore/tuf/PassthroughCacheMetaStore.java index ee6f3314..146d9208 100644 --- a/sigstore-java/src/main/java/dev/sigstore/tuf/PassthroughCacheMetaStore.java +++ b/sigstore-java/src/main/java/dev/sigstore/tuf/PassthroughCacheMetaStore.java @@ -15,14 +15,11 @@ */ package dev.sigstore.tuf; -import dev.sigstore.tuf.model.Root; -import dev.sigstore.tuf.model.RootRole; import dev.sigstore.tuf.model.SignedTufMeta; import dev.sigstore.tuf.model.TufMeta; import java.io.IOException; import java.util.HashMap; import java.util.Map; -import java.util.Objects; import java.util.Optional; /** An in memory cache that will pass through to a provided local tuf store. */ @@ -44,13 +41,6 @@ public static PassthroughCacheMetaStore newPassthroughMetaCache(MetaStore localS return new PassthroughCacheMetaStore(localStore); } - @Override - public void writeRoot(Root root) throws IOException { - // call writeRoot instead of generic writeMeta because it may do extra work when storing on disk - localStore.writeRoot(root); - cache.put(RootRole.ROOT, root); - } - @Override @SuppressWarnings("unchecked") public > Optional readMeta( @@ -69,17 +59,13 @@ public > Optional readMeta( @Override public void writeMeta(String roleName, SignedTufMeta meta) throws IOException { - if (Objects.equals(roleName, RootRole.ROOT)) { - throw new IllegalArgumentException("Calling writeMeta on root instead of writeRoot"); - } localStore.writeMeta(roleName, meta); cache.put(roleName, meta); } @Override - public void clearMetaDueToKeyRotation() throws IOException { - localStore.clearMetaDueToKeyRotation(); - cache.remove(RootRole.TIMESTAMP); - cache.remove(RootRole.SNAPSHOT); + public void clearMeta(String role) throws IOException { + localStore.clearMeta(role); + cache.remove(role); } } diff --git a/sigstore-java/src/main/java/dev/sigstore/tuf/TrustedMetaStore.java b/sigstore-java/src/main/java/dev/sigstore/tuf/TrustedMetaStore.java index 5471a95b..221b281e 100644 --- a/sigstore-java/src/main/java/dev/sigstore/tuf/TrustedMetaStore.java +++ b/sigstore-java/src/main/java/dev/sigstore/tuf/TrustedMetaStore.java @@ -70,7 +70,7 @@ > T getMeta(String roleName, Class } public void setRoot(Root root) throws IOException { - metaStore.writeRoot(root); + metaStore.writeMeta(RootRole.ROOT, root); } public Root getRoot() throws IOException { @@ -118,6 +118,7 @@ public Optional findTargets() throws IOException { } public void clearMetaDueToKeyRotation() throws IOException { - metaStore.clearMetaDueToKeyRotation(); + metaStore.clearMeta(RootRole.TIMESTAMP); + metaStore.clearMeta(RootRole.SNAPSHOT); } } diff --git a/sigstore-java/src/main/java/dev/sigstore/tuf/Updater.java b/sigstore-java/src/main/java/dev/sigstore/tuf/Updater.java index 6cd04628..071a8f39 100644 --- a/sigstore-java/src/main/java/dev/sigstore/tuf/Updater.java +++ b/sigstore-java/src/main/java/dev/sigstore/tuf/Updater.java @@ -104,15 +104,14 @@ public void updateMeta() throws IOException, NoSuchAlgorithmException, InvalidKe updateRoot(); var oldTimestamp = trustedMetaStore.findTimestamp(); updateTimestamp(); - if (Objects.equals(oldTimestamp.orElse(null), trustedMetaStore.getTimestamp()) - && trustedMetaStore.findSnapshot().isPresent() - && trustedMetaStore.findTargets().isPresent()) { - return; + if (!Objects.equals(oldTimestamp.orElse(null), trustedMetaStore.getTimestamp()) + || trustedMetaStore.findSnapshot().isEmpty() + || trustedMetaStore.findTargets().isEmpty()) { + // if we need to update or we can't find targets/snapshots locally then grab new snapshot and + // targets from remote + updateSnapshot(); + updateTargets(); } - // if we need to update or we can't find targets/timestamps locally then grab new snapshot and - // targets from remote - updateSnapshot(); - updateTargets(); } /** Download a single target defined in targets. Does not handle delegated targets. */ @@ -141,7 +140,11 @@ void updateRoot() trustedRoot = localRoot.get(); } else { trustedRoot = GSON.get().fromJson(trustedRootPath.get(), Root.class); + trustedMetaStore.setRoot(trustedRoot); } + // verify root that we're bootstrapping this update with is good to go + verifyDelegate(trustedRoot, trustedRoot); + int baseVersion = trustedRoot.getSignedMeta().getVersion(); int nextVersion = baseVersion + 1; // keep these for verifying the last step. 5.3.11 @@ -194,7 +197,6 @@ void updateRoot() trustedRoot.getSignedMeta().getRoles().get(RootRole.TIMESTAMP))) { trustedMetaStore.clearMetaDueToKeyRotation(); } - trustedMetaStore.setRoot(trustedRoot); } private void throwIfExpired(ZonedDateTime expires) { diff --git a/sigstore-java/src/main/java/dev/sigstore/tuf/model/Root.java b/sigstore-java/src/main/java/dev/sigstore/tuf/model/Root.java index ece796c1..77c8005f 100644 --- a/sigstore-java/src/main/java/dev/sigstore/tuf/model/Root.java +++ b/sigstore-java/src/main/java/dev/sigstore/tuf/model/Root.java @@ -15,6 +15,7 @@ */ package dev.sigstore.tuf.model; +import com.google.common.base.Preconditions; import org.immutables.gson.Gson; import org.immutables.value.Value; import org.immutables.value.Value.Derived; @@ -29,4 +30,9 @@ public interface Root extends SignedTufMeta { default RootMeta getSignedMeta() { return getSignedMeta(RootMeta.class); } + + @Value.Check + default void checkType() { + Preconditions.checkState(getSignedMeta().getType().equals("root")); + } } diff --git a/sigstore-java/src/main/java/dev/sigstore/tuf/model/Snapshot.java b/sigstore-java/src/main/java/dev/sigstore/tuf/model/Snapshot.java index fba892d5..d40aa12a 100644 --- a/sigstore-java/src/main/java/dev/sigstore/tuf/model/Snapshot.java +++ b/sigstore-java/src/main/java/dev/sigstore/tuf/model/Snapshot.java @@ -15,6 +15,7 @@ */ package dev.sigstore.tuf.model; +import com.google.common.base.Preconditions; import org.immutables.gson.Gson; import org.immutables.value.Value; import org.immutables.value.Value.Derived; @@ -29,4 +30,9 @@ public interface Snapshot extends SignedTufMeta { default SnapshotMeta getSignedMeta() { return getSignedMeta(SnapshotMeta.class); } + + @Value.Check + default void checkType() { + Preconditions.checkState(getSignedMeta().getType().equals("snapshot")); + } } diff --git a/sigstore-java/src/main/java/dev/sigstore/tuf/model/Targets.java b/sigstore-java/src/main/java/dev/sigstore/tuf/model/Targets.java index e45f74b5..ca3b4f22 100644 --- a/sigstore-java/src/main/java/dev/sigstore/tuf/model/Targets.java +++ b/sigstore-java/src/main/java/dev/sigstore/tuf/model/Targets.java @@ -15,6 +15,7 @@ */ package dev.sigstore.tuf.model; +import com.google.common.base.Preconditions; import org.immutables.gson.Gson; import org.immutables.value.Value; import org.immutables.value.Value.Derived; @@ -29,4 +30,9 @@ public interface Targets extends SignedTufMeta { default TargetMeta getSignedMeta() { return getSignedMeta(TargetMeta.class); } + + @Value.Check + default void checkType() { + Preconditions.checkState(getSignedMeta().getType().equals("targets")); + } } diff --git a/sigstore-java/src/main/java/dev/sigstore/tuf/model/Timestamp.java b/sigstore-java/src/main/java/dev/sigstore/tuf/model/Timestamp.java index 5f5d3cee..2b4ccc28 100644 --- a/sigstore-java/src/main/java/dev/sigstore/tuf/model/Timestamp.java +++ b/sigstore-java/src/main/java/dev/sigstore/tuf/model/Timestamp.java @@ -15,6 +15,7 @@ */ package dev.sigstore.tuf.model; +import com.google.common.base.Preconditions; import org.immutables.gson.Gson; import org.immutables.value.Value; import org.immutables.value.Value.Derived; @@ -30,4 +31,9 @@ public interface Timestamp extends SignedTufMeta { default TimestampMeta getSignedMeta() { return getSignedMeta(TimestampMeta.class); } + + @Value.Check + default void checkType() { + Preconditions.checkState(getSignedMeta().getType().equals("timestamp")); + } } diff --git a/sigstore-java/src/test/java/dev/sigstore/tuf/FileSystemTufStoreTest.java b/sigstore-java/src/test/java/dev/sigstore/tuf/FileSystemTufStoreTest.java index 0694b20d..d6d28042 100644 --- a/sigstore-java/src/test/java/dev/sigstore/tuf/FileSystemTufStoreTest.java +++ b/sigstore-java/src/test/java/dev/sigstore/tuf/FileSystemTufStoreTest.java @@ -43,33 +43,24 @@ void newFileSystemStore_hasRepo(@TempDir Path repoBase) throws IOException { } @Test - void setTrustedRoot_noPrevious(@TempDir Path repoBase) throws IOException { + void writeMeta(@TempDir Path repoBase) throws IOException { FileSystemTufStore tufStore = FileSystemTufStore.newFileSystemStore(repoBase); assertFalse(repoBase.resolve("root.json").toFile().exists()); - tufStore.writeRoot(TestResources.loadRoot(TestResources.UPDATER_REAL_TRUSTED_ROOT)); + tufStore.writeMeta( + RootRole.ROOT, TestResources.loadRoot(TestResources.UPDATER_REAL_TRUSTED_ROOT)); assertEquals(2, repoBase.toFile().list().length, "Expect 2: root.json plus the /targets dir."); assertTrue(repoBase.resolve("root.json").toFile().exists()); assertTrue(repoBase.resolve("targets").toFile().isDirectory()); } @Test - void setTrustedRoot_backupPerformed(@TempDir Path repoBase) throws IOException { - TestResources.setupRepoFiles(PROD_REPO, repoBase, "root.json"); - FileSystemTufStore tufStore = FileSystemTufStore.newFileSystemStore(repoBase); - int version = tufStore.readMeta(RootRole.ROOT, Root.class).get().getSignedMeta().getVersion(); - assertFalse(repoBase.resolve(version + ".root.json").toFile().exists()); - tufStore.writeRoot(TestResources.loadRoot(TestResources.UPDATER_REAL_TRUSTED_ROOT)); - assertTrue(repoBase.resolve(version + ".root.json").toFile().exists()); - } - - @Test - void clearMetaDueToKeyRotation(@TempDir Path repoBase) throws IOException { + void clearMeta(@TempDir Path repoBase) throws IOException { TestResources.setupRepoFiles(PROD_REPO, repoBase, "snapshot.json", "timestamp.json"); FileSystemTufStore tufStore = FileSystemTufStore.newFileSystemStore(repoBase); assertTrue(repoBase.resolve("snapshot.json").toFile().exists()); assertTrue(repoBase.resolve("timestamp.json").toFile().exists()); - tufStore.clearMetaDueToKeyRotation(); - assertFalse(repoBase.resolve("snapshot.json").toFile().exists()); + tufStore.clearMeta(RootRole.TIMESTAMP); + assertTrue(repoBase.resolve("snapshot.json").toFile().exists()); assertFalse(repoBase.resolve("timestamp.json").toFile().exists()); } } diff --git a/sigstore-java/src/test/java/dev/sigstore/tuf/PassthroughCacheMetaStoreTest.java b/sigstore-java/src/test/java/dev/sigstore/tuf/PassthroughCacheMetaStoreTest.java index d1618782..3596b905 100644 --- a/sigstore-java/src/test/java/dev/sigstore/tuf/PassthroughCacheMetaStoreTest.java +++ b/sigstore-java/src/test/java/dev/sigstore/tuf/PassthroughCacheMetaStoreTest.java @@ -20,12 +20,10 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import com.google.common.io.Resources; -import dev.sigstore.tuf.model.Root; import dev.sigstore.tuf.model.RootRole; import dev.sigstore.tuf.model.Timestamp; import java.io.BufferedWriter; import java.io.IOException; -import java.net.URISyntaxException; import java.nio.file.Files; import java.nio.file.Path; import org.junit.jupiter.api.BeforeAll; @@ -39,14 +37,10 @@ class PassthroughCacheMetaStoreTest { private FileSystemTufStore fileSystemTufStore; private PassthroughCacheMetaStore passthroughCacheMetaStore; - private static Root root; private static Timestamp timestamp; @BeforeAll - public static void readAllMeta() throws IOException, URISyntaxException { - Path rootResource = - Path.of(Resources.getResource("dev/sigstore/tuf/real/prod/root.json").getPath()); - root = GSON.get().fromJson(Files.newBufferedReader(rootResource), Root.class); + public static void readAllMeta() throws IOException { Path timestampResource = Path.of(Resources.getResource("dev/sigstore/tuf/real/prod/timestamp.json").getPath()); timestamp = GSON.get().fromJson(Files.newBufferedReader(timestampResource), Timestamp.class); @@ -59,34 +53,9 @@ public void setup() throws IOException { PassthroughCacheMetaStore.newPassthroughMetaCache(fileSystemTufStore); } - @Test - public void root_test() throws Exception { - assertTrue(fileSystemTufStore.readMeta(RootRole.ROOT, Root.class).isEmpty()); - assertTrue(passthroughCacheMetaStore.readMeta(RootRole.ROOT, Root.class).isEmpty()); - - passthroughCacheMetaStore.writeRoot(root); - - assertEquals(root, fileSystemTufStore.readMeta(RootRole.ROOT, Root.class).get()); - assertEquals(root, passthroughCacheMetaStore.readMeta(RootRole.ROOT, Root.class).get()); - } - - @Test - public void root_canInitFromDisk() throws Exception { - assertTrue(fileSystemTufStore.readMeta(RootRole.ROOT, Root.class).isEmpty()); - assertTrue(passthroughCacheMetaStore.readMeta(RootRole.ROOT, Root.class).isEmpty()); - - try (BufferedWriter fileWriter = Files.newBufferedWriter(localStore.resolve("root.json"))) { - GSON.get().toJson(root, fileWriter); - } - - assertEquals(root, fileSystemTufStore.readMeta(RootRole.ROOT, Root.class).get()); - assertEquals(root, passthroughCacheMetaStore.readMeta(RootRole.ROOT, Root.class).get()); - } - @Test public void meta_test() throws Exception { - // root uses special handling for writing, but the rest of them don't, so we just test - // timestamp here arbitrarily + // test timestamp here arbitrarily assertTrue(fileSystemTufStore.readMeta(RootRole.TIMESTAMP, Timestamp.class).isEmpty()); assertTrue(passthroughCacheMetaStore.readMeta(RootRole.TIMESTAMP, Timestamp.class).isEmpty()); @@ -95,10 +64,15 @@ public void meta_test() throws Exception { assertEquals(timestamp, fileSystemTufStore.readMeta(RootRole.TIMESTAMP, Timestamp.class).get()); assertEquals( timestamp, passthroughCacheMetaStore.readMeta(RootRole.TIMESTAMP, Timestamp.class).get()); + + passthroughCacheMetaStore.clearMeta(RootRole.TIMESTAMP); + + assertTrue(fileSystemTufStore.readMeta(RootRole.TIMESTAMP, Timestamp.class).isEmpty()); + assertTrue(passthroughCacheMetaStore.readMeta(RootRole.TIMESTAMP, Timestamp.class).isEmpty()); } @Test - public void timestamp_canInitFromDisk() throws Exception { + public void readMeta_canInitFromDisk() throws Exception { assertTrue(fileSystemTufStore.readMeta(RootRole.TIMESTAMP, Timestamp.class).isEmpty()); assertTrue(passthroughCacheMetaStore.readMeta(RootRole.TIMESTAMP, Timestamp.class).isEmpty());