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

Ability to work with PEM/DER for secp256k1 #1116

Closed
excavador opened this issue Apr 18, 2024 · 14 comments · Fixed by #1117
Closed

Ability to work with PEM/DER for secp256k1 #1116

excavador opened this issue Apr 18, 2024 · 14 comments · Fixed by #1117
Assignees

Comments

@excavador
Copy link

excavador commented Apr 18, 2024

Abstract

I would like to see workable Pem and ParseKey(data, WithPem()) functions for key secp256k1.

Describe the proposed solution/change

The vanilla golang crypto/x509 does not support marshal/unmarshal PEM according to PKCS#8 and PKIX.
There should be some alternative PEM parser/writer.

Additional context

We could sponsor this work

@lestrrat
Copy link
Collaborator

Hi! From a tech perspective, I'm OK to consider this, albeit not being a high priority for me, as it doesn't feel like it's part of the jwx suite. But I'm OK with the idea of having something that fills the gap while somebody else comes up with a "real" solution.

(I'm adding the long-term label so that this issue doesn't get closed immediately by my bots)

===

Now, I'm pretty sure you mentioned sponsoring the work out of your well-intentions, and to that I'm thankful, but let me just use this as a chance to leave a note for all those who might see this issue later. Please note that while I'm piggybacking on your issue, following is more of a general statement rather than a response to you, @excavador .


For those of you who need a feature and are even willing to pay for it: I generally don't appreciate people hinting at sponsoring FOSS work. This is because through years of experience dealing in businesses and FOSS projects I have come to expect very little out of people who offer vague mentions of help or rewards.

That is to say, there's a lot of concrete actions that you could be taking if you are serious about paying me.

For example, if you want a proper contract b/c of legal/finance reasons, you should already be stating it as such, and also share private means of correspondence so that we can discuss details.

If you just simply want to sponsor this project, you could always just go the GitHub sponsor feature without asking me.

And out of the above two, I prefer the latter, mainly because this is a hobby project for me, though I'm perfectly willing to be hired for developing a feature given the right conditions.

So if you are willing to sponsor our work, thank you, but instead of hinting at it, please just do. And I will be much obliged.

@lestrrat
Copy link
Collaborator

lestrrat commented Apr 19, 2024

@excavador Well, I looked at the code, and it was just mainly a lot of copy-pasting, so here it is: #1117

Please note that because I usually don't deal with secp256k1, not much actual testing has been run against the code. Please check that the code works for your actual keys. Or better yet, please contribute test code.

@excavador
Copy link
Author

excavador commented Apr 19, 2024

Hello, @lestrrat!

I am sorry, I do not know your name, only nickname.

Regarding sponsoring - seems like we have a huge missunderstanding there.
I am sorry for confusion, let me clarify this part.

  1. I am impressed by your work and popularity of your library. You made the amazing work
  2. I need some specific technical future in our project. I considered the several opportunities, I tried to do patch by myself but get lost
  3. I wanted to ask you make this patch. I want to pay to this work.

Meanwhile, I did not find any way to contact you personally.

  • GitHub does not provide functionality of the direct messaging
  • You did not mention any your email
  • I did not find any section on how to contact you
  • I tried to find you on LinkedIn, but I did not find your profile

As the result, it was the real challenge - how exactly I could contact you in private?
I am not aware about this way.
If I missed something, then please note me! I am personally do not have idea how to do it!

I had an idea to use "Security Report" for this private communication, but this is even worse, correct?

Because of that I did not find any better way, except write a public feature request and note that my company willing to pay for this work.
I did not mean to offense you or "abuse sponsorship" in any way!

If it's possible, then could you please contact me by o.tsarev@truvity.com to clarify this part?

Thank you!

@excavador Well, I looked at the code, and it was just mainly a lot of copy-pasting, so here it is: #1117
Please note that because I usually don't deal with secp256k1, not much actual testing has been run against the code. > Please check that the code works for your actual keys. Or better yet, please contribute test code.

Thank you so much! I will test it today or during weekend and provide feedback about any issues.

@lestrrat
Copy link
Collaborator

@excavador Yeah, I'm sorry, my comments came a bit more harsh than I meant huh.

Again, it wasn't that I was upset about your issue or anything, it's really the fear that others might see this thread and go "oh, hey, that guy is easy to manipulate", you know what I mean? So again, I only piggybacked on your issue to make that statement.

