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

Add optional feature to verify the chain of trust from bootstrap trusted root metadata to trusted root metadata #1214

Closed

Conversation

MVrachev
Copy link
Collaborator

@MVrachev MVrachev commented Nov 17, 2020

NOTE: This pr includes commits from #1205

Fixes #1168

Description of the changes being introduced by the pull request:
Add an optional feature to establish a chain of trust between
a bootstrap root metadata file and the trusted current root
metadata.

If bootstrap_root_path is provided, the folder of the bootstrap root
will be expected to contain all intermediate root files between
the bootstrap root version and the current root version.

Also, if this feature is enabled, when updating a root.json file
with _update_metadata call (from updater.py) the root metadata files
marked as "previous" won't be deleted, but instead, they will be moved
to the provided bootstrap root folder.
That way after updating to a new version of the root metadata,
the chain of trust will be preserved.

New root.json metadata files had to be generated in order to
tests properly the establishment of a chain of trust between
a bootstrap root file and the current trustworthy root file.
The sequence 1.root.json, 2.root.json is generated by marking
the older one X.root.json as "dirty" and using writeall()
function from repository_tool.py and this will generate X+1.root.json.

PS: The diff is big because of the 5 new root.json files...

Please verify and check that the pull request fulfills the following
requirements
:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

Currently, the `_get_metadata_file()` from `tuf/client/updater.py`
is really large and does too many things and some of them partially.

Right now, it validates the specification version
and the metadata version in it, but doesn't validate
if the metadata has expired or if the signature could be trusted.
So, it's partially validating the metadata file
and at the same time calling  `_verify_metadata_file()` from updater.py
for the rest of the work which doesn't make sense.

It's logically better if we move the validation of the specification
and the metadata version in separate functions which would
be then called in ` _verify_metadata_file()` and that way
all of the validation will be done in ` _verify_metadata_file()`.

Signed-off-by: Martin Vrachev <[email protected]>
Add an optional feature to establish a chain of trust between
a bootstrap root metadata file and the trusted current root
metadata.

If bootstrap_root_path is provided, the folder of the bootstrap root
will be expected to contain all intermediate root files between
the bootstrap root version and the current root version.
Also, if this feature is enabled, when updating a root.json file
with _update_metadata call (from updater.py) the root metadata files
marked as "previous" won't be deleted, but instead, they will be moved
to the provided bootstrap root folder.
That way after updating to a new version of the root metadata,
the chain of trust will be preserved.

Signed-off-by: Martin Vrachev <[email protected]>
New root.json metadata files had to be generated in order to
tests properly the establishment of a chain of trust between
a bootstrap root file and the current trustworthy root file.

The sequence "1.root.json", "2.root.json" is generated by marking
the older one "X.root.json" as "dirty" and using "writeall()"
function from repository_tool.py.

Signed-off-by: Martin Vrachev <[email protected]>
@MVrachev MVrachev changed the title Chain of trust root Add optional feature to verify the chain of trust from bootstrap trusted root metadata to trusted root metadata Nov 17, 2020
@MVrachev
Copy link
Collaborator Author

I haven't addressed edge cases like the possible situation where a user uses two versions of pip with two different bootstrap files. Read more in this comment #1168 (comment).

Also, I decided that initially, I will focus on the root chain of trust, instead of making a more general function that could be used with snapshot too. I wanted to make sure we catch all of the edge cases here, before making something more general.
Read more here: #1168 (comment) and here #1168 (comment).

@jku
Copy link
Member

jku commented Dec 1, 2020

I think this requires a bit more design work or at least discussion -- either here or maybe rather in the issue. As an end result I'd like to see the core algorithm written as text, with explanation and other options discussed (e.g. why are we loading current runtime metadata first, and only then looking at the bootstrap if we think the bootstrap is more trustworthy?) and the variables at play as well (what should happen if root chain is not complete locally, what if bootstrap root is newer than current runtime root, what if bootstrap and runtime root are just different).

Also might be useful to consider other (non-root) bootstrap metadata already (even if the intention is not to implement that) -- even if just to clearly state that this approach is / is not viable for the other metadata types. The related TAP might be useful to refer to here.

My comments might be a bit all over the place, hope you can find the threads...

If bootstrap_root_path is provided, the folder of the bootstrap root
will be expected to contain all intermediate root files between
the bootstrap root version and the current root version.

Is this a reasonable requirement? What happens if it does not?

Also, if this feature is enabled, when updating a root.json file
with _update_metadata call (from updater.py) the root metadata files
marked as "previous" won't be deleted, but instead, they will be moved
to the provided bootstrap root folder.

This is not possible: bootstrap location may be on a read-only filesystem (or not writable by current user) -- the core idea is that bootstrap may be more trustworthy because user cannot modify it.

I don't see a reason not to store the "old" root metadata files somewhere in the normal runtime metadata directory: please explain if you've noticed one.

The question of whether bootstrap checking should modify runtime metadata (e.g. should it store the bootstrap metadata itself in runtime metadata if it's not there already, should it download new files needed to chain up to current runtime metadata root) needs to be decided... I would think downloading and storing the missing files is appropriate but this probably needs some thinking and playing out the different scenarios -- what if I run pip X that has bootstrap A, and then run pip Y that has bootstrap B, with various values of A and B?

I think I'd like to see a variant of the process that starts the local metadata verification process from the bootstrap root, and continues using the files in the runtime metadata (as a design to compare with this one) -- maybe it even makes sense to put this all in the existing root metadata update loop?

I haven't addressed edge cases like the possible situation where a user uses two versions of pip with two different bootstrap files

The comments above probably make this obvious but: I don't think that is an edge case. Even just upgrading the same pip could mean new, unexpected bootstrap root

@MVrachev
Copy link
Collaborator Author

MVrachev commented Dec 2, 2020

Good points @jku!
This could be considered as a prototype.
I had a brainstorming session with @joshuagl where I wrote down the assumptions I am making in the current proposal
and what and why can be approved.
The steps I am going to take are:

  1. Familiarize with the proposal made in this TAP: Add TAP for client initialization and metadata backstop taps#128
  2. Address @jku comments and change this pr.

@joshuagl joshuagl marked this pull request as draft January 5, 2021 10:17
@MVrachev
Copy link
Collaborator Author

MVrachev commented Feb 23, 2021

On hold, until this TAP is clarified to a point where I would be able to make this pr a prototype of the TAP.

@jku
Copy link
Member

jku commented Jul 19, 2021

On hold, until this TAP is clarified to a point where I would be able to make this pr a prototype of the TAP.

I'm closing this: no point in keeping PRs cluttering the list if they're not going anywhere. I don't think we'll want to touch the legacy updater anymore anyway.

Please re-open if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updater feature request: verify chain of trust from bootstrapped root metadata
2 participants