Split formating of a ed25519 dict from key generation.

[KOLANICH]

Description of the changes being introduced by the pull request:

This PR just splits a securesystemslib key dict generation from key generation for ed25519 keys into a separate function. This is needed to allow code reuse when importing ed25519 keys.

Please verify and check that the pull request fulfils the following requirements:

• The code follows the Code Style Guidelines
• Tests have been added for the bug fix or new feature - not needed, tested through testing higher-level functions
• Docs have been added for the bug fix or new feature - not needed, we use it internally for now

[jku]

So the core design idea here is that in future all keys should support importing from existing key material? or maybe just some keys where it's more likely people would want to re-use keys?

does format_ed25519_dict() need to be public? the secureystemslib public API is already ridiculously large, I would rather it become smaller than bigger.

Is it important that new method is in keys, would ed25519_keys be a more logical location?

[KOLANICH]

in future all keys should support importing from existing key material?

As much of widespread ones (not the custom ones used in the software used by 1 person only, but the ones used in well-known tools like openssl, openssh, minisign, signify, OpenPGP (already here) and so on) as contributors had taken effort to implement importing. Not being able to use existing keys is a major issue. In real world different places that can be used as roots of trust use different key serializations. For example GitHub allows to bind the SSH keys for signing to accounts, so it can be used as a root of trust, I can imagine an app fetching a SSH public key from a GH account and using it to verify the snapshot and timestamp roles in TUF for the packages originating from that GitHub account.

The next milestone should be supporting formats of signatures used by other libs and tools.

Also I think that securesystemlib should eliminate own key serialization formats, we already have too many of them used by other software that we'll (and other impls) have to support, there is no need to making the compatibility situation worse.

does format_ed25519_dict() need to be public? the secureystemslib public API is already ridiculously large, I would rather it become smaller than bigger.

I think it should be public within securesystemslib (not limited to the moduke it is in), also available to everyone wanting to use it from other packages (I hate private and protected keywords of C++ which often causes people to maintain own forks of libs just to make one field public, fortunately there are ways to circumvent these restrictions, but most of devs don't know about them. I prefer we all are consenting adults approach of Python), but without any guarantees of stability. If people use this API, it should be only a stopgap measure to allow them to develop their software without the burden of maintaining an own fork, in long term the code using the unstable API should be landed into mainline securesystemslib.

Is it important that new method is in keys, would ed25519_keys be a more logical location?

No, it's not important, in fact I'm surprised that there are any methods dealing with certain key types within keys. I think that keys should contain only the functionality working on all the key types uniformly, wrapping the funcs working on specific key types, and those type-specific functions should not be considered stable API at all.

[jku]

I'm surprised that there are any methods dealing with certain key types within keys.

I think the way that happens is that developers add those methods there, just like you are doing now :)

My question in other words is why can't this function be a private one in ed25519_keys, and ed25519_keys.generate_public_and_private() calls it? This way you would get the reusability inside ed25519_keys but the public API does not grow.

[KOLANICH]

My question in other words is why can't this function be a private one in ed25519_keys, and ed25519_keys.generate_public_and_private() calls it? This way you would get the reusability inside ed25519_keys but the public API does not grow.

Well, it can, but it will require large enough refactoring of the lib (_get_keyid is defined in keys and probably should be moved into an own module to avoid a cycle of imports) to make reaching consensus about how it should look like last ethernity. Such refactorings are better to be done by repo maintainers, who won't say to oneself "persuade me it is needed", "I don't like the way it is done", but will just do the things the way that will be OK for them to land into master immediately. So I propose to land this PR first, and then do the needed restructuring of the modules (by the people who have authority in this project to do such things).

[lukpueh]

This PR seems to be mainly motivated by #451. I suggest to defer merging until we have a clearer vision about the key import API. See #451 (comment).

Regarding your statement above that maintainers "just do the things the way that will be OK for them to land into master immediately.", I'd like to point out that maintainers need to go through the same process as any other contributors, which includes describing the reason for the patch and their design decisions, as well as a review by another maintainer.

[KOLANICH]

This PR seems to be mainly motivated by #451.

It is.

I'd like to point out that maintainers need to go through the same process as any other contributors, which includes describing the reason for the patch and their design decisions, as well as a review by another maintainer.

I know. But at least the maintainer, who has created an own PR, won't block its landing, because the fact of him sending the PR means he is likely OK with it.

I suggest to defer mergin

OK.

[KOLANICH]

Worked around by carrying qn impl in https://github.com/KOLANICH/securesystemslib_KOLANICH.py

[lukpueh]

This patch is also part of #451. Let's discuss how to proceed with SSH keys there: #451 (comment)