-
Notifications
You must be signed in to change notification settings - Fork 11
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
Fix noncompliant keyids #338
Conversation
This modifies keyids in place: The key contents remain the same * Unfortunately keys get shuffled around since they are sorted by keyid * The result should be that same HW key will now sign the "new" keyids * for root, the HW key will sign for both new keyids and old keyid (so that both root N and N+1 will reach threshold) this command can be run on "root" and "targets" (and will fix all keyids defined in that roles metadata). New versions of the delegated roles will then be created to make sure they get signed with new keyids.
This makes it possible to sign the same metadata twice: * currently this is only useful when fixing the keyid compliance issue in root (see theupdateframework#292). Basically the user will be asked to sign with both the keyid from root N+1 and the keyid from root N. * there are clear use cases with one signer with multiple keys in future (think e.g. key rotation).
I'm still going to do a bit more manual testing but I think this will work |
Code looks reasonable! Waiting for more test results. |
Test in jku/tuf-demo#107:
|
awslabs/tough client does now "succeed" in that root is considered valid but
This one I'm not as inclined to "fix" in tuf-on-ci. I don't know if my decision to avoid lengths and hashes in timestamp and snapshot is the best choice but both options seem to have upsides and downsides and both are spec compliant |
@kommendorkapten I'm done I think: As far as I can tell the code is good. and the results metadata update is solid. awslabs/tough tuftool still doesn't fully work, but the specific issue of noncompliant keyids seems to gets solved by this code |
Hmm, actually tuf-on-ci-repo-import could now also modify the keyids immediately when the custom metadata is added... I'll try to add this in this PR. |
This comment was marked as outdated.
This comment was marked as outdated.
There is a complication in the root case:
|
Since the import adds custom metadata into existing keys, keyids become non-compliant. Run force_compliant_keyids() for imported roles too. This very annoyingly requires a special case in _sign(): basically a heuristic that figures that we want to sign with "previous version" root keys if the keyid of this legacy key can be calculated from "current version" root key by just removing the custom metadata.
650fad9
to
5f3d3fa
Compare
Example import signing event jku/root-signing-test#16
|
Maybe a hack but what if tuf-on-ci creates an in-memory representation of the yubikey using the "old format", i.e without |
We discussed this live but documenting for others: this is pretty much what the current PR does. For each signer it takes the "tuf-on-ci managed key", calculates keyid without the custom metadata, and looks for keys with that keyid in the non-managed (N-1) metadata. These keys will then also be used to sign. |
Fixes #336 , relates to #292
This change will ensure that a a keyid fix can be done in a single signing event
--force-compliant-keyids
flag to delegate: this will update all keyids in the role and will mark all affected delegations to be resignedThis approach (just switching the all keyids) looks surprising but should be 100% spec compliant. Example:
tuf-on-ci-delegate --force-compliant-keyids sign/fix-keyids root
is used: the signing event now contains a root v2 with one root signer with key A using valid keyid efgh (same key, different keyid)tuf-on-ci-sign sign/fix-keyids
as usual: this signs twice with A; The metadata signatures now contain a sig from abcd and a sig from efgh