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

Metadata file contains a key with an invalid keytype. #1388

Closed
asraa opened this issue May 13, 2021 · 5 comments · Fixed by #1453
Closed

Metadata file contains a key with an invalid keytype. #1388

asraa opened this issue May 13, 2021 · 5 comments · Fixed by #1453
Assignees
Labels
bug good first issue Bite-sized items for first time contributors
Milestone

Comments

@asraa
Copy link

asraa commented May 13, 2021

ECDSA keys are generated with a keytype of ecdsa using the securesystemslib https://github.com/secure-systems-lab/securesystemslib/blob/2d33d91bef92f53b5e9489d57033bb9d046a6d23/securesystemslib/keys.py#L1678.

However, if you load a repository that's created with a key from import_ecdsakey_from_public_pem, you run a TUF error that the keytype is unsupported because of:

https://github.com/theupdateframework/tuf/blob/30ba6e9f9ab25e0370e29ce574dada2d8809afa0/tuf/keydb.py#L48

Should the keytype instead be ecdsa, rather than ecdsa-sha2-nistp256, which is the scheme type?

@joshuagl
Copy link
Member

Thanks for the bug report @asraa ! Until relatively recently securesystemslib used ecdsa-sha2-nistp256 as the key type, but that was clearly incorrect (secure-systems-lab/securesystemslib#239) as it encoded scheme and we changed it to be more consistent with the other key types in secure-systems-lab/securesystemslib#267

I think the correct fix for this is to add the 'new' ecdsa key type to the list of supported key types. Would you be willing to submit a PR with a change to _SUPPORTED_KEY_TYPES = ['rsa', 'ed25519', 'ecdsa', 'ecdsa-sha2-nistp256']?

@joshuagl joshuagl added bug good first issue Bite-sized items for first time contributors labels May 14, 2021
@asraa
Copy link
Author

asraa commented May 14, 2021

Ohhh I see. Thanks for the context -- I agree that that fix makes the most sense.

@trishankatdatadog
Copy link
Member

We should deprecate ecdsa-sha2-nistp256 keytype now, and remove it altogether later for 2.0 or something.

@sechkova sechkova added this to the weeks24-25 milestone Jun 9, 2021
@avelichka
Copy link
Contributor

I'd like to give that a try, please?

@asraa
Copy link
Author

asraa commented Jun 9, 2021

go ahead! i moved to using the go-tuf implementation and had a workaround

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug good first issue Bite-sized items for first time contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants