Implement ability to handle PEM-formatted secp256k1 keys (#1117)

* Work with secp256k1 keys in PEM format

* Add missing files

* tweak bazel files

* guard against go < 1.20

* protect the test from go < 1.20

* Fix import path

* Override ParsePKCS8PrivateKey

* Make this feature available through a separate build tag

* Add lots of warnings about the feature

* Add missing rules

.github/workflows/ci.yml

@@ -10,7 +10,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        go_tags: [ 'stdlib', 'goccy', 'es256k', 'asmbase64', 'alltags']
+        go_tags: [ 'stdlib', 'goccy', 'es256k', 'secp256k1-pem', 'asmbase64', 'alltags']
         go: [ '1.21', '1.20', '1.19' ]
     name: "Test [ Go ${{ matrix.go }} / Tags ${{ matrix.go_tags }} ]"
     steps:

Changes

@@ -37,6 +37,46 @@ v2.0.22 UNRELEASED
     use this option unless you know exactly what you are doing, as this will pose
     significant security issues when used incorrectly.
 
+  * [jwk] Provide a _stop-gap_ measure to work with PEM format ASN.1 DER encoded secp256k1 keys.
+
+    In order to enable this feature, you must compile jwx with TWO build tags:
+    `jwx_es256k` to enable ES256K/secp256k1, and `jwx_secp256k1_pem` to enable PEM handling.
+    Not one, but BOTH tags need to be present.
+
+    With this change, by suppliying the `WithPEM(true)` option, `jwk.Parse()` is now
+    able to read sep256k1 keys. Also, `jwk.Pem()` should be able to handle `jwk.Key` objects
+    that represent a secp256k1 key.
+
+    Please do note that the implementation of this feature is dodgy at best. Currently
+    Go's crypto/x509 does not allow handling additional EC curves, and thus in order to
+    accomodate secp256k1 keys in PEM/ASN.1 DER format we need to "patch" the stdlib.
+    We do this by copy-and-pasting relevant parts of go 1.22.2's crypto/x509 code and
+    adding the minimum required code to make secp256k1 keys work.
+
+    Because of the above, there are several important caveats for this feature:
+
+    1. This feature is provided solely as a stop-gap measure until such time Go's stdlib
+    provides a way to handle non-standard EC curves, or another external module
+    is able to solve this issue. 
+
+    2. This feature should be considered unstable and not guaranteed by semantic versioning
+    backward compatibility. At any given point we may drop this feature. It may be
+    because we can no longer maintain the code, or perhaps a security issue is found in the
+    version of the code that we ship with, etc.
+
+    3. Please always remember that we are now bundling a static set of code for handling
+    x509 formats. You are taking a possible security risk by possibly using outdated code.
+    Please always do your own research, and if possible, please notify us if the bundled
+    code needs to be updated. Unless you know what you are doing, it is not recommended
+    that you enable this feature.
+
+    4. Please note that because we mostly just imported the code from go 1.22's src/crypto/x509,
+    it has some go1.20-isms in its code. Therefore you will not be able to use the
+    `jwx_secp256k1_pem` tag to enable secp256k1 key PEM handling against codebases
+    that are built using go 1.19 and below. 
+
+    5. We have no plans to include more curves this way. One is already one too many.
+
 v2.0.21 07 Mar 2024
 [Security]
   * [jwe] Added `jwe.Settings(jwe.WithMaxDecompressBufferSize(int64))` to specify the

Makefile

@@ -25,11 +25,14 @@ test-goccy:
 test-es256k:
 	$(MAKE) test-cmd TESTOPTS="-tags jwx_es256k"
 
+test-secp256k1-pem:
+	$(MAKE) test-cmd TESTOPTS="-tags jwx_es256k,jwx_secp256k1_pem"
+
 test-asmbase64:
 	$(MAKE) test-cmd TESTOPTS="-tags jwx_asmbase64"
 
 test-alltags:
-	$(MAKE) test-cmd TESTOPTS="-tags jwx_asmbase64,jwx_goccy,jwx_es256k"
+	$(MAKE) test-cmd TESTOPTS="-tags jwx_asmbase64,jwx_goccy,jwx_es256k,jwx_secp256k1_pem"
 
 cover-cmd:
 	env MODE=cover ./tools/test.sh
@@ -46,11 +49,14 @@ cover-goccy:
 cover-es256k:
 	$(MAKE) cover-cmd TESTOPTS="-tags jwx_es256k"
 
+cover-secp256k1-pem:
+	$(MAKE) cover-cmd TESTOPTS="-tags jwx_es256k,jwx_secp256k1"
+
 cover-asmbase64:
 	$(MAKE) cover-cmd TESTOPTS="-tags jwx_asmbase64"
 
 cover-alltags:
-	$(MAKE) cover-cmd TESTOPTS="-tags jwx_asmbase64,jwx_goccy,jwx_es256k"
+	$(MAKE) cover-cmd TESTOPTS="-tags jwx_asmbase64,jwx_goccy,jwx_es256k,jwx_secp256k1_pem"
 
 smoke-cmd:
 	env MODE=short ./tools/test.sh
@@ -67,8 +73,11 @@ smoke-goccy:
 smoke-es256k:
 	$(MAKE) smoke-cmd TESTOPTS="-tags jwx_es256k"
 
+smoke-secp256k1-pem:
+	$(MAKE) smoke-cmd TESTOPTS="-tags jwx_es256k,jwx_secp256k1_pem"
+
 smoke-alltags:
-	$(MAKE) smoke-cmd TESTOPTS="-tags jwx_goccy,jwx_es256k"
+	$(MAKE) smoke-cmd TESTOPTS="-tags jwx_goccy,jwx_es256k,jwx_secp256k1_pem"
 
 viewcover:
 	go tool cover -html=coverage.out

jwk/BUILD.bazel

@@ -35,6 +35,7 @@ go_library(
         "//internal/pool",
         "//jwa",
         "//x25519",
+	"//jwk/internal/x509",
         "@com_github_lestrrat_go_blackmagic//:go_default_library",
         "@com_github_lestrrat_go_httprc//:go_default_library",
         "@com_github_lestrrat_go_iter//arrayiter:go_default_library",
@@ -66,6 +67,7 @@ go_test(
         "//jwa",
         "//jws",
         "//x25519",
+        "//jwk/internal/x509",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
     ],

jwk/es256k_go1.20_test.go

@@ -0,0 +1,44 @@
+//go:build jwx_es256k && jwx_secp256k1_pem && go1.20
+
+package jwk_test
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/decred/dcrd/dcrec/secp256k1/v4"
+	"github.com/lestrrat-go/jwx/v2/jwk"
+	"github.com/stretchr/testify/require"
+)
+
+func TestES256KPem(t *testing.T) {
+	raw, err := secp256k1.GeneratePrivateKey()
+	require.NoError(t, err, `GeneratePrivateKey should succeed`)
+
+	testcases := []interface{}{raw.ToECDSA(), raw.PubKey().ToECDSA()}
+	for _, tc := range testcases {
+		t.Run(fmt.Sprintf("Marshal %T", tc), func(t *testing.T) {
+			key, err := jwk.FromRaw(tc)
+			require.NoError(t, err, `FromRaw should succeed`)
+
+			pem, err := jwk.Pem(key)
+			require.NoError(t, err, `Pem should succeed`)
+			require.NotEmpty(t, pem, `Pem should not be empty`)
+
+			parsed, err := jwk.Parse(pem, jwk.WithPEM(true))
+			require.NoError(t, err, `Parse should succeed`)
+			_ = parsed
+		})
+	}
+
+	t.Run("ParsePKCS8PrivateKey", func(t *testing.T) {
+		const src = `-----BEGIN PRIVATE KEY-----
+MIGEAgEAMBAGByqGSM49AgEGBSuBBAAKBG0wawIBAQQggS9t6iYyj9JSL+btkMEq
+pMYitWV4X+/Jg9zu3L8Ob5ShRANCAAT/YrxWHfw3e8lfDncJLLkPRbdby0L4qT95
+vyWU5lPpSwRbEAfSFR1E5RD9irkN1mCY8D1ko1PAlmHVB78pNzq4
+-----END PRIVATE KEY-----`
+		key, err := jwk.Parse([]byte(src), jwk.WithPEM(true))
+		require.NoError(t, err, `Parse should succeed`)
+		require.NotNil(t, key, `key should not be nil`)
+	})
+}

jwk/internal/x509/BUILD.bazel

@@ -0,0 +1,14 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+
+go_library(
+    name = "x509",
+    srcs = [ "x509.go", "x509_nosecp256k1.go", "x509_sepc256k1.go" ],
+    importpath = "github.com/lestrrat-go/jwx/v2/jwk/internal/x509",
+    visibility = ["//:__subpackages__"],
+)
+
+alias(
+    name = "go_default_library",
+    actual = ":x509",
+    visibility = ["//jwe:__subpackages__"],
+)

jwk/internal/x509/x509.go

@@ -0,0 +1,31 @@
+package x509
+
+import (
+	"crypto/rsa"
+	"crypto/x509"
+)
+
+// In this x509 package we provide a proxy for crypto/x509 methods,
+// so that we can easily swap out the ParseECPrivateKey method with
+// our version of it that recognizes the secp256k1 curve...
+// _IF_ the jwx_es256k build tag is set.
+
+func MarshalPKCS1PrivateKey(priv *rsa.PrivateKey) []byte {
+	return x509.MarshalPKCS1PrivateKey(priv)
+}
+
+func MarshalPKCS8PrivateKey(priv interface{}) ([]byte, error) {
+	return x509.MarshalPKCS8PrivateKey(priv)
+}
+
+func ParsePKCS1PrivateKey(der []byte) (*rsa.PrivateKey, error) {
+	return x509.ParsePKCS1PrivateKey(der)
+}
+
+func ParsePKCS1PublicKey(der []byte) (*rsa.PublicKey, error) {
+	return x509.ParsePKCS1PublicKey(der)
+}
+
+func ParseCertificate(der []byte) (*x509.Certificate, error) {
+	return x509.ParseCertificate(der)
+}

jwk/internal/x509/x509_nosecp256k1.go

@@ -0,0 +1,28 @@
+//go:build !jwx_es256k || !jwx_secp256k1_pem || !go1.20
+
+package x509
+
+import (
+	"crypto/ecdsa"
+	"crypto/x509"
+)
+
+func MarshalECPrivateKey(priv *ecdsa.PrivateKey) ([]byte, error) {
+	return x509.MarshalECPrivateKey(priv)
+}
+
+func ParseECPrivateKey(der []byte) (*ecdsa.PrivateKey, error) {
+	return x509.ParseECPrivateKey(der)
+}
+
+func MarshalPKIXPublicKey(pub any) ([]byte, error) {
+	return x509.MarshalPKIXPublicKey(pub)
+}
+
+func ParsePKIXPublicKey(der []byte) (any, error) {
+	return x509.ParsePKIXPublicKey(der)
+}
+
+func ParsePKCS8PrivateKey(der []byte) (interface{}, error) {
+	return x509.ParsePKCS8PrivateKey(der)
+}