Don't worry about the sponsorship. I appreciate your intentions. Maybe next time, yeah?

BTW for future reference, re I think the smartest way to contact somebody who you don't know, this is the best way, as you just did :)

@excavador
Copy link
Author

excavador commented Apr 19, 2024

@excavador Yeah, I'm sorry, my comments came a bit more harsh than I meant huh.

Again, it wasn't that I was upset about your issue or anything, it's really the fear that others might see this thread and go "oh, hey, that guy is easy to manipulate", you know what I mean? So again, I only piggybacked on your issue to make that statement.

Oh, I did not ever think in this way! It's not about the manipulation, it's about how to make a business contract.

Don't worry about the sponsorship. I appreciate your intentions. Maybe next time, yeah?

Of course! No problem!

BTW for future reference, re I think the smartest way to contact somebody who you don't know, this is the best way, as you just did :)

I am former opensource developer (ex-MySQL) and I was really bothering for contact like "let's discuss details in private" without any specific details.

Because of that I made the similar issue, but I have provided my problem/request to get straight to the point, what exactly the subject and purpose of my contact!

@excavador
Copy link
Author

excavador commented Apr 19, 2024

@excavador Well, I looked at the code, and it was just mainly a lot of copy-pasting, so here it is: #1117

Please note that because I usually don't deal with secp256k1, not much actual testing has been run against the code. Please check that the code works for your actual keys. Or better yet, please contribute test code.

@lestrrat I have tested the branch. It works, but only partially, unfortunatelly.

If you generate key by the following command:

openssl ecparam -name secp256k1 -genkey

then the result

-----BEGIN EC PRIVATE KEY-----
MHQCAQEEIOx7TKIzjZt+IY2cy5JmvUSkt3Opqi8Msm+cMWhH92WyoAcGBSuBBAAK
oUQDQgAESuRIrgW3xbN2nJp1azTnS/sVw+4dS72U337lNhDNm55G6MID6I1kbVjT
904lWe3/mfOlEIvHdPSwHtHFkUzL5g==
-----END EC PRIVATE KEY-----

accepted by library and work perfectly

However, if you try to generate key in PKCS8 format by the following command:

openssl ecparam -name secp256k1 -genkey | openssl pkcs8 -nocrypt -topk8

then the result

-----BEGIN PRIVATE KEY-----
MIGEAgEAMBAGByqGSM49AgEGBSuBBAAKBG0wawIBAQQggS9t6iYyj9JSL+btkMEq
pMYitWV4X+/Jg9zu3L8Ob5ShRANCAAT/YrxWHfw3e8lfDncJLLkPRbdby0L4qT95
vyWU5lPpSwRbEAfSFR1E5RD9irkN1mCY8D1ko1PAlmHVB78pNzq4
-----END PRIVATE KEY-----

does not work

failed to parse PEM encoded key: failed to parse PKCS8 private key: x509: failed to parse EC private key embedded in PKCS#8: x509: unknown elliptic curve

Let me provide more context, why exactly I need PKCS8

I am generating these keys by AWS KMS - kms:GenerateDataKeyPair (AWS KMS KeySpec = ECC_SECG_P256K1)

