Reconsider keyid "compliance"

[jku]

Metadata API currently does not validate that keyid is a hex digest of the keys canonical form, we also do not test for this.

I maintain that this is fine from a security perspective: being able to calculate keyid from key value brings us no value that I can see, just more complicated code. See also https://github.com/theupdateframework/taps/blob/master/tap12.md

However, as shown in theupdateframework/go-tuf#228 go-tuf client does seem to check that keyids are as specified and arguably they are spec compliant in doing so. So let's reconsider: should we try to ensure that this creates keys with keyids that fulfill the requirements:

sslib_key = generate_ed25519_key()
key = Key.from_securesystemslib_key(sslib_key)

I assume that last call removes some unused fields from the key structure to make the metadata cleaner... and that probably makes the key not hash correctly.

[mnm678]

Rather than reverting this change in python-tuf, we should aim to get this change into the spec. I don't think it's worth adding some extra complexity here that we'll just remove very soon.

[jku]

I agree on avoiding complexity: I think we should still investigate if we are accidentally doing something that triggers this issue.

[jku]

I've had a first look to remind myself how the keyid is calculated:

• spec hand waves the definition with hexdigest of the SHA-256 hash of the canonical form of the key. So what is the canonical form of a key?
• My impression of the process that securesystemslib uses to get canonical key form is here:
  • First, construct this json object:

    {'keytype': KEYTYPE,
     'scheme': SCHEME,
     'keyval': {'public': PUBLIC, 'private': ''}
    }
    

  • Then run that through JSON canonicalization process.

note that the used JSON object is not the key from the metadata file (as an example it has a "private" field with an empty string for an unexplained purpose). This does not feel like a process that was designed for interoperability -- for a start it does not really seem to be documented

[lukpueh]

For reference: secure-systems-lab/securesystemslib#308 summarizes securesystemslib/TUF/in-toto public key formats and points out issues/inconsistencies, such as the ambiguously defined "private" field you mention. It also suggests to "better decouple default keyids from keys, to allow the use of custom keyids, but with a canonical json computed keyid as fallback"

[jku]

Right, so I suppose the result is then:

• there's no standard definitition for canonical form of key. The form securesystemslib uses is a bit weird but there's likely not much point in tweaking that
• consensus is that keyids should be freeform (generating a "unique" hash id by default is fine but keyids should still be 100% in control of whoever inserts them into delegating metadata): this is how metadata API already operates
• My original assumption that we did something differently in Metadata API does not seem correct: the securesystemslib method of keyid generation is described above and has been used for a long time
• We are not planning to change keyid handling in Metadata API: if a user wants to use their definition of "canonical form of key" to calculate a new keyid and use that instead of the default they are free to do so

Closing as WONTFIX