fix: check root metadata verification before snapshotting #293
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Asra Ali <[email protected]>
asraa
changed the title
|
I'm open to having the specification make it clear that one should
re-validate the root and the current snapshot key before signing a new
snapshot.
…On Wed, May 18, 2022 at 2:24 AM asraa ***@***.***> wrote:
Signed-off-by: Asra Ali ***@***.***
Details in #292 <#292>
See sigstore/root-signing#238
<sigstore/root-signing#238>
Because root.json is not pinned in snapshot.json, it's metadata is not
verified before Snapshotting. For robustness, we should verify the
root.json is signed correctly, because we're using DB role data from root
to verify other manifests that are pinned, e.g. targets.
I'm open to having this just be a warning, because I don't see this
mentioned in the spec, but overall repo management lacks in the spec.
Please fill in the fields below to submit a pull request. The more
information that is provided, the better.
Fixes #
Release Notes:
*Types of changes*:
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
*Description of the changes being introduced by the pull request*:
*Please verify and check that the pull request fulfills the following
requirements*:
- Tests have been added for the bug fix or new feature
- Docs have been added for the bug fix or new feature
------------------------------
You can view, comment on, or merge this pull request online at:
#293
Commit Summary
- fcb076b
<fcb076b>
Check root metadata verification before snapshotting
File Changes
(2 files <https://github.com/theupdateframework/go-tuf/pull/293/files>)
- *M* repo.go
<https://github.com/theupdateframework/go-tuf/pull/293/files#diff-878b59cdb12e82cfc9660e809860c76059db205b70e2d3032f7e96c29559829d>
(5)
- *M* repo_test.go
<https://github.com/theupdateframework/go-tuf/pull/293/files#diff-6ef45a9c7fed188621d3ae07e39d3f95d45daefac59fa892f7e4393180e00679>
(36)
Patch Links:
- https://github.com/theupdateframework/go-tuf/pull/293.patch
- https://github.com/theupdateframework/go-tuf/pull/293.diff
—
Reply to this email directly, view it on GitHub
<#293>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAGROD63XVV3IDLK3GWGB2DVKPP4TANCNFSM5WFZ2LUQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
|
To be clear, one need not do this if the root has not changed in the
interim. But if the root has been updated, this step is important.
On Thu, May 19, 2022 at 2:45 PM Justin Cappos ***@***.***>
wrote:
… I'm open to having the specification make it clear that one should
re-validate the root and the current snapshot key before signing a new
snapshot.
On Wed, May 18, 2022 at 2:24 AM asraa ***@***.***> wrote:
> Signed-off-by: Asra Ali ***@***.***
>
> Details in #292 <#292>
> See sigstore/root-signing#238
> <sigstore/root-signing#238>
>
> Because root.json is not pinned in snapshot.json, it's metadata is not
> verified before Snapshotting. For robustness, we should verify the
> root.json is signed correctly, because we're using DB role data from root
> to verify other manifests that are pinned, e.g. targets.
>
> I'm open to having this just be a warning, because I don't see this
> mentioned in the spec, but overall repo management lacks in the spec.
>
> Please fill in the fields below to submit a pull request. The more
> information that is provided, the better.
>
> Fixes #
> Release Notes:
>
> *Types of changes*:
>
> - [ ] Bug fix (non-breaking change which fixes an issue)
> - [ ] New feature (non-breaking change which adds functionality)
> - [ ] Breaking change (fix or feature that would cause existing
> functionality to not work as expected)
>
> *Description of the changes being introduced by the pull request*:
>
> *Please verify and check that the pull request fulfills the following
> requirements*:
>
> - Tests have been added for the bug fix or new feature
> - Docs have been added for the bug fix or new feature
>
> ------------------------------
> You can view, comment on, or merge this pull request online at:
>
> #293
> Commit Summary
>
> - fcb076b
> <fcb076b>
> Check root metadata verification before snapshotting
>
> File Changes
>
> (2 files <https://github.com/theupdateframework/go-tuf/pull/293/files>)
>
> - *M* repo.go
> <https://github.com/theupdateframework/go-tuf/pull/293/files#diff-878b59cdb12e82cfc9660e809860c76059db205b70e2d3032f7e96c29559829d>
> (5)
> - *M* repo_test.go
> <https://github.com/theupdateframework/go-tuf/pull/293/files#diff-6ef45a9c7fed188621d3ae07e39d3f95d45daefac59fa892f7e4393180e00679>
> (36)
>
> Patch Links:
>
> - https://github.com/theupdateframework/go-tuf/pull/293.patch
> - https://github.com/theupdateframework/go-tuf/pull/293.diff
>
> —
> Reply to this email directly, view it on GitHub
> <#293>, or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAGROD63XVV3IDLK3GWGB2DVKPP4TANCNFSM5WFZ2LUQ>
> .
> You are receiving this because you are subscribed to this thread.Message
> ID: ***@***.***>
>
|
joshuagl
approved these changes
May 19, 2022
Indeed, we should probably add sections on generating snapshot, timestamp, and root metadata to the repository operations section of the spec, it currently only describes adding and updating targets. |
I can open up a specification PR to add some words on updating snapshot! If I remember other valuable repo operation stuff, will add too. |
znewman01
approved these changes
May 23, 2022
rdimitrov
approved these changes
May 25, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Signed-off-by: Asra Ali [email protected]
Details in #292
See sigstore/root-signing#238
Because
root.jsonis not pinned insnapshot.json, it's metadata is not verified before Snapshotting. For robustness, we should verify the root.json is signed correctly, because we're using DB role data from root to verify other manifests that are pinned, e.g. targets.I'm open to having this just be a warning, because I don't see this mentioned in the spec, but overall repo management lacks in the spec.
Please fill in the fields below to submit a pull request. The more information that is provided, the better.
Fixes #
Release Notes:
Types of changes:
Description of the changes being introduced by the pull request:
Please verify and check that the pull request fulfills the following requirements: