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

bug: python-tuf incompatibility because of hex-encoded ECDSA keys #329

Closed
asraa opened this issue Jul 21, 2022 · 12 comments · Fixed by #380
Closed

bug: python-tuf incompatibility because of hex-encoded ECDSA keys #329

asraa opened this issue Jul 21, 2022 · 12 comments · Fixed by #380
Labels
bug Something isn't working
Milestone

Comments

@asraa
Copy link
Contributor

asraa commented Jul 21, 2022

Description

Right now, python-tuf can't validate our root metadata because go-tuf used hex-encoded ecdsa keys, rather than what securesystemslib requires (PEM encoded keys). This is actually a hard problem (see go-tuf issue links) because of compatibility: if we change keys to be PEM-encoded keys, then the key material changes, preventing verification through a valid chain of roots.

Options to fix:

  1. As we add new keyholders, add them with PEM-encoded keys. Over the course of 5 more root rotations, now we are compatible. This takes too long.
  2. Fix this in the next root signing and mandate that everyone uses this root as an initial trusted root. Scary, because old roots will not chain up to the new root.
  3. Change the keymaterial without changing the keyid. Looking at the implementations, this will LIKELY work, since keyIDs do not need to be based off hashes of the key material anymore, and because there is no verification done that keymaterial matches: just key IDs do. That seems like a bug, but we can exploit it. If we do this, then roots will continue to chain with go-tuf, and we will achieve python-tuf verification in the next root-signing.
  1. Somehow augment python-tuf for this verification. But this would create a loophole that people may have a hard time moving away from.

Blocks #242

See theupdateframework/go-tuf#223, theupdateframework/go-tuf#270

@trishankatdatadog @mnm678 @joshuagl @kommendorkapten

@asraa asraa added the bug Something isn't working label Jul 21, 2022
@asraa asraa added this to the v5 milestone Jul 21, 2022
@asraa
Copy link
Contributor Author

asraa commented Jul 21, 2022

See #103 (comment)

@jku

@asraa asraa changed the title Make TUF root compatible with python-tuf bug: python-tuf incompatibility because of hex-encoded ECDSA keys Jul 21, 2022
@mnm678
Copy link
Contributor

mnm678 commented Jul 21, 2022

2. Fix this in the next root signing and mandate that everyone uses this root as an initial trusted root. Scary, because old roots will not chain up to the new root.

Can we do this, but also sign with the old keys to allow for a seamless transition? So existing keys a, b, c and PEM encoded keys from the same people A, B, C all sign the metadata that lists only A, B, C as the new trusted root keyholders.

@haydentherapper
Copy link
Contributor

Question: What about other TUF implementations? Has everyone but python-tuf added support for hex-encoded keys?

@asraa
Copy link
Contributor Author

asraa commented Jul 21, 2022

Can we do this, but also sign with the old keys to allow for a seamless transition?

I see, dupe the old keys with hex-encoded and add their PEM-encoded files as well, so that we will have ~10 sigs in the root metadata.

That's a great idea!

@asraa
Copy link
Contributor Author

asraa commented Jul 21, 2022

Question: What about other TUF implementations? Has everyone but python-tuf added support for hex-encoded keys?

PEM-encoded keys are the specification, see links!

@haydentherapper
Copy link
Contributor

I was curious if other implementations are broken too besides python-tuf. Otherwise, option (4) seems like a possibility, even if it's deviating from the spec.

@asraa
Copy link
Contributor Author

asraa commented Jul 21, 2022

I was curious if other implementations are broken too besides python-tuf.

It's just go-tuf. Rust's tough implemented a work-around for go-tuf, but it was fundamentally just go-tuf's lack of compatibility with the ecosystem.

For context, in the linked issue theupdateframework/go-tuf#223, it was because go-tuf couldn't canonicalize json with newlines. that's now fixed in go-tuf so this doesn't block go-tuf anymore, we now write json to the store, not canonicalized json.

Here's the rust tough discussion on accomodating our keys: awslabs/tough#426.

@woodruffw
Copy link
Member

Sorry for butting in, but just to make sure I understand: this is a failure with python-tuf, but technically an error on go-tuf's side, correct? As in, go-tuf uses hex-encoding for its key material instead of PEM, which violates the TUF spec?

Can we do this, but also sign with the old keys to allow for a seamless transition?

I see, dupe the old keys with hex-encoded and add their PEM-encoded files as well, so that we will have ~10 sigs in the root metadata.

That's a great idea!

This makes sense to me too. This also enables an eventual graceful transition to just PEM-encoded keys, right?

@asraa
Copy link
Contributor Author

asraa commented Jul 21, 2022

As in, go-tuf uses hex-encoding for its key material instead of PEM, which violates the TUF spec?

Yes, exactly correct! So for python sigstore integrations, they would then be able to pin their trusted root at the next upcoming root version v5, and continue verification onwards for future updates, all keys will be PEM-encoded from then on.

@joshuagl
Copy link
Member

2. Fix this in the next root signing and mandate that everyone uses this root as an initial trusted root. Scary, because old roots will not chain up to the new root.

Can we do this, but also sign with the old keys to allow for a seamless transition? So existing keys a, b, c and PEM encoded keys from the same people A, B, C all sign the metadata that lists only A, B, C as the new trusted root keyholders.

I like this idea.

@asraa
Copy link
Contributor Author

asraa commented Sep 12, 2022

The plan for the PRs (started #372) will be that I will create four separate PRs with the following changes and testing:

  1. Bump go-tuf and no change in functionality. Hex-only ecdsa keys.
    • Testing: Verify that hex-encoded keys were used, and that repos verify.
  2. Update HSM key parsing logic to allow for parsing into hex and parsing into PEM.
    • Testing: Verify that HSM key to hex AND PEM successfully parses with go-tuf.
  3. Update init logic: Update init to take a parameter on hex/pem parsing: HSM and online keys to be written to TUF metadata as PEM.
    • Testing: Verify that we add HSM PEM keys and revoke the hex keys.
    • Testing: Verify that key reference keys are written as PEM.
  4. Update signing logic: HSM keys result in two types of signers, PEM and hex. Online keys result in PEM signers
    • Testing: Verify that Sign CMD on test HSM keys result in two signatures, one for each key id
    • Testing: Verify that we can successfully sign a complete metadata.

@trishankatdatadog
Copy link
Contributor

I wish there was a way to say in TUF: "Count only 1 towards the threshold out of these N same public keys (even if they look or are different)."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants