theupdateframework / specification

The Update Framework specification
https://theupdateframework.github.io/specification/
Other
368 stars 54 forks source link

Clarify key format #201

Open flavio opened 2 years ago

flavio commented 2 years ago

Preamble

I'm filing this issue as suggested by @trishankkarthik inside of this conversation. The comments relevant to this issue are this one and this one.

The issue

The TUF specification states that ecdsa-sha2-nistp256 keys should be PEM encoded (see here):

The "ecdsa-sha2-nistp256" format is:

{
  "keytype" : "ecdsa-sha2-nistp256",
  "scheme" : "ecdsa-sha2-nistp256",
  "keyval" : {
    "public" : PUBLIC
  }
}

PUBLIC PEM format and a string.

However, the specification also states:

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.

The question is: are ecdsa-sha2-nistp256 expected to be encoded only with PEM, or can they be encoded with another format (like hex)?

Personally I agree with what @webern wrote on the linked issue, ecdsa-sha2-nistp256 should be encoded only with PEM.

webern commented 2 years ago

ecdsa-sha2-nistp256 should be encoded only with PEM.

For the sake of interoperability, it seems better to have the "defined" key types' schemas unchangeable. I would consider updating the wording to indicate that new keytypes can be created, but that the keytypes defined in the spec should not be altered or used differently than specified.

trishankatdatadog commented 2 years ago

For the sake of interoperability, it seems better to have the "defined" key types' schemas unchangeable. I would consider updating the wording to indicate that new keytypes can be created, but that the keytypes defined in the spec should not be altered or used differently than specified.

For the sake of interoperability, I agree, but it would break existing implementations right now and render them spec-uncompliant. I would reserve any such backwards-incompatible proposal for a future MAJOR version of the spec (following the SemVer convention).

For the moment, I would recommend reaching out to coordinate between the go-tuf and rust-tuf/tough implementations.

jku commented 2 years ago

it would break existing implementations right now and render them spec-uncompliant.

After looking at some example data, I don't quite get this: the existing implementations are non-compliant right now, or do I misunderstand something?

go-tuf (or at least the sigstore metadata) seems to include hex encoded bytes in ecdsa-sha2-nistp256 keyval.public. Specification clear defines ecdsa-sha2-nistp256 keyval.public as a PEM string.

Why would the spec define keytypes if everyone can then redefine the same keytypes to be something different? I don't think it's useful to bend the definition of "compliant" just to keep go-tuf "compliant": I would much rather see documentation that clearly states A) what is not compliant and B) whether there is a plan to be compliant in future

lukpueh commented 2 years ago

FYI https://github.com/secure-systems-lab/securesystemslib/issues/308 provides an overview of all public key formats available for python-tuf (and python-in-toto) via the in-house crypto library securesystemslib. It also identifies some problems and ideas for consolidation.