The public key is a DER-encoded X.509 SubjectPublicKeyInfo, as specified in [RFC 5280](https://tools.ietf.org/html/rfc5280).
The private key is a DER-encoded PKCS8 PrivateKeyInfo, as specified in [RFC 5958](https://tools.ietf.org/html/rfc5958).

ok, more correctly by kms:GenerateDataKeyPairWithoutPlaintext, but the same story (DER is encrypted by AWS KMS key, does not matter)

I need to convert this PKIX and PKCS8 keys to JWX, to use JWK to sign Verifiable Credential.
Golang vanilla "crypto/x509" complain about "unknown elliptic curve" when you have secp256k1 inside PKCS8

What you could advise to workaround this issue?

P.S. NIST curves, generated by AWS KMS works perfectly

@excavador
Copy link
Author

excavador commented Apr 19, 2024

I also think, you could find the following shell script useful

    openssl genpkey -algorithm ed25519 > ed25519.EC.private.pem
    openssl genpkey -algorithm ed25519 | openssl pkcs8 -nocrypt -topk8 > ed25519.PKCS8.private.pem
    for spec in secp256r1 secp384r1 secp521r1 secp256k1; do
        openssl ecparam -name $spec -noout -genkey > $spec.EC.private.pem
        openssl ecparam -name $spec -noout -genkey | openssl pkcs8 -nocrypt -topk8 > $spec.PKCS8.private.pem
    done;
    for spec in ed25519 secp256r1 secp384r1 secp521r1 secp256k1; do
        for name in EC PKCS8; do 
            openssl pkey -in $spec.$name.private.pem -pubout -out $spec.$name.public.pem
        done;
    done;

@lestrrat
Copy link
Collaborator

I haven't used your shell script yet, but I think I fixed your problem in this comment via 9480ead

@excavador
Copy link
Author

excavador commented Apr 19, 2024

I haven't used your shell script yet, but I think I fixed your problem in this comment via 9480ead

Amazing! Works! Thank you so much!

I could confirm it for the following cases

	//go:embed testdata/ed25519.EC.private.pem
	Pem_Ed25519_EC_private []byte
	//go:embed testdata/ed25519.EC.public.pem
	Pem_Ed25519_EC_public []byte

	//go:embed testdata/ed25519.PKCS8.private.pem
	Pem_Ed25519_PKSC8_private []byte
	//go:embed testdata/ed25519.PKCS8.public.pem
	Pem_Ed25519_PKSC8_public []byte

	//go:embed testdata/secp256r1.EC.private.pem
	Pem_NIST256_EC_private []byte
	//go:embed testdata/secp256r1.EC.public.pem
	Pem_NIST256_EC_public []byte

	//go:embed testdata/secp256r1.PKCS8.public.pem
	Pem_NIST256_PKSC8_public []byte
	//go:embed testdata/secp256r1.PKCS8.private.pem
	Pem_NIST256_PKSC8_private []byte

	//go:embed testdata/secp384r1.EC.private.pem
	Pem_NIST384_EC_private []byte
	//go:embed testdata/secp384r1.EC.public.pem
	Pem_NIST384_EC_public []byte

	//go:embed testdata/secp384r1.PKCS8.private.pem
	Pem_NIST384_PKSC8_private []byte
	//go:embed testdata/secp384r1.PKCS8.public.pem
	Pem_NIST384_PKSC8_public []byte

	//go:embed testdata/secp256k1.EC.private.pem
	Pem_SECP256K1_EC_private []byte
	//go:embed testdata/secp256k1.EC.public.pem
	Pem_SECP256K1_EC_public []byte

	//go:embed testdata/secp256k1.PKCS8.private.pem
	Pem_SECP256K1_PKSC8_private []byte
	//go:embed testdata/secp256k1.PKCS8.public.pem
	Pem_SECP256K1_PKSC8_public []byte

@lestrrat
Copy link
Collaborator

I think I'm okay with this fix, but this is really not great in that I'd need to keep up with changes to the core src/x509 code in order to make this work.

So I think I'm going to have to slap a warning like "This feature is only available as a stop-gap until such time handling of secp256k1 keys are more streamlined either in Go's stdlib or as another external module. It is NOT guaranteed to be available indefinitely, nor should you expect this feature to be covered by semantic versioning backward compatibility"

@excavador
Copy link
Author

So I think I'm going to have to slap a warning like "This feature is only available as a stop-gap until such time handling of secp256k1 keys are more streamlined either in Go's stdlib or as another external module. It is NOT guaranteed to be available indefinitely, nor should you expect this feature to be covered by semantic versioning backward compatibility"

Yes, complete understandable. The problem is the following - the native golang library does not suppor secp256k1, and because of that you have issue like this one :(

But you manage to fix it!

@excavador
Copy link
Author

@lestrrat the last bit - will be so cool to see it in stable version :)

@lestrrat
Copy link
Collaborator

Sorry, but a release probably won't happen immediately. I've been doing this enough that you tend to find problems, typos, omissions the moment you release a new version :)

I'll merge but will wait for a bit until we release a new version.


Also, please note that I've decided to put this feature under a separate build tag (jwx_secp256k1_pem) than the one that enable ES256K support (jwx_es256k). Please read the Changes file for details.

@excavador
Copy link
Author

Sorry, but a release probably won't happen immediately. I've been doing this enough that you tend to find problems, typos, omissions the moment you release a new version :)

I'll merge but will wait for a bit until we release a new version.

Also, please note that I've decided to put this feature under a separate build tag (jwx_secp256k1_pem) than the one that enable ES256K support (jwx_es256k). Please read the Changes file for details.

No problem, it could wait. Thank you for your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants