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

compatibility: go-tuf ecdsa key format is incompatible #1859

Closed
jku opened this issue Feb 12, 2022 · 7 comments
Closed

compatibility: go-tuf ecdsa key format is incompatible #1859

jku opened this issue Feb 12, 2022 · 7 comments

Comments

@jku
Copy link
Member

jku commented Feb 12, 2022

Spec defines ecdsa public keys as PEM strings: python-tuf implements them as PEM strings (with the "BEGIN PUBLIC KEY" preamble).

go-tuf implements ecdsa public keys as hex encoded bytes.

Here is awslabs working around the issue awslabs/tough#426: they now apparently support both PEM and hex encoded DER.

There is a spec issue about clarifying key format theupdateframework/specification#201 but I don't see much needing clarification: understandably people would like go-tuf to be compliant but it does not seem to be -- it's not the end of the world.

We will have to decide if we want to support this (hex encoded bytes in ecdsa keys): it's non-compliant but it's being used.

@jku
Copy link
Member Author

jku commented Feb 14, 2022

I'm not a fan of adding code complexity to python-tuf or sslib to accept multiple formats but it might be the only way to be interoperable in a reasonable time frame: go-tuf would need to be able to update all clients to understand PEM before they can start generating metadata where keys are PEM... or create some scheme where repositories can publish both existing broken keys and new good keys

@jku jku changed the title compatibility: key formats are incompatible compatibility: go-tuf ecdsa key format is incompatible Feb 14, 2022
@di
Copy link

di commented Apr 8, 2022

@asraa or @haydentherapper, any thoughts here?

@haydentherapper
Copy link

Supporting both should be easy. I greatly prefer PEM encoded keys as the spec already defines, but if DER has been supported currently, I don’t think we should drop it.

Should the spec be flexible in its format? Possibly provide at minimum PEM support, but optionally DER?

@mnm678
Copy link
Contributor

mnm678 commented Apr 8, 2022

The spec says that "We define three keytypes below: "rsa", "ed25519", and "ecdsa-sha2-nistp256", but adopters can define and use any particular keytype, signing scheme, and cryptographic library." So the spec leaves it open for adopters to use other keytypes. This does imply that other keytypes would be differently named (ie ecdsa-der), but for the sake of compatibility I agree that the best option at this point seems to be supporting both in python-tuf.

In the long term, this might be the kind of issue solved by TAP 14.

@jku
Copy link
Member Author

jku commented Apr 11, 2022

Should the spec be flexible in its format? Possibly provide at minimum PEM support, but optionally DER?

I don't think you were suggesting modifying the existing keytype but I will say it just to be sure:

  • spec should not approve of the current go-tuf ecdsa format: it seems to be a bug
  • adding new actual keytypes is easy as marine points out: defining a new DER ecdsa format is trivial (I think?), it just should not use the same keytype name as an existing keytype
  • if someone defines a new keytype/scheme and implementations are willing to support it, we could even mention the new keytype name in the spec (to avoid name clashes)

whether any of the above happens, python-tuf could support the buggy go-tuf keys as a workaround. I would support this if reasonable effort was first put into actually fixing the problem in go-tuf side (but the research result ends up being that existing repositories cannot be reasonably migrated to new compliant key implementation).

@asraa
Copy link

asraa commented Apr 11, 2022

spec should not approve of the current go-tuf ecdsa format: it seems to be a bug

I agree, it seems it was just legacy code. On the other hand, how do we deal with old roots who were using this? Will chains still verify if early root version describe keys with the hex encoded bytes? Can we support but issue a warning? Then remove any methods creating new keys with this encoding.

Supporting both at the same time is a little tricky, if we're doing a key ID match, we might need to create two key IDs, one based on the DER and one based on the PEM encoding. I guess it may be good that go-tuf uses a list of keyIDs for matching.

Possibly provide at minimum PEM support, but optionally DER?

Maybe this is the best way to resolve the above.

@jku
Copy link
Member Author

jku commented Oct 31, 2022

🙏 current go-tuf metadata works with python-tuf-ngclient ( see e.g. sigstore, boostrap from 5.root.json only). Thanks Asra and others who worked on this!

Now would be a good time to start working on a compliance test suite...

@jku jku closed this as completed Oct 31, 2022
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

No branches or pull requests

5 participants