Better distinguish between keytype and scheme for ECDSA keys #267
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Joshua Lock <[email protected]>
Prior to this change ECDSA keys had a keytype that include their scheme. Change this to simply 'ecdsa' for all ECDSA keys, regardless of scheme, to better match other key types. Continue to support reading keys of the old keytype+scheme format. Signed-off-by: Joshua Lock <[email protected]>
|
cc @adityasaky |
|
cc @shibumi |
shibumi
reviewed
Aug 19, 2020
| # in order to support key files generated with older versions of | ||
| # securesystemslib. At some point this backwards compatibility should be | ||
| # removed. | ||
| if key_object['keytype'] not in['ecdsa', 'ecdsa-sha2-nistp256', |
There was a problem hiding this comment.
Should this be in in-toto-golang, too?
CC: @lukpueh
There was a problem hiding this comment.
I don't think that's necessary. Just wanted to loop you in.
There was a problem hiding this comment.
I don't think that's necessary. Just wanted to loop you in.
adityasaky
reviewed
Aug 19, 2020
securesystemslib/keys.py
Outdated
| @@ -687,7 +687,7 @@ def create_signature(key_dict, data): | |||
| securesystemslib.formats.ANYKEY_SCHEMA.check_match(key_dict) | |||
|
|
|||
| # Signing the 'data' object requires a private key. Signing schemes that are | |||
| # currently supported are: 'ed25519', 'ecdsa-sha2-nistp256', and rsa schemes | |||
| # currently supported are: 'ed25519', 'ecdsa', and rsa schemes | |||
There was a problem hiding this comment.
If I'm not misreading this, we're talking about the schemes themselves here, right? Meaning we leave it as is / include ecdsa-sha2-nistp384?
There was a problem hiding this comment.
Well spotted, thanks!
lukpueh
reviewed
Aug 20, 2020
| elif keytype == 'ecdsa-sha2-nistp256': | ||
| # Continue to support keytypes of ecdsa-sha2-nistp256 and ecdsa-sha2-nistp384 | ||
| # for backwards compatibility with older securesystemslib releases | ||
| elif keytype in ['ecdsa', 'ecdsa-sha2-nistp256', 'ecdsa-sha2-nistp384']: |
There was a problem hiding this comment.
NOTE: We didn't support generation of ecdsa-sha2-nistp384 signatures prior to this PR (#228 only added verification support).
With this patch, we'll fail one level deeper when schemes are checked, which IMO is fine.
lukpueh
approved these changes
Aug 20, 2020
joshuagl
added a commit
to joshuagl/specification
that referenced
this pull request
Mar 1, 2023
We claim that the spec is just documenting the signature _schemes_ from the reference implementation, but that we define three _keytypes_ within the spec. This change first updates the _keytypes_ to match the reference implementation (we have defaulted to a generic "ecdsa" keytype since secure-systems-lab/securesystemslib#267). Further, we update the specification to clarify that within we are documenting the keytypes and schemes from the reference implementation, and that we recommend implementing these keytypes and schemes as specified. Signed-off-by: Joshua Lock <[email protected]>
joshuagl
added a commit
to joshuagl/specification
that referenced
this pull request
Mar 2, 2023
We claim that the spec is just documenting the signature _schemes_ from the reference implementation, but that we define three _keytypes_ within the spec. This change first updates the _keytypes_ to match the reference implementation (we have defaulted to a generic "ecdsa" keytype since secure-systems-lab/securesystemslib#267). Further, we update the specification to clarify that within we are documenting the keytypes and schemes from the reference implementation, and that we recommend implementing these keytypes and schemes as specified. Signed-off-by: Joshua Lock <[email protected]>
joshuagl
added a commit
to joshuagl/specification
that referenced
this pull request
Mar 15, 2023
We claim that the spec is just documenting the signature _schemes_ from the reference implementation, but that we define three _keytypes_ within the spec. This change first updates the _keytypes_ to match the reference implementation (we have defaulted to a generic "ecdsa" keytype since secure-systems-lab/securesystemslib#267). Further, we update the specification to clarify that within we are documenting the keytypes and schemes from the reference implementation, and that we recommend implementing these keytypes and schemes as specified. Signed-off-by: Joshua Lock <[email protected]>
joshuagl
added a commit
to joshuagl/specification
that referenced
this pull request
May 12, 2023
We claim that the spec is just documenting the signature _schemes_ from the reference implementation, but that we define three _keytypes_ within the spec. This change first updates the _keytypes_ to match the reference implementation (we have defaulted to a generic "ecdsa" keytype since secure-systems-lab/securesystemslib#267). Further, we update the specification to clarify that within we are documenting the keytypes and schemes from the reference implementation, and that we recommend implementing these keytypes and schemes as specified. Signed-off-by: Joshua Lock <[email protected]>
joshuagl
added a commit
to theupdateframework/specification
that referenced
this pull request
May 15, 2023
We claim that the spec is just documenting the signature _schemes_ from the reference implementation, but that we define three _keytypes_ within the spec. This change first updates the _keytypes_ to match the reference implementation (we have defaulted to a generic "ecdsa" keytype since secure-systems-lab/securesystemslib#267). Further, we update the specification to clarify that within we are documenting the keytypes and schemes from the reference implementation, and that we recommend implementing these keytypes and schemes as specified. Signed-off-by: Joshua Lock <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes issue #: #239
Description of the changes being introduced by the pull request: use just 'ecdsa' as the keytype for newly created ECDSA keys, while continuing to support keys that were generated with the old style keytype that included the signing scheme.
Please verify and check that the pull request fulfils the following
requirements: