-
Notifications
You must be signed in to change notification settings - Fork 19
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 initial draft of TAP 8 - Key rotation and explicit self-revocation #20
Conversation
TAP 8 has been contributed by Hannes Mehnert. Vlad has formatted his original submission to include missing headings and a TAP header
is compromised can introduce a rotation cycle, all delegation to them | ||
will be invalid. | ||
|
||
NOTE: Since TAP 5, root is not part of snapshot anymore. does this matter? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a link to the specific change in TAP 5:
https://github.com/theupdateframework/taps/blob/685d1c88fe8cc36496f16e273920bccc37b8c53f/tap5.md#changes-to-the-snapshot-metadata-file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrong link, Vlad :)
TAP 5 does not mandate that the root metadata file is removed from being listed in the snapshot metadata file. I think we should make it much clearer there that it should not be removed, at least for backwards-compatibility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I linked to the wrong document. Link has been updated!
Received. Will look at it as soon as possible.
Lois
…On Wed, May 10, 2017 at 11:30 AM, Vladimir Diaz ***@***.***> wrote:
@vladimir-v-diaz <https://github.com/vladimir-v-diaz> requested your
review on: theupdateframework/taps#20
<#20> Add initial draft of
TAP 8.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AN5qX09SKnmcBFNcGBNLU_oLyLS1A4_2ks5r4dgHgaJpZM4NW1jv>
.
|
thanks for putting this up, @vladimir-v-diaz |
@JustinCappos thanks for pinging us via email. My feedback and questions copied below as requested: I like the feature conceptually, just a few comments on the proposed implementation and some clarifications that would be helpful: • Rather than having a new foo.rotate.hash(…) file, could that same data simply be included in the foo.hash(…) file? I assume the idea is the rotate file would be potentially much smaller, do we expect rotations of this form to happen frequently enough for it to really be necessary to have a new file? |
@hannesm, please weigh in if you want to add anything or disagree with
anything I say below...
On Fri, May 12, 2017 at 12:19 PM, David Lawrence ***@***.***> wrote:
@JustinCappos <https://github.com/justincappos> thanks for pinging us via
email. My feedback and questions copied below as requested:
I like the feature conceptually, just a few comments on the proposed
implementation and some clarifications that would be helpful:
• Rather than having a new foo.rotate.hash(…) file, could that same data
simply be included in the foo.hash(…) file? I assume the idea is the rotate
file would be potentially much smaller, do we expect rotations of this form
to happen frequently enough for it to really be necessary to have a new
file?
I think that this is not optimal for a few reasons, but let me focus on the
security aspect. In this case, I believe an attacker with control of a
repo could later still use a 'revoked' key.
For example, suppose alice rotates her key to a new value. A client
updates, but since the client does not use alice's targets metadata, the
client does not download it. An attacker (who has alice's old key)
compromises the repository and then creates a new, later version of alice's
metadata that does not have the rotation. Now when the client returns and
uses alice's metadata, the client trusts the revoked key / metadata.
By having separate .rotate files, clients will always download the rotate
files and know that any later version of that key's metadata must not be
trusted.
• Just for documentation purposes, I disagree with the claim it has no
security implications. This is essentially an extensions of delegations
that allows them to go “sideways” in addition to down. If depth of
delegation has meaning when searching the targets tree, that then has
security implications. N.B. I still think this is a good feature and the
tradeoff is *very* justifiable, it should just be appropriately
documented.
Oh, I see. I've worked on the TAP so that it is clear that we feel there
are security benefits. I do not believe there are negative security
ramifications from the TAP, but would love to discuss more.
• Was there some particular attacker model for which breaking on cycles
provided protection? It seems like a poor use case that when I add, then
eventually remove a key in a delegation, that I or one of the other
remaining members have to also change our keys. I would expect the cycles
to typically be benign (employee joins and leaves, contributor is assigned
a finite “tour of duty”).
This is really a situation where we could not find a good way to handle
this apart from revoking all keys. We discussed quite a few other
proposals, but all seemed to have security and usability issues. Our
thinking is that the tooling (repository, developer tools, etc.) could
detect and warn the user if this is about to happen. To avoid a loop, the
developer can simply rotate their own key first, and then change the
delegation to their new key.
o Also, there needs to be clarification of whether this cycle logic applies
when the canonical delegation record in the parent role has a cycle.
Okay, thanks! I tried to clarify.
• Having a root.rotate.HHH doesn’t seem to make sense. If I’m going to
need a threshold of the previous root keys anyway, I’d just use the
existing, accepted, root rotation mechanism. I don’t think there’s a
security implication here, root rotation is essentially the same “sideways”
transfer of responsibility, but in the spirit of The Tao of Python: “There
should be one-- and preferably only one --obvious way to do it.”
I totally agree with the philosophy, but came to a different conclusion. If
we are adding a rotation mechanism, the question was whether to just use
the root rotation mechanism on every file or to change the root rotation
mechanism to support the same thing we use elsewhere. My thinking is that
there are some fairly ugly things in the root rotation (like needing to
retain / walk all of the versions of the file) that we would not want
elsewhere. This rotation primitive seemed to me like a simpler, more
general way to do it that could apply to all roles...
|
On 13/05/2017 15:09, Justin Cappos wrote:
> • Was there some particular attacker model for which breaking on cycles
> provided protection? It seems like a poor use case that when I add, then
> eventually remove a key in a delegation, that I or one of the other
> remaining members have to also change our keys. I would expect the cycles
> to typically be benign (employee joins and leaves, contributor is assigned
> a finite “tour of duty”).
>
This is really a situation where we could not find a good way to handle
this apart from revoking all keys. We discussed quite a few other
proposals, but all seemed to have security and usability issues. Our
thinking is that the tooling (repository, developer tools, etc.) could
detect and warn the user if this is about to happen. To avoid a loop, the
developer can simply rotate their own key first, and then change the
delegation to their new key.
I'd be happy to have a different solution for the situation where a
project expands its members, and contracts to the same initial project.
As Justin mentioned, we discussed several proposals, one having a
version number in the delegation (which feels a bit awkward), another
would be to include a 'name' in the rollover file (but then, delegations
could only point to the initial project since that name is generated by
a hash of all participating keys). I don't like these solutions, and
think the case is rare enough, and one developer rolling over their key
is a fine solution (esp. since I think practicing rollovers is good for
developers, and they should do this more regularly).
I totally agree with the philosophy, but came to a different conclusion. If
we are adding a rotation mechanism, the question was whether to just use
the root rotation mechanism on every file or to change the root rotation
mechanism to support the same thing we use elsewhere. My thinking is that
there are some fairly ugly things in the root rotation (like needing to
retain / walk all of the versions of the file) that we would not want
elsewhere. This rotation primitive seemed to me like a simpler, more
general way to do it that could apply to all roles...
And the proposed mechanism allows us to mark a repository as invalid (by
producing a cycle in the root rotation) -- which AFAICS can't be done
nowadays!
|
The different root metadata versions build a chain of trust, where if | ||
the initial root is trusted (TOFU or by being distributed over a trusted | ||
channel), following the chain of key rotations leads to verified keys | ||
for the current root metadata. This means that even if the keys are not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify: there's no reason to update the "version" of the root file unless the keys change, which makes root rotation equivalent to the rotate files here.
This TAP essentially adds the rotation mechanism for root keys to all of the other keys, it might be worth nothing that this increases the efficiency concerns mentioned here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIU, if the snapshot or timestamp key(s) change, you need a new root "version", don't you? You only need a rotation in case the root keys change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The requirement is only that the version bump happen when root keys change. It's up to implementations how frequently to version outside of that (for example, etags could easily handle changes to ss/ts keys without bumping the metadata version)
change. If there is a scenario where new keys are generated and | ||
delegated to by a master key this model remains the same as in TUF | ||
today. For example a HSM may generate and sign new timestamp keys every | ||
hour. Since the HSM's key does not change, this is not a rotation and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can someone describe how this model would work with TUF today? Timestamp key rotation requires a signature from the root key and isn't delegation-like.
If this TAP were applied without addressing this, I think there will be some issues with the size of the snapshot file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from what I've seen, you can delegate nowadays the timestamp to a key stored in a HSM, which periodically (e.g. daily) re-signs an online timestamp key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you point to an example where you have seen this? Does it make use of TAPs 4 or 5?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @hannesm meant to say that an HSM signs new timestamp metadata every hour, not that it generates new keys every hour. Generating new a Timestamp key requires the Root role's intervention (at least in current TUF).
may want to check if an uploaded rotate file would lead to a loop and | ||
warn the user. | ||
|
||
The rotate files should be listed in the snapshot metadata and should be |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this blow up fast?
Why can't we distribute this across the files? E.g. snapshot points to the "latest" rotations, and rotations reference their parents?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for timestamp and snapshot, rotations can be removed once keys are re-done (and the root file is updated). I suspect for targets, the rotation files need to be kept in place (IIRC justin had issues with garbage collecting rotate files -- i.e. if nothing delegates to the role anymore, it and all rotations can be safely removed)
|
||
# Security Analysis | ||
|
||
There should be no negative security impact. The major benefits are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There may not be a security "impact" but this is a very different security model than TUF before this TAP.
This changes the key hierarchy into a key bonanza (for lack of a better word :P). I like that this TAP enables you to change the trust relationship to "self-service" roles, but I'm a little concerned that those are the only types of relationships you can have under this TAP.
How can I have a delegation layer that requires sign-off from a higher role in this model? Can we make both of these things possible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
with this tap, projects can modify their "members" by rotation. a higher role can always decide to delegate elsewhere (e.g. to a single key owned by them), which then requires the higher role to approve all changes, if this is intended.
you may ask for another feature, to have (for lack of something better) colored multi-role delegations, and you require an approval from different groups of people. I'm not entirely sure whether this can be modelled with this tap, but it is worth a try.
themselves without an individual with elevated privileges, but based | ||
on a threshold of signatures. | ||
|
||
Clients need to take care to check for 'a rotation loop' where key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that on every update, I have to download the entire history of rotations for all roles? (i.e. what happens if there's a rotation loop back to the first key, and I'm on key 4,006?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but I've heard that caching should be possible
the key. | ||
|
||
Intentionally creating cycles in rotations enables a repository, a | ||
project, and individuals to explicitly revoke their key material |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about just rotating to a "null" role or something more explicit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can create a null
role (if you have sufficient trust that you destroyed the private key after generation), and delegate to this role. in case you don't have such a (centralised) trust, I suspect you have a hard time generating a widely accepted null
role.
delegations to the project being replaced. The ``project key'' | ||
represents a single point of failure / compromise. (Some projects may | ||
prefer a quorum of developers instead, but this would require | ||
delegations to be redone whenever this quorum changed.) Adding and |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does redone mean here?
If a targets file is signed by a set of keys, the people holding those keys only need to re-sign the existing targets file with new keys.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIU, atm if you have a multi-role delegation with a threshold for a project foo
, and the threshold should be modified, the delegation itself needs to be updated -- this is nothing the project foo
can influence
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The threshold and keys are modified in the parent metadata, the signatures only need to be updated in the delegation if the key change in the parent indicates it.
I.e. if a delegation foo
has keys A,B,C
with a threshold of 2, then the parent can change the signature requirements to C,D,E,F
with a threshold of 3 without touching the delegation. In this example the delegation would need to be updated to meet the new requirements, but there's nothing stopping you from updating in lock-step to rotate keys so that the delegation is never invalid.
|
||
# Specification | ||
|
||
To support key rotation, the new metadata file type `rotate` is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a new top-level role? rotate.json? How are the top-level roles refreshed with the introduction of this new role?
# Specification | ||
|
||
To support key rotation, the new metadata file type `rotate` is | ||
introduced, which contains the new public key(s), the threshold, and is |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the exact format of this file? Where are the signatures stored?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
roughly:
signed: { "_type" : "rotate" ; keys: ... ; threshold: .. }
signatures: ...
|
||
Let's consider the project foo, initially delegated to a multi-role | ||
threshold (of 2) to Alice, Bob, and Charlie. Let's assume the targets | ||
file to be foo.hash(ABC2). When they want to add Dan to the project, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Root filenames are named as follows: <version_number>.root.json
Is this convention changing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it will now be root.json
, and root.rotate.<hash>.json
, where <hash>
is a secure hash of the concatenation of the (old) root keys. This file will contain the new public keys, the new threshold, and is signed by the old keys. A client starts with some initial root keys (IR), it computes H=hash(IR+threshold)
and there are two possibilities: either root.rotate.H.json
exists and contains new root keys, NR, or it does not exist. In the latter case, the client uses IR for verification of the signature on root.json
. In the former case, the client computes NH=hash(NR+threshold)
and attempts to download root.rotate.NH.json
, or root.json
is signed by NR directly.
|
||
Let's consider the project foo, initially delegated to a multi-role | ||
threshold (of 2) to Alice, Bob, and Charlie. Let's assume the targets | ||
file to be foo.hash(ABC2). When they want to add Dan to the project, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which hashing algorithm is used?
|
||
Let's consider the project foo, initially delegated to a multi-role | ||
threshold (of 2) to Alice, Bob, and Charlie. Let's assume the targets | ||
file to be foo.hash(ABC2). When they want to add Dan to the project, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What exactly is ABC2? A file? The concatenation of public keys A + B + C + 2 (integer 2)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this and above - I thought multi-role delegations already have this specified -- a secure hash of the concatenation of the public keys, and the threshold value. thus, if there are multiple delegations to the same set of public keys with the same threshold, it will be identical (assuming the public keys have a total order)
under any circumstances. If a client wants to rotate to a different key | ||
(perhaps due to losing the new key), this requires a key revocation. | ||
|
||
TODO: Obviously example file formats, etc. need to be added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My reading of the TAP 8 draft has left me unsure of the proposed changes. Example formats would greatly help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll work on more concrete examples with files this week, as time permits.
|
||
TODO: Obviously example file formats, etc. need to be added. | ||
|
||
# Security Analysis |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this section needs to analyse what happens when a threshold, and less than a threshold, of keys are compromised. The current specification explains how the Root role revokes keys that have been compromised, so how does the new scheme handle this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same, we don't remove the ability of the root keys (or any other higher keys) to revoke keys and modify delegations.
Another thought. Say we have a our |
thanks for your comments; highly appreciated. I try to address them accurately, as far as I can. |
I may have misspoken / misremembered when talking to @hannesm about this.
We had discussed the potential for delegation from the top level timestamp
and release roles at some point in the past, but didn't include it.
…On Mon, May 15, 2017 at 4:40 PM, Vladimir Diaz ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In tap8.md
<#20 (comment)>
:
> +
+Roles which typically have short lived keys, like the timestamp role,
+may wish to revoke trust in the prior key and sign a new key with the
+old. This limits the ability for attackers to walk back the set of
+trusted keys. Right now, there is not a good way to do this within TUF,
+which may result in some keys being trusted more than they should be.
+
+
+Using TAP 8, the online key can rotate itself without a signature by the
+offline key.
+
+Note, this TAP is not meant to address all situations where keys will
+change. If there is a scenario where new keys are generated and
+delegated to by a master key this model remains the same as in TUF
+today. For example a HSM may generate and sign new timestamp keys every
+hour. Since the HSM's key does not change, this is not a rotation and
I think @hannesm <https://github.com/hannesm> meant to say that an HSM
signs new timestamp metadata every hour, not that it generates new keys
every hour. Generating new a Timestamp key requires the Root role's
intervention (at least in current TUF).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#20 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA0XD_S1Il_RuO-CYvT9DqKx_OnexEV-ks5r6LhNgaJpZM4NW1jv>
.
|
TAPs that have been assigned a TAP number will now be merged into the "master" branch. This does not mean that the merged TAP has been accepted, only that the proposed change (with potential tweaks) is likely to be incorporated into TUF. We'll likely continue to comment on the pull request after it's been closed, so that we can easily comment on specific lines of the TAP. @hannesm Thanks for submitting TAP 8! |
On Fri, May 19, 2017 at 11:24 AM, Vladimir Diaz ***@***.***> wrote:
TAPs that have been assigned a TAP number will now be merged into the
"master" branch. This does not mean that the merged TAP has been accepted,
only that the proposed change (with potential tweaks) is likely to be
incorporated into TUF.
I would say instead that merging a TAP means it is worth having a permanent
/ fixed TAP number to point to this discussion. Moving from 'draft' to
'accepted' status is what means it is likely to be merged with tweaks.
|
@hannesm: I wanted to revive work on this so that we can get these changes in. What are your thoughts about David's comments? I'd be happy to discuss to figure out the best way to move this forward... |
TAP 8 has been contributed by Hannes Mehnert. Vlad has formatted Hannes' original submission to include missing headings and a TAP header.