keyids_hash_algorithms missing in private keys?

[awwad]

Per Vlad, both public and private keys generated / imported by TUF should include a 'keyid_hash_algorithms' element (which according to tuf.formats.ANYKEY_SCHEMA is optional).

However, this was executed on code current on the develop branch as of this commenting:

>>> rt.generate_and_write_ed25519_keypair('testkey', 'pw')

>>> rt.import_ed25519_privatekey_from_file('testkey', 'pw')

{'keyval': {'public': '7c13e8c6f7842a892d36c2bee6fdf6327dcafa94466389c1dd83422b99871dc6', 'private': 'e2f7828fca73f68d4c0e14f2a06cee40cf1b7950293490cf71ff16cbac76e030'}, 'keytype': 'ed25519', 'keyid': '9a417cf2dc6771fc4b62eb8a50136e3f2bf1b9fc5d51dac33c08b0cb7af6b01c'}

>>> rt.import_ed25519_publickey_from_file('testkey.pub')

{'keyval': {'public': '7e6ff8cb56d7c1bda03524eb728c14577e143c839b08e720c56e69d6259c1a2e'}, 'keytype': 'ed25519', 'keyid': 'c78e1cf42f659af39706347e242f59e4c7e9c6c1d5cf0e2fd3e7056995ca10d3', 'keyid_hash_algorithms': ['sha256', 'sha512']}

The imported private key does not include keyid_hash_algorithms.

[lukpueh]

Should we just adapt import_ed25519_privatekey_from_file to add keyid_hash_algorithms?

Or do we additionally want to make the property mandatory for some or all of ANYKEY_SCHEMA, RSAKEY_SCHEMA, ECDSAKEY_SCHEMA?

[vladimir-v-diaz]

I'd recommend that keyid_hash_algorithms be added by the import functions: import_ecdsa_privatekey_from_file, import_ed25519_privatekey_from_file, etc.

[lukpueh]

Furthermore, I don't see how including the hash algorithms portion in the key object returned by above import_* functions is useful, especially if we only return the key object with a default keyid, and not the other keyids, hashed with other algorithms. Can you elaborate on above "Per Vlad, ... should ..." ?

[vladimir-v-diaz]

Here's some background information on keyid_hash_algorithms: #272

Let me know if it clears things up.

[lukpueh]

import_ed25519_publickey_from_file already has wished behavior, see line 1155 but discards the keyids (second return value) as junk.

ed25519_key, junk = securesystemslib.keys.format_metadata_to_key(ed25519_key_metadata)

Providing the same behavior for import_ed25519_privatekey_from_file is as simple as adding above line to the function.

I wonder if we want to (in addition to having the key_id_hash_algorithms in the key object):

• return the keyids (junk) from the import_ed25519_* functions,
• do the same for import_rsa_* functions,
• adapt *KEY_SCHEMAs in securesystemslib.formats` to make keyid hash algorithms mandatory?

[vladimir-v-diaz]

I prefer not to make keyids_hash_algorithms mandatory at the moment, so we don't cause issues for integrations that don't expect it. Perhaps we can reconsider once we make a major breaking change to the format of key files / objects.

[lukpueh]

Hm. I just read your #272 (comment).

It does not fully make sense to me, that these functions specify a keyid using a default algorithm, e.g. sha256 but pretend to provide flexibility, by adding multiple possible hashing algorithms.

Given a key

{
  "keyid": HASH256(KEYVAL),
  "keyid_hash_algorithms": ['sha256', 'sha512'],
  "keyval" : KEYVAL
}

The keyid will always require HASH256, no matter what is specified in the keyid_hash_algorithms field. Doesn't it make more sense to explicitly state which algorithm was used?

[lukpueh]

Re format: Okay, makes sense.

[lukpueh]

Okay, it does make sense now. It is required to associate a key with signatures signed by that key, but potentially having a different key id, due to a different hash algorithm. Thanks for the clarification, Vlad!

I'll update import_ed25519_*, import_rsa_* and import_ecdsa_*

[lukpueh]

Btw. we have partially redundant code in the tuf's repository lib (1) and securesystemslib's key module (2).

(1)
import_rsa_publickey_from_file(filepath)
import_rsa_privatekey_from_file(filepath)
import_ed25519_publickey_from_file(filepath)
import_ed25519_privatekey_from_file(filepath)
(2)
import_rsakey_from_pem(pem)
import_rsakey_from_public_pem(pem)
import_rsakey_from_private_pem(pem)
import_ecdsakey_from_pem(pem)
import_ecdsakey_from_public_pem(pem)
import_ecdsakey_from_private_pem(pem)

Should we merge?

[vladimir-v-diaz]

Yup, I added an interface to securesystemslib that does a lot of what repository_lib.py currently does. We should consider using the functions provided by securesystemslib.

[lukpueh]

Actually the securesystemslib functions load PEM formatted keys, whereas the repository lib functions load json.
I guess, we should provide a load PEM and a load JSON function, for each of (public and private) key, public key, private key, for each of rsa, ed25519 and ecdsa, in securesystemslib.

[lukpueh]

Vlad, do you want to keep the interfaces in repository_lib? e.g.:

def import_ed25519_publickey_from_file(filepath):
  securesystemslib.import_ed25519_publickey_from_file(filepath)

[vladimir-v-diaz]

Keeping the interface in repository_lib makes the most sense. I'd imagine that the format of JSON keys would differ between projects, so adding it to securesystemslib is not ideal.

[lukpueh]

I don't agree. The JSON/dictionary schemas are integral part of securesystemslib, c.f. securesystemslib.formats. We use the same schemas in in-toto. So I guess it makes sense to move/add the implementation to securesystemslib.keys.

[lukpueh]

But yes, we should still keep the interface in repository_lib for backwards compatibility.

[lukpueh]

I created a new issue in securesystemslib:
secure-systems-lab/securesystemslib#35