-
Notifications
You must be signed in to change notification settings - Fork 2
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
Updates to Key Compromise Analysis #43
Updates to Key Compromise Analysis #43
Conversation
…all stored together
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 is a lot better. I left a few minor suggestions inline. Feel free to use them as you see fit.
pep-0458.txt
Outdated
| **AND** | limited by | limited by | limited by earliest root, | | ||
| snapshot | earliest root, | earliest root, | targets, or bins metadata | | ||
| **AND** | targets, or bins | targets, or | expiry time | | ||
| bin-n | expiry time | bins metadata | | | ||
| | | expiry time | | | ||
+-----------------+-------------------+----------------+--------------------------------+ | ||
| root | YES | YES | YES | |
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.
Just a suggestions: Would collapsing the columns make the table easier to read?
+-----------------+-------------------+----------------+--------------------------------+
| Role Compromise | Malicious Updates | Freeze Attack | Metadata Inconsistency Attacks |
+=================+===================+================+================================+
| targets | NO |
| **OR** | timestamp and snapshot need to cooperate for any of the attacks |
| bins | |
+-----------------+-------------------+----------------+--------------------------------+
| timestamp | YES |
| **AND** | but all of the attacks are limited by earliest root, targets or |
| snapshot | bins metadata expiry time |
| **AND** | |
| bin-n | |
+-----------------+-------------------+----------------+--------------------------------+
| root | YES |
+-----------------+-------------------+----------------+--------------------------------+
If not, I advise to use the same wording in every box, if the condition is the same, i.e.:
- "[roles] need to cooperate" vs. "need [roles]" (in row 1)
- "[... roles] expiry time" vs. "[... roles] metadata expiry" (in row 2)
pep-0458.txt
Outdated
|
||
Finally, a compromise of the PyPI infrastructure MAY introduce malicious | ||
updates to *bins* projects because the keys for these roles are online. The | ||
updates to *bin-n* projects because the keys for these roles are online. The |
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.
It feels like the leading two sentences in this paragraph pretty much repeat what has been said at the end of the previous paragraph. I suggest to remove them.
To bridge from online key compromise to PEP 480. I suggest to rephrase the remainder of this paragraph to something like:
The maximum security model shows how TUF mitigates online key compromises by introducing additional roles for end-to-signing. Details about how to generate developer keys and singing upload distributions are provided in PEP 480 [26]_.
Co-Authored-By: lukpueh <[email protected]>
@JustinCappos I made some changes based on @lukpueh's feedback. |
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.
LGTM
Fixes #28
Update the analysis and table to be clear what the security properties are in the case that the online keys are the same or stored in the same location. A version of the more detailed table is available in PEP 480.