Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Snapshot contains root #603

Open
haydentherapper opened this issue Jan 25, 2023 · 10 comments
Open

Snapshot contains root #603

haydentherapper opened this issue Jan 25, 2023 · 10 comments
Labels
bug Something isn't working

Comments

@haydentherapper
Copy link
Contributor

Description

The snapshot currently signs over the root here. A TUF TAP specifies that the snapshot no longer needs to sign over the root.

cc @asraa

@haydentherapper haydentherapper added the bug Something isn't working label Jan 25, 2023
@asraa
Copy link
Contributor

asraa commented Jan 25, 2023

Yeah - for context I think we stopped signing over the root at root version 2 with a go-tuf update.

Then it remained because TUF requires persisting metadata files in the snapshot, and that included the root.json at the time.

@jku
Copy link
Member

jku commented Apr 3, 2023

The way to "reset" snapshot contents is to do it while changing snapshot keys (because now the old snapshot is no longer signed by valid keys so clients should not use it for persistence checks)...

A reset like that could obviously have interop issues (if clients disagree on how it actually happens) so not something to do too hastily but it's something to maybe plan for when snapshot keys are next rotated?

@asraa
Copy link
Contributor

asraa commented Apr 3, 2023

The way to "reset" snapshot contents is to do it while changing snapshot keys (because now the old snapshot is no longer signed by valid keys so clients should not use it for persistence checks)...

We have done this before, but the go-tuf binary doesn't actually clear this (i thought it would when we rotated keys I think in v5, but it did not alas)

But I agree - we should stage this next time we rotate snapshot keys.

@haydentherapper
Copy link
Contributor Author

@jku this will be resolved when we switch to tuf-on-ci, yea?

@kommendorkapten
Copy link
Member

@haydentherapper yes, see an example here from Sigstage: https://tuf-repo-cdn.sigstage.dev/6.snapshot.json

@jku
Copy link
Member

jku commented Mar 25, 2024

I think staging does not prove anything (it never had root listed in snapshot as far as I know).

  • I think this (removal of old items from snapshot) likely does not happen automatically as tuf-on-ci is careful about preserving snapshot, but this seems like a good feature for tuf-on-ci (when snapshot keys change, remove old items from snapshot)
  • we may want to be careful with regards to client compat: the spec seems clear to me but knowing the state of TUF conformance testing (nonexistent), we'd want to test this on staging first -- this of course requires some way of adding root into snapshot contents for this test...

@kommendorkapten
Copy link
Member

Either I'm missing the question or I'm not understanding something, but I think tuf-on-ci only adds target infos into the snapshot: https://github.com/theupdateframework/python-tuf/blob/develop/tuf/repository/_repository.py#L192 called from here https://github.com/theupdateframework/tuf-on-ci/blob/main/repo/tuf_on_ci/online_sign.py#L52

If the root meta is stored in the snapshot file, is that updated in a different part of the code?
We can continue this in slack.

@kommendorkapten
Copy link
Member

@jku clarified in slack: the old version of root would remain in the snapshot file, but no new would be added.

@kommendorkapten
Copy link
Member

Also, the root.json included in snapshot.json is very old: https://tuf-repo-cdn.sigstore.dev/132.snapshot.json

it's version 2, which expired two years ago.

@haydentherapper
Copy link
Contributor Author

Yea, this issue was just about TUF specification conformance, that currently root.json is included in the list of files that the snapshot is signed over. Looks like the snapshot generated from TUF-on-CI is correct. I'll leave this open til we migrate prod over.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants