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

Strip public key pem value #457

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link

@danbev danbev commented Nov 14, 2022

Description of the changes being introduced by the pull request:

This commit adds a call to strip the public key pem value in import_ecdsakey_from_private_pem.

The motivation for this that a later call to import_ecdsakey_from_public_pem will otherwise cause an incorrect keyid to be generated. This is because the call to extract_pem will strip any new line character from the public pem, which will then be used to generate the keyid, but this will be a different value than the keyid generated earlier due to the removal of the new line character.

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
  • Docs have been added for the bug fix or new feature

This commit adds a call to strip the public key pem value in
import_ecdsakey_from_private_pem.

The motivation for this that a later call to
import_ecdsakey_from_public_pem will otherwise cause an incorrect keyid
to be generated. This is because the call to `extract_pem` will strip
any new line character from the public pem, which will then be used to
generate the keyid, but this will be a different value than the keyid
generated earlier due to the removal of the new line character.

Signed-off-by: Daniel Bevenius <[email protected]>
@jku
Copy link
Collaborator

jku commented Nov 14, 2022

this looks a lot like #453 just on a slightly higher level: I assume that one would likely fix the issue for you too?

I'll also leave the same comment as in that pr: I would not trust the keyid generation to be stable and would instead generate a keyid once and then use that consistently in the application.

@danbev
Copy link
Author

danbev commented Nov 15, 2022

this looks a lot like #453 just on a slightly higher level: I assume that one would likely fix the issue for you too?

Thanks for pointing that out, and yes I think that would solve this issue.

I would not trust the keyid generation to be stable and would instead generate a keyid once and then use that consistently in the application.

Ah I see, good to know that and I'll do that. Thanks!

@danbev danbev closed this Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants