From 14cc5b5a235d5e928b1db8c4970f44315b9a047f Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Mon, 29 May 2023 12:11:00 +0300 Subject: [PATCH 1/3] Targets management version 4 Redesign targets management once more: * Users still make target content changes (outside of the tool) * Let repository do the actual metadata changes during signing-event (the method is called status() but signing_event_update() might be better) * Repository also documents the target changes at high level in the signing event * Users sign normally: "playground-sign " This makes the UX much more neat and does not complicate the repository too much. The expected targets metadata change in e2e tests is because the PlaygroundRepository does not figure out version numbers as smartly as SignerRepository does, and ends up bumping versions more than needed: this could be fixed but it's not critical targets metadata. This change means "playground-status" now makes commits and pushes them to signing event branch. This means the commands name is now not very descriptive. Changing it should not be disruptive but I did not want to grow this PR, so leaving that for later. --- playground/README.md | 33 ++--- .../repo/playground/_playground_repository.py | 104 +++++++++++++- playground/repo/playground/status.py | 115 +++++++++++---- .../playground_sign/_signer_repository.py | 131 ------------------ playground/signer/playground_sign/sign.py | 11 +- playground/tests/e2e.sh | 59 ++++---- .../metadata/2.snapshot.json | 2 +- .../{2.targets.json => 3.targets.json} | 2 +- 8 files changed, 226 insertions(+), 231 deletions(-) rename playground/tests/expected/target-file-changes/metadata/{2.targets.json => 3.targets.json} (97%) diff --git a/playground/README.md b/playground/README.md index e0c67cb..46c12e7 100644 --- a/playground/README.md +++ b/playground/README.md @@ -111,27 +111,18 @@ Notes on remotes configured in `.playground-sign.ini`: ### Modify target files -1. Make target file changes in the signing event git branch - * Choose a signing event name, create a branch - ``` - git fetch origin - git switch -C sign/my-target-changes origin/main - ``` - * Make changes with tools of your choosing: - ``` - echo "test content" > targets/file.txt - git add targets/file.txt - git commit -m "Add a target file" - ``` - * Submit changes to a signing event branch on the repository (by pushing to repository - or by using a PR to a signing event branch): This starts a signing event - ``` - git push origin sign/my-target-changes - ``` -1. Update targets metadata - ``` - playground-sign sign/my-target-changes - ``` +Make target file changes in the signing event git branch using tools and review processes +of your choice. + +``` +git fetch origin +git switch -C sign/my-target-changes origin/main +echo "test content" > targets/file.txt +git commit -m "Add a target file" -- targets/file.txt +git push origin sign/my-target-changes +``` + +This starts a signing event (or updates an existing signing event). ### Sign changes made by others diff --git a/playground/repo/playground/_playground_repository.py b/playground/repo/playground/_playground_repository.py index 68c89ef..f450ae2 100644 --- a/playground/repo/playground/_playground_repository.py +++ b/playground/repo/playground/_playground_repository.py @@ -1,5 +1,6 @@ from dataclasses import dataclass from datetime import datetime, timedelta +from enum import Enum, unique from glob import glob import json import logging @@ -9,7 +10,7 @@ from securesystemslib.signer import Signature, Signer, SigstoreKey, SigstoreSigner, KEY_FOR_TYPE_AND_SCHEME from sigstore.oidc import detect_credential -from tuf.api.metadata import Key, Metadata, MetaFile, Root, Snapshot, Targets, Timestamp +from tuf.api.metadata import Key, Metadata, MetaFile, Root, Snapshot, TargetFile, Targets, Timestamp from tuf.repository import AbortEdit, Repository from tuf.api.serialization.json import CanonicalJSONSerializer, JSONSerializer @@ -21,12 +22,29 @@ logger = logging.getLogger(__name__) +@unique +class State(Enum): + ADDED = 0, + MODIFIED = 1, + REMOVED = 2, + + +@dataclass +class TargetState: + target: TargetFile + state: State + + def __str__(self): + return f"{self.target.path}: {self.state.name}" + + @dataclass class SigningStatus: invites: set[str] # invites to _delegations_ of the role signed: set[str] missing: set[str] threshold: int + target_changes: list[TargetState] valid: bool message: str | None @@ -238,6 +256,64 @@ def _validate_role(self, delegator: Metadata, rolename: str) -> tuple[bool, str return True, None + def _build_targets(self, target_dir: str, rolename: str) -> dict[str, TargetFile]: + """Build a roles dict of TargetFile based on target files in a directory""" + targetfiles = {} + + if rolename == "targets": + root_dir = target_dir + else: + root_dir = os.path.join(target_dir, rolename) + + for fname in glob("*", root_dir=root_dir): + realpath = os.path.join(root_dir, fname) + if not os.path.isfile(realpath): + continue + + # targetpath is a URL path, not OS path + if rolename == "targets": + targetpath = fname + else: + targetpath = f"{rolename}/{fname}" + targetfiles[targetpath] = TargetFile.from_file(targetpath, realpath, ["sha256"]) + + return targetfiles + + def _known_good_targets(self, rolename: str) -> Targets: + """Return Targets from the known good version (signing event start point)""" + prev_path = os.path.join(self._prev_dir, f"{rolename}.json") + if os.path.exists(prev_path): + with open(prev_path, "rb") as f: + md = Metadata.from_bytes(f.read()) + assert isinstance(md.signed, Targets) + return md.signed + else: + # this role did not exist: return an empty one for comparison purposes + return Targets() + + def _get_target_changes(self, rolename: str) -> list[TargetState]: + """Compare targetfiles in known good version and signing event version: + return list of changes""" + + if rolename in ["root", "timestamp", "snapshot"]: + return [] + + changes = [] + + known_good_targetfiles = self._known_good_targets(rolename).targets + for targetfile in self.targets(rolename).targets.values(): + if targetfile.path not in known_good_targetfiles: + changes.append(TargetState(targetfile, State.ADDED)) + elif targetfile != known_good_targetfiles[targetfile.path]: + changes.append(TargetState(targetfile, State.MODIFIED)) + del known_good_targetfiles[targetfile.path] + + for targetfile in known_good_targetfiles.values(): + changes.append(TargetState(targetfile, State.REMOVED)) + + return changes + + def _get_signing_status(self, delegator: Metadata, rolename: str) -> SigningStatus: """Build signing status for role. @@ -270,10 +346,16 @@ def _get_signing_status(self, delegator: Metadata, rolename: str) -> SigningStat except (KeyError, UnverifiedSignatureError): missing_sigs.add(keyowner) + # Document changes to targets metadata in this signing event + target_changes = self._get_target_changes(rolename) + # Just to be sure: double check that delegation threshold is reached - valid, msg = self._validate_role(delegator, rolename) + if invites: + valid, msg = False, None + else: + valid, msg = self._validate_role(delegator, rolename) - return SigningStatus(invites, sigs, missing_sigs, role.threshold, valid, msg) + return SigningStatus(invites, sigs, missing_sigs, role.threshold, target_changes, valid, msg) def status(self, rolename: str) -> tuple[SigningStatus, SigningStatus | None]: """Returns signing status for role. @@ -343,3 +425,19 @@ def bump_expiring(self, rolename:str) -> int | None: raise AbortEdit return signed.version if bumped else None + + + def update_targets(self, rolename: str) -> bool: + if rolename in ["root", "timestamp", "snapshot"]: + return False + + new_target_dict = self._build_targets(os.path.join(self._dir, "..", "targets"), rolename) + with self.edit_targets(rolename) as targets: + # if targets dict has no changes, cancel the metadata edit + if targets.targets == new_target_dict: + raise AbortEdit("No target changes needed") + + targets.targets = new_target_dict + return True + + return False \ No newline at end of file diff --git a/playground/repo/playground/status.py b/playground/repo/playground/status.py index fad672f..d52a96b 100644 --- a/playground/repo/playground/status.py +++ b/playground/repo/playground/status.py @@ -18,17 +18,21 @@ def _git(cmd: list[str]) -> subprocess.CompletedProcess: cmd = ["git", "-c", "user.name=repository-playground", "-c", "user.email=41898282+github-actions[bot]@users.noreply.github.com"] + cmd - proc = subprocess.run(cmd, check=True, capture_output=True, text=True) - logger.debug("%s:\n%s", cmd, proc.stdout) - return proc + try: + proc = subprocess.run(cmd, check=True, capture_output=True, text=True) + logger.debug("%s:\n%s", cmd, proc.stdout) + return proc + except subprocess.CalledProcessError as e: + print ("Git output on error:", e.stdout, e.stderr) + raise e -def _find_changed_roles(known_good_dir: str, signing_event_dir: str) -> list[str]: +def _find_changed_roles(known_good_dir: str, signing_event_dir: str) -> set[str]: # find the files that have changed or been added - # TODO what about removed? + # TODO what about removed roles? files = glob("*.json", root_dir=signing_event_dir) - changed_roles = [] + changed_roles = set() for fname in files: if ( not os.path.exists(f"{known_good_dir}/{fname}") or @@ -37,13 +41,33 @@ def _find_changed_roles(known_good_dir: str, signing_event_dir: str) -> list[str if fname in ["timestamp.json", "snapshot.json"]: assert("Unexpected change in online files") - changed_roles.append(fname[:-len(".json")]) + changed_roles.add(fname[:-len(".json")]) + + return changed_roles - # reorder, toplevels first - for toplevel in ["targets", "root"]: - if toplevel in changed_roles: - changed_roles.remove(toplevel) - changed_roles.insert(0, toplevel) + +def _find_changed_target_roles(known_good_targets_dir: str, targets_dir: str) -> set[str]: + files = ( + glob("*", root_dir=targets_dir) + + glob("*/*", root_dir=targets_dir) + + glob("*", root_dir=known_good_targets_dir) + + glob("*/*", root_dir=known_good_targets_dir) + ) + changed_roles = set() + for filepath in files: + f1 = os.path.join(targets_dir, filepath) + f2 = os.path.join(known_good_targets_dir, filepath) + try: + if filecmp.cmp(f1 ,f2, shallow=False): + continue + except FileNotFoundError: + pass + + # we've found a changed target, add the rolename to list. Handle "targets" as special case + rolename, _, _ = filepath.rpartition(filepath) + if not rolename: + rolename = "targets" + changed_roles.add(rolename) return changed_roles @@ -61,32 +85,45 @@ def _role_status(repo: PlaygroundRepository, role:str, event_name) -> bool: signed = signed | prev_status.signed missing = missing | prev_status.missing + if role_is_valid and not status.invites: + emoji = "heavy_check_mark" + else: + emoji = "x" + click.echo(f"#### :{emoji}: {role}") + if status.invites: - click.echo(f"#### :x: {role}") click.echo(f"{role} delegations have open invites ({', '.join(status.invites)}).") click.echo(f"Invitees can accept the invitations by running `playground-sign {event_name}`") - elif role_is_valid: - click.echo(f"#### :heavy_check_mark: {role}") - click.echo(f"{role} is verified and signed by {sig_counts} signers ({', '.join(signed)}).") - elif signed: - click.echo(f"#### :x:{role}") - click.echo(f"{role} is not yet verified. It is signed by {sig_counts} signers ({', '.join(signed)}).") - else: - click.echo(f"#### :x: {role}") - click.echo(f"{role} is unsigned and not yet verified") + + if not status.invites: + if status.target_changes: + click.echo(f"{role} contains following target file changes:") + for target_state in status.target_changes: + click.echo(f" * {target_state}") + click.echo("") + + + if role_is_valid: + click.echo(f"{role} is verified and signed by {sig_counts} signers ({', '.join(signed)}).") + elif signed: + click.echo(f"{role} is not yet verified. It is signed by {sig_counts} signers ({', '.join(signed)}).") + else: + click.echo(f"{role} is unsigned and not yet verified") + + if missing: + click.echo(f"Still missing signatures from {', '.join(missing)}") + click.echo(f"Signers can sign these changes by running `playground-sign {event_name}`") if status.message: click.echo(f"**Error**: {status.message}") - elif missing and not status.invites: - click.echo(f"Still missing signatures from {', '.join(missing)}") - click.echo(f"Signers can sign these changes by running `playground-sign {event_name}`") - return role_is_valid and len(status.invites) == 0 + return role_is_valid and not status.invites @click.command() @click.option("-v", "--verbose", count=True, default=0) -def status(verbose: int) -> None: +@click.option("--push/--no-push", default=True) +def status(verbose: int, push: bool) -> None: """Status markdown output tool for Repository Playground CI""" logging.basicConfig(level=logging.WARNING - verbose * 10) @@ -110,14 +147,32 @@ def status(verbose: int) -> None: _git(["clone", "--quiet", ".", known_good_dir]) _git(["-C", known_good_dir, "checkout", "--quiet", merge_base]) - good_dir = os.path.join(known_good_dir, "metadata") + good_metadata = os.path.join(known_good_dir, "metadata") + good_targets = os.path.join(known_good_dir, "targets") success = True # Compare current repository and the known good version. # Print status for each role, count invalid roles - repo = PlaygroundRepository("metadata", good_dir) - for role in _find_changed_roles(good_dir, "metadata"): + repo = PlaygroundRepository("metadata", good_metadata) + + roles = list(_find_changed_roles(good_metadata, "metadata") | _find_changed_target_roles(good_targets, "targets")) + + # reorder, toplevels first + for toplevel in ["targets", "root"]: + if toplevel in roles: + roles.remove(toplevel) + roles.insert(0, toplevel) + + for role in roles: + if repo.update_targets(role): + # metadata and target content are not in sync: make a commit with metadata changes + msg = f"Update targets metadata for role {role}" + _git(["commit", "-m", msg, "--", f"metadata/{role}.json"]) + if not _role_status(repo, role, event_name): success = False + if push: + _git(["push", "origin", event_name]) + sys.exit(0 if success else 1) diff --git a/playground/signer/playground_sign/_signer_repository.py b/playground/signer/playground_sign/_signer_repository.py index fc90086..c43dd1c 100644 --- a/playground/signer/playground_sign/_signer_repository.py +++ b/playground/signer/playground_sign/_signer_repository.py @@ -26,31 +26,14 @@ KEY_FOR_TYPE_AND_SCHEME[("sigstore-oidc", "Fulcio")] = SigstoreKey SIGNER_FOR_URI_SCHEME[SigstoreSigner.SCHEME] = SigstoreSigner -# NOTE This signer state should likely be just separate attributes -# of the SignerRepository: It should be possible to have multiple states -# "on" at the same time (e.g. INVITED, TARGETS_CHANGED & SIGNATURE_NEEDED) @unique class SignerState(Enum): NO_ACTION = 0, UNINITIALIZED = 1, INVITED = 2, - TARGETS_CHANGED = 3 SIGNATURE_NEEDED = 4, -@unique -class State(Enum): - ADDED = 0, - MODIFIED = 1, - REMOVED = 2, - - -@dataclass -class TargetState: - target: TargetFile - state: State - - @dataclass class OnlineConfig: # All keys are used as signing keys for both snapshot and timestamp @@ -68,43 +51,6 @@ class OfflineConfig: expiry_period: int signing_period: int -class TargetStates(defaultdict[str, dict[str, TargetState]]): - def __init__(self, target_dir: str): - self.default_factory=dict - # Check what targets we have on disk, mark them as ADDED for now - self.unknown_rolenames = set() - for path in glob("*", root_dir=target_dir) + glob("*/*", root_dir=target_dir): - realpath = os.path.join(target_dir, path) - if not os.path.isfile(realpath): - continue - - # targetpath is a URL path, not OS path - rolename, fname = os.path.split(path) - if rolename: - targetpath = f"{rolename}/{fname}" - else: - rolename = "targets" - targetpath = fname - - target = TargetFile.from_file(targetpath, realpath, ["sha256"]) - # actual state may also be MODIFIED (or no change), see below - self[rolename][targetpath] = TargetState(target, State.ADDED) - self.unknown_rolenames.add(rolename) - - def update_target_states(self, rolename: str, targets: Targets): - """Mark target state as MODIFIED or REMOVED (or remove the state if target is unchanged)""" - self.unknown_rolenames.discard(rolename) - for target in targets.targets.values(): - if target.path in self[rolename]: - if target == self[rolename][target.path].target: - del self[rolename][target.path] - if not self[rolename]: - del self[rolename] - else: - self[rolename][target.path].state = State.MODIFIED - else: - self[rolename][target.path] = TargetState(target, State.REMOVED) - def _find_changed_roles(known_good_dir: str, signing_event_dir: str) -> list[str]: """Return list of roles that exist and have changed in this signing event""" @@ -147,12 +93,6 @@ def __init__(self, dir: str, prev_dir: str, user_name: str, secret_func: Callabl config = json.load(f) self._invites = config["invites"] - # Find changes between known good metadata and the target files in signing event. - # NOTE: Currently target file location is hard coded to a directory in the git-tree - # There is a plan to expose an external targets location in the UI as well. - target_dir = os.path.join(self._dir, "..", "targets") - self.target_changes = self._get_target_states(target_dir) - # Figure out needed signatures self.unsigned = [] for rolename in _find_changed_roles(self._prev_dir, self._dir): @@ -164,8 +104,6 @@ def __init__(self, dir: str, prev_dir: str, user_name: str, secret_func: Callabl self.state = SignerState.UNINITIALIZED elif self.invites: self.state = SignerState.INVITED - elif self._unapplied_target_changes(): - self.state = SignerState.TARGETS_CHANGED elif self.unsigned: self.state = SignerState.SIGNATURE_NEEDED else: @@ -179,46 +117,6 @@ def invites(self) -> list[str]: except KeyError: return [] - def _get_target_states(self, target_dir: str) -> dict[str, dict[str, TargetState]]: - """Returns current state of target files vs target metadata. - - Current state of target files comes from given targets directory. - Target metadata on the other hand is from the "known good metadata state". - - Raises ValueError if target files have been added for a role that does not exist. - First dict key in return value is rolename, second key is targetpath - """ - - # Check what targets we have in the signing event, mark them as ADDED for now - target_states = TargetStates(target_dir) - - # Update target states based on targets metadata in known good state - targets = self._known_good_targets("targets") - target_states.update_target_states("targets", targets) - if targets.delegations and targets.delegations.roles: - for rolename in targets.delegations.roles: - target_states.update_target_states(rolename, self._known_good_targets(rolename)) - - if target_states.unknown_rolenames: - raise ValueError(f"Targets have been added for unknown roles {target_states.unknown_rolenames}") - - return target_states - - def _unapplied_target_changes(self) -> bool: - """Returns True if there are target changes in the signing event branch that are - not yet included in the signing event metadata""" - for rolename, target_states in self.target_changes.items(): - targets = self.targets(rolename) - for path, target_state in target_states.items(): - if target_state.state == State.REMOVED: - if path in targets.targets: - return True - else: - if path not in targets.targets or targets.targets[target_state.target.path] != target_state.target: - return True - - return False - def _user_signature_needed(self, rolename: str) -> bool: """Return true if current role metadata is unsigned by user""" md = self.open(rolename) @@ -349,18 +247,6 @@ def close(self, role: str, md: Metadata) -> None: self._write(role, md) - def _known_good_targets(self, rolename: str) -> Targets: - prev_path = os.path.join(self._prev_dir, f"{rolename}.json") - if os.path.exists(prev_path): - with open(prev_path, "rb") as f: - md = Metadata.from_bytes(f.read()) - assert isinstance(md.signed, Targets) - return md.signed - else: - # this role did not exist: return an empty one for comparison purposes - return Targets() - - @staticmethod def _get_delegated_rolenames(md: Metadata) -> list[str]: if isinstance(md.signed, Root): @@ -551,23 +437,6 @@ def set_role_config(self, rolename: str, config: OfflineConfig, signing_key: Key def status(self, rolename: str) -> str: return "TODO: Describe the changes in the signing event for this role" - def update_targets(self): - """Modify targets metadata to match the target file changes and sign - - Start with 'known good' TargetFiles: the metadata in the signing - event could have been changed in unpredictable ways: target_changes - documents changes from known good state""" - for rolename, target_states in self.target_changes.items(): - known_good_targets = self._known_good_targets(rolename).targets - for target_state in target_states.values(): - if target_state.state == State.REMOVED: - del known_good_targets[target_state.target.path] - else: - known_good_targets[target_state.target.path] = target_state.target - - with self.edit_targets(rolename) as targets: - targets.targets = known_good_targets - def sign(self, rolename: str): """Sign without payload changes""" md = self.open(rolename) diff --git a/playground/signer/playground_sign/sign.py b/playground/signer/playground_sign/sign.py index 99f7d1d..256c256 100755 --- a/playground/signer/playground_sign/sign.py +++ b/playground/signer/playground_sign/sign.py @@ -14,7 +14,7 @@ SignerConfig, signing_event, ) -from playground_sign._signer_repository import SignerState, State +from playground_sign._signer_repository import SignerState logger = logging.getLogger(__name__) @@ -54,15 +54,6 @@ def sign(verbose: int, push: bool, event_name: str): click.echo(repo.status(rolename)) repo.sign(rolename) changed = True - elif repo.state == SignerState.TARGETS_CHANGED: - click.echo(f"Target file changes have been found in this signing event:") - for rolename, states in repo.target_changes.items(): - for target_state in states.values(): - click.echo(f" {target_state.target.path} ({target_state.state.name})") - click.prompt(bold("Press enter to approve these changes"), default=True, show_default=False) - - repo.update_targets() - changed = True elif repo.state == SignerState.SIGNATURE_NEEDED: click.echo(f"Your signature is requested for role(s) {repo.unsigned}.") for rolename in repo.unsigned: diff --git a/playground/tests/e2e.sh b/playground/tests/e2e.sh index 8be4210..eff764a 100755 --- a/playground/tests/e2e.sh +++ b/playground/tests/e2e.sh @@ -81,7 +81,7 @@ repo_setup() # Clone upstream to repo, create a dummy commit so merges are possible git_repo clone --quiet $UPSTREAM_GIT . 2>/dev/null - touch $REPO_GIT/.dummy + touch $REPO_GIT/.dummy $REPO_DIR/out git_repo add .dummy git_repo commit -m "init" --quiet git_repo push --quiet @@ -261,7 +261,7 @@ signer_sign() done | playground-sign $EVENT >> $SIGNER_DIR/out 2>&1 } -signer_add_targets_and_sign() +signer_add_targets() { USER=$1 EVENT=$2 @@ -281,20 +281,9 @@ signer_add_targets_and_sign() git add targets/file1.txt targets/file2.txt git commit --quiet -m "Add 2 target files" git push --quiet origin $EVENT - - # run playground-sign: creates a commit, pushes it to remote signing event branch - INPUT=( - "" # press enter to approve target changes - "0000" # sign the role - "" # press enter to push - ) - - for line in "${INPUT[@]}"; do - echo $line - done | playground-sign $EVENT >> $SIGNER_DIR/out 2>&1 } -signer_modify_targets_and_sign() +signer_modify_targets() { USER=$1 EVENT=$2 @@ -313,17 +302,6 @@ signer_modify_targets_and_sign() git add targets/file1.txt git commit --quiet -m "Modify target files" git push --quiet origin $EVENT - - # run playground-sign: creates a commit, pushes it to remote signing event branch - INPUT=( - "" # press enter to approve target changes - "0000" # sign the role - "" # press enter to push - ) - - for line in "${INPUT[@]}"; do - echo $line - done | playground-sign $EVENT >> $SIGNER_DIR/out 2>&1 } @@ -332,8 +310,8 @@ repo_merge() EVENT=$1 # update repo from upstream and merge the event branch + git_repo switch --quiet main git_repo fetch --quiet origin - git_repo pull --quiet git_repo merge --quiet origin/$EVENT # run playground-status to check that all is ok @@ -349,13 +327,15 @@ repo_status_fail() # update repo from upstream and merge the event branch git_repo fetch --quiet origin - git_repo checkout --quiet origin/$EVENT + git_repo checkout --quiet $EVENT + git_repo pull --quiet # run playground-status, expect failure + # Note that playground-status may make a commit (to modify targets metadata) even if end result is failure # TODO: check output for specifics cd $REPO_GIT + if playground-status >> $REPO_DIR/out; then - echo "Unexpected status success" return 1 fi git_repo checkout --quiet main @@ -363,9 +343,12 @@ repo_status_fail() repo_snapshot() { + git_repo switch --quiet main + git_repo pull --quiet + cd $REPO_GIT - git_repo pull --quiet + if LOCAL_TESTING_KEY=$ONLINE_KEY playground-snapshot --push $PUBLISH_DIR >> $REPO_DIR/out 2>&1; then echo "generated=true" >> $REPO_DIR/out else @@ -375,9 +358,11 @@ repo_snapshot() repo_bump_versions() { + git_repo switch --quiet main + git_repo pull --quiet + cd $REPO_GIT - git_repo pull --quiet if LOCAL_TESTING_KEY=$ONLINE_KEY playground-bump-online --push $PUBLISH_DIR >> $REPO_DIR/out 2>&1; then echo "generated=true" >> $REPO_DIR/out else @@ -510,10 +495,16 @@ test_target_changes() repo_snapshot # This section modifies targets in a new signing event - # user1: Add two targets. Sign - signer_add_targets_and_sign user1 sign/new-targets - # user2: delete one target, modify another. Sign - signer_modify_targets_and_sign user2 sign/new-targets + # User 1 adds target files, repository modifies metadata, user 1 signs + signer_add_targets user1 sign/new-targets + repo_status_fail sign/new-targets + signer_sign user1 sign/new-targets + + # user2: delete one target, modify another. repo modifies metadata, user2 signs + signer_modify_targets user2 sign/new-targets + repo_status_fail sign/new-targets + signer_sign user2 sign/new-targets + # user1: original signature is no longer valid: sign again signer_sign user1 sign/new-targets diff --git a/playground/tests/expected/target-file-changes/metadata/2.snapshot.json b/playground/tests/expected/target-file-changes/metadata/2.snapshot.json index ff0ec07..1154239 100644 --- a/playground/tests/expected/target-file-changes/metadata/2.snapshot.json +++ b/playground/tests/expected/target-file-changes/metadata/2.snapshot.json @@ -10,7 +10,7 @@ "expires": "2022-02-03T01:02:03Z", "meta": { "targets.json": { - "version": 2 + "version": 3 } }, "spec_version": "1.0.31", diff --git a/playground/tests/expected/target-file-changes/metadata/2.targets.json b/playground/tests/expected/target-file-changes/metadata/3.targets.json similarity index 97% rename from playground/tests/expected/target-file-changes/metadata/2.targets.json rename to playground/tests/expected/target-file-changes/metadata/3.targets.json index 00bae3d..60cfa30 100644 --- a/playground/tests/expected/target-file-changes/metadata/2.targets.json +++ b/playground/tests/expected/target-file-changes/metadata/3.targets.json @@ -21,7 +21,7 @@ "length": 15 } }, - "version": 2, + "version": 3, "x-playground-expiry-period": 365, "x-playground-signing-period": 60 } From f99b61de43270b8dcf1678af92b8b272759b35e5 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Tue, 30 May 2023 10:40:02 +0300 Subject: [PATCH 2/3] repo: Make a static method actually static --- playground/repo/playground/_playground_repository.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/playground/repo/playground/_playground_repository.py b/playground/repo/playground/_playground_repository.py index f450ae2..8de4e27 100644 --- a/playground/repo/playground/_playground_repository.py +++ b/playground/repo/playground/_playground_repository.py @@ -256,7 +256,8 @@ def _validate_role(self, delegator: Metadata, rolename: str) -> tuple[bool, str return True, None - def _build_targets(self, target_dir: str, rolename: str) -> dict[str, TargetFile]: + @staticmethod + def _build_targets(target_dir: str, rolename: str) -> dict[str, TargetFile]: """Build a roles dict of TargetFile based on target files in a directory""" targetfiles = {} @@ -440,4 +441,4 @@ def update_targets(self, rolename: str) -> bool: targets.targets = new_target_dict return True - return False \ No newline at end of file + return False From 994d74a5eaeacd9927f8d755b5d072ccd4c76ff6 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Tue, 30 May 2023 10:42:10 +0300 Subject: [PATCH 3/3] repo: Run black on the files for lint fixes --- .../repo/playground/_playground_repository.py | 71 ++++++++++++------- playground/repo/playground/bump_expiring.py | 20 ++++-- playground/repo/playground/snapshot.py | 10 ++- playground/repo/playground/status.py | 66 +++++++++++------ 4 files changed, 112 insertions(+), 55 deletions(-) diff --git a/playground/repo/playground/_playground_repository.py b/playground/repo/playground/_playground_repository.py index 8de4e27..07badf3 100644 --- a/playground/repo/playground/_playground_repository.py +++ b/playground/repo/playground/_playground_repository.py @@ -7,10 +7,25 @@ import os import shutil from securesystemslib.exceptions import UnverifiedSignatureError -from securesystemslib.signer import Signature, Signer, SigstoreKey, SigstoreSigner, KEY_FOR_TYPE_AND_SCHEME +from securesystemslib.signer import ( + Signature, + Signer, + SigstoreKey, + SigstoreSigner, + KEY_FOR_TYPE_AND_SCHEME, +) from sigstore.oidc import detect_credential -from tuf.api.metadata import Key, Metadata, MetaFile, Root, Snapshot, TargetFile, Targets, Timestamp +from tuf.api.metadata import ( + Key, + Metadata, + MetaFile, + Root, + Snapshot, + TargetFile, + Targets, + Timestamp, +) from tuf.repository import AbortEdit, Repository from tuf.api.serialization.json import CanonicalJSONSerializer, JSONSerializer @@ -22,11 +37,12 @@ logger = logging.getLogger(__name__) + @unique class State(Enum): - ADDED = 0, - MODIFIED = 1, - REMOVED = 2, + ADDED = (0,) + MODIFIED = (1,) + REMOVED = (2,) @dataclass @@ -40,7 +56,7 @@ def __str__(self): @dataclass class SigningStatus: - invites: set[str] # invites to _delegations_ of the role + invites: set[str] # invites to _delegations_ of the role signed: set[str] missing: set[str] threshold: int @@ -48,8 +64,10 @@ class SigningStatus: valid: bool message: str | None + class SigningEventState: """Class to manage the .signing-event-state file""" + def __init__(self, file_path: str): self._file_path = file_path self._invites = {} @@ -73,7 +91,8 @@ class PlaygroundRepository(Repository): dir: metadata directory to operate on prev_dir: optional known good repository directory """ - def __init__(self, dir: str, prev_dir: str|None = None): + + def __init__(self, dir: str, prev_dir: str | None = None): self._dir = dir self._prev_dir = prev_dir @@ -86,7 +105,7 @@ def _get_filename(self, role: str) -> str: def _get_keys(self, role: str) -> list[Key]: """Return public keys for delegated role""" if role in ["root", "timestamp", "snapshot", "targets"]: - delegator: Root|Targets = self.root() + delegator: Root | Targets = self.root() else: delegator = self.targets() @@ -99,7 +118,7 @@ def _get_keys(self, role: str) -> list[Key]: pass return keys - def open(self, role:str) -> Metadata: + def open(self, role: str) -> Metadata: """Return existing metadata, or create new metadata This is an implementation of Repository.open() @@ -125,7 +144,6 @@ def open(self, role:str) -> Metadata: return md - def signing_expiry_period(self, rolename: str) -> tuple[int, int]: """Extracts the signing and expiry period for a role @@ -145,7 +163,6 @@ def signing_expiry_period(self, rolename: str) -> tuple[int, int]: return (signing_days, expiry_days) - def close(self, rolename: str, md: Metadata) -> None: """Write metadata to a file in repo dir @@ -172,7 +189,7 @@ def close(self, rolename: str, md: Metadata) -> None: md.signatures[key.keyid] = Signature(key.keyid, "") if rolename in ["timestamp", "snapshot"]: - root_md:Metadata[Root] = self.open("root") + root_md: Metadata[Root] = self.open("root") # repository should never write unsigned online roles root_md.verify_delegate(rolename, md) @@ -181,7 +198,6 @@ def close(self, rolename: str, md: Metadata) -> None: with open(filename, "wb") as f: f.write(data) - @property def targets_infos(self) -> dict[str, MetaFile]: """Implementation of Repository.target_infos @@ -212,7 +228,7 @@ def snapshot_info(self) -> MetaFile: """ return MetaFile(self.snapshot().version) - def open_prev(self, role:str) -> Metadata | None: + def open_prev(self, role: str) -> Metadata | None: """Return known good metadata for role (if it exists)""" prev_fname = f"{self._prev_dir}/{role}.json" if os.path.exists(prev_fname): @@ -221,7 +237,9 @@ def open_prev(self, role:str) -> Metadata | None: return None - def _validate_role(self, delegator: Metadata, rolename: str) -> tuple[bool, str | None]: + def _validate_role( + self, delegator: Metadata, rolename: str + ) -> tuple[bool, str | None]: """Validate role compatibility with this repository Returns bool for validity and optional error message""" @@ -276,7 +294,9 @@ def _build_targets(target_dir: str, rolename: str) -> dict[str, TargetFile]: targetpath = fname else: targetpath = f"{rolename}/{fname}" - targetfiles[targetpath] = TargetFile.from_file(targetpath, realpath, ["sha256"]) + targetfiles[targetpath] = TargetFile.from_file( + targetpath, realpath, ["sha256"] + ) return targetfiles @@ -314,7 +334,6 @@ def _get_target_changes(self, rolename: str) -> list[TargetState]: return changes - def _get_signing_status(self, delegator: Metadata, rolename: str) -> SigningStatus: """Build signing status for role. @@ -356,7 +375,9 @@ def _get_signing_status(self, delegator: Metadata, rolename: str) -> SigningStat else: valid, msg = self._validate_role(delegator, rolename) - return SigningStatus(invites, sigs, missing_sigs, role.threshold, target_changes, valid, msg) + return SigningStatus( + invites, sigs, missing_sigs, role.threshold, target_changes, valid, msg + ) def status(self, rolename: str) -> tuple[SigningStatus, SigningStatus | None]: """Returns signing status for role. @@ -395,12 +416,12 @@ def publish(self, directory: str): dst_path = os.path.join(metadata_dir, f"{snapshot.version}.snapshot.json") shutil.copy(os.path.join(self._dir, "snapshot.json"), dst_path) - for filename, metafile in snapshot.meta.items(): + for filename, metafile in snapshot.meta.items(): src_path = os.path.join(self._dir, filename) dst_path = os.path.join(metadata_dir, f"{metafile.version}.{filename}") shutil.copy(src_path, dst_path) - targets = self.targets(filename[:-len(".json")]) + targets = self.targets(filename[: -len(".json")]) for target in targets.targets.values(): parent, sep, name = target.path.rpartition("/") os.makedirs(os.path.join(targets_dir, parent), exist_ok=True) @@ -409,8 +430,7 @@ def publish(self, directory: str): dst_path = os.path.join(targets_dir, parent, f"{hash}.{name}") shutil.copy(src_path, dst_path) - - def bump_expiring(self, rolename:str) -> int | None: + def bump_expiring(self, rolename: str) -> int | None: """Create a new version of role if it is about to expire""" now = datetime.utcnow() bumped = True @@ -427,12 +447,13 @@ def bump_expiring(self, rolename:str) -> int | None: return signed.version if bumped else None - def update_targets(self, rolename: str) -> bool: if rolename in ["root", "timestamp", "snapshot"]: return False - - new_target_dict = self._build_targets(os.path.join(self._dir, "..", "targets"), rolename) + + new_target_dict = self._build_targets( + os.path.join(self._dir, "..", "targets"), rolename + ) with self.edit_targets(rolename) as targets: # if targets dict has no changes, cancel the metadata edit if targets.targets == new_target_dict: diff --git a/playground/repo/playground/bump_expiring.py b/playground/repo/playground/bump_expiring.py index cbc47f2..624da05 100644 --- a/playground/repo/playground/bump_expiring.py +++ b/playground/repo/playground/bump_expiring.py @@ -14,7 +14,13 @@ def _git(cmd: list[str]) -> subprocess.CompletedProcess: - cmd = ["git", "-c", "user.name=repository-playground", "-c", "user.email=41898282+github-actions[bot]@users.noreply.github.com"] + cmd + cmd = [ + "git", + "-c", + "user.name=repository-playground", + "-c", + "user.email=41898282+github-actions[bot]@users.noreply.github.com", + ] + cmd proc = subprocess.run(cmd, check=True, capture_output=True, text=True) logger.debug("%s:\n%s", cmd, proc.stdout) return proc @@ -24,7 +30,7 @@ def _git(cmd: list[str]) -> subprocess.CompletedProcess: @click.option("-v", "--verbose", count=True, default=0) @click.option("--push/--no-push", default=False) @click.argument("publish-dir", required=False) -def bump_online(verbose: int, push: bool, publish_dir: str|None) -> None: +def bump_online(verbose: int, push: bool, publish_dir: str | None) -> None: """Commit new metadata versions for online roles if needed New versions will be signed. @@ -54,7 +60,9 @@ def bump_online(verbose: int, push: bool, publish_dir: str|None) -> None: sys.exit(1) click.echo(msg) - _git(["commit", "-m", msg, "--", "metadata/timestamp.json", "metadata/snapshot.json"]) + _git( + ["commit", "-m", msg, "--", "metadata/timestamp.json", "metadata/snapshot.json"] + ) if push: _git(["push", "origin", "HEAD"]) @@ -78,12 +86,12 @@ def bump_offline(verbose: int, push: bool) -> None: logging.basicConfig(level=logging.WARNING - verbose * 10) repo = PlaygroundRepository("metadata") - events=[] + events = [] for filename in glob("*.json", root_dir="metadata"): if filename in ["timestamp.json", "snapshot.json"]: continue - rolename = filename[:-len(".json")] + rolename = filename[: -len(".json")] version = repo.bump_expiring(rolename) if version is None: logging.debug("No version bump needed for %s", rolename) @@ -107,4 +115,4 @@ def bump_offline(verbose: int, push: bool) -> None: _git(["reset", "--hard", "HEAD^"]) # print out list of created event branches - click.echo(" ".join(events)) \ No newline at end of file + click.echo(" ".join(events)) diff --git a/playground/repo/playground/snapshot.py b/playground/repo/playground/snapshot.py index d78fc3f..aeaad16 100644 --- a/playground/repo/playground/snapshot.py +++ b/playground/repo/playground/snapshot.py @@ -14,7 +14,13 @@ def _git(cmd: list[str]) -> subprocess.CompletedProcess: - cmd = ["git", "-c", "user.name=repository-playground", "-c", "user.email=41898282+github-actions[bot]@users.noreply.github.com"] + cmd + cmd = [ + "git", + "-c", + "user.name=repository-playground", + "-c", + "user.email=41898282+github-actions[bot]@users.noreply.github.com", + ] + cmd proc = subprocess.run(cmd, check=True, text=True) logger.debug("%s:\n%s", cmd, proc.stdout) return proc @@ -24,7 +30,7 @@ def _git(cmd: list[str]) -> subprocess.CompletedProcess: @click.option("-v", "--verbose", count=True, default=0) @click.option("--push/--no-push", default=False) @click.argument("publish-dir", required=False) -def snapshot(verbose: int, push: bool, publish_dir: str|None) -> None: +def snapshot(verbose: int, push: bool, publish_dir: str | None) -> None: """Update The TUF snapshot based on current repository content Create a commit with the snapshot and timestamp changes (if any). diff --git a/playground/repo/playground/status.py b/playground/repo/playground/status.py index d52a96b..66cb420 100644 --- a/playground/repo/playground/status.py +++ b/playground/repo/playground/status.py @@ -16,14 +16,21 @@ logger = logging.getLogger(__name__) + def _git(cmd: list[str]) -> subprocess.CompletedProcess: - cmd = ["git", "-c", "user.name=repository-playground", "-c", "user.email=41898282+github-actions[bot]@users.noreply.github.com"] + cmd + cmd = [ + "git", + "-c", + "user.name=repository-playground", + "-c", + "user.email=41898282+github-actions[bot]@users.noreply.github.com", + ] + cmd try: proc = subprocess.run(cmd, check=True, capture_output=True, text=True) logger.debug("%s:\n%s", cmd, proc.stdout) return proc except subprocess.CalledProcessError as e: - print ("Git output on error:", e.stdout, e.stderr) + print("Git output on error:", e.stdout, e.stderr) raise e @@ -34,31 +41,32 @@ def _find_changed_roles(known_good_dir: str, signing_event_dir: str) -> set[str] files = glob("*.json", root_dir=signing_event_dir) changed_roles = set() for fname in files: - if ( - not os.path.exists(f"{known_good_dir}/{fname}") or - not filecmp.cmp(f"{signing_event_dir}/{fname}", f"{known_good_dir}/{fname}", shallow=False) + if not os.path.exists(f"{known_good_dir}/{fname}") or not filecmp.cmp( + f"{signing_event_dir}/{fname}", f"{known_good_dir}/{fname}", shallow=False ): if fname in ["timestamp.json", "snapshot.json"]: - assert("Unexpected change in online files") + assert "Unexpected change in online files" - changed_roles.add(fname[:-len(".json")]) + changed_roles.add(fname[: -len(".json")]) return changed_roles -def _find_changed_target_roles(known_good_targets_dir: str, targets_dir: str) -> set[str]: +def _find_changed_target_roles( + known_good_targets_dir: str, targets_dir: str +) -> set[str]: files = ( - glob("*", root_dir=targets_dir) + - glob("*/*", root_dir=targets_dir) + - glob("*", root_dir=known_good_targets_dir) + - glob("*/*", root_dir=known_good_targets_dir) + glob("*", root_dir=targets_dir) + + glob("*/*", root_dir=targets_dir) + + glob("*", root_dir=known_good_targets_dir) + + glob("*/*", root_dir=known_good_targets_dir) ) changed_roles = set() for filepath in files: f1 = os.path.join(targets_dir, filepath) f2 = os.path.join(known_good_targets_dir, filepath) try: - if filecmp.cmp(f1 ,f2, shallow=False): + if filecmp.cmp(f1, f2, shallow=False): continue except FileNotFoundError: pass @@ -72,7 +80,7 @@ def _find_changed_target_roles(known_good_targets_dir: str, targets_dir: str) -> return changed_roles -def _role_status(repo: PlaygroundRepository, role:str, event_name) -> bool: +def _role_status(repo: PlaygroundRepository, role: str, event_name) -> bool: status, prev_status = repo.status(role) role_is_valid = status.valid sig_counts = f"{len(status.signed)}/{status.threshold}" @@ -92,8 +100,12 @@ def _role_status(repo: PlaygroundRepository, role:str, event_name) -> bool: click.echo(f"#### :{emoji}: {role}") if status.invites: - click.echo(f"{role} delegations have open invites ({', '.join(status.invites)}).") - click.echo(f"Invitees can accept the invitations by running `playground-sign {event_name}`") + click.echo( + f"{role} delegations have open invites ({', '.join(status.invites)})." + ) + click.echo( + f"Invitees can accept the invitations by running `playground-sign {event_name}`" + ) if not status.invites: if status.target_changes: @@ -102,17 +114,22 @@ def _role_status(repo: PlaygroundRepository, role:str, event_name) -> bool: click.echo(f" * {target_state}") click.echo("") - if role_is_valid: - click.echo(f"{role} is verified and signed by {sig_counts} signers ({', '.join(signed)}).") + click.echo( + f"{role} is verified and signed by {sig_counts} signers ({', '.join(signed)})." + ) elif signed: - click.echo(f"{role} is not yet verified. It is signed by {sig_counts} signers ({', '.join(signed)}).") + click.echo( + f"{role} is not yet verified. It is signed by {sig_counts} signers ({', '.join(signed)})." + ) else: click.echo(f"{role} is unsigned and not yet verified") if missing: click.echo(f"Still missing signatures from {', '.join(missing)}") - click.echo(f"Signers can sign these changes by running `playground-sign {event_name}`") + click.echo( + f"Signers can sign these changes by running `playground-sign {event_name}`" + ) if status.message: click.echo(f"**Error**: {status.message}") @@ -133,7 +150,9 @@ def status(verbose: int, push: bool) -> None: click.echo(f"Event [{event_name}](../compare/{event_name})") if not os.path.exists("metadata/root.json"): - click.echo(f"Repository does not exist yet. Create one with `playground-delegate {event_name}`.") + click.echo( + f"Repository does not exist yet. Create one with `playground-delegate {event_name}`." + ) sys.exit(1) # Find the known-good commit @@ -155,7 +174,10 @@ def status(verbose: int, push: bool) -> None: # Print status for each role, count invalid roles repo = PlaygroundRepository("metadata", good_metadata) - roles = list(_find_changed_roles(good_metadata, "metadata") | _find_changed_target_roles(good_targets, "targets")) + roles = list( + _find_changed_roles(good_metadata, "metadata") + | _find_changed_target_roles(good_targets, "targets") + ) # reorder, toplevels first for toplevel in ["targets", "root"]: