Fix PEM armor for EC private keys when encoding (#876)

* Incorporate #875

* Test PEM roundtrip for other key types

* Use more constants

jwk/jwk.go

@@ -234,8 +234,11 @@ func PublicRawKeyOf(v interface{}) (interface{}, error) {
 }
 
 const (
- pmPrivateKey = `PRIVATE KEY`
- pmPublicKey = `PUBLIC KEY`
+ pmPrivateKey = `PRIVATE KEY`
+ pmPublicKey = `PUBLIC KEY`
+ pmECPrivateKey = `EC PRIVATE KEY`
+ pmRSAPublicKey = `RSA PUBLIC KEY`
+ pmRSAPrivateKey = `RSA PRIVATE KEY`
 )
 
 // EncodeX509 encodes the key into a byte sequence in ASN.1 DER format
@@ -263,13 +266,13 @@ func EncodeX509(v interface{}) (string, []byte, error) {
  // Try to convert it into a certificate
  switch v := v.(type) {
  case *rsa.PrivateKey:
- return "RSA PRIVATE KEY", x509.MarshalPKCS1PrivateKey(v), nil
+ return pmRSAPrivateKey, x509.MarshalPKCS1PrivateKey(v), nil
  case *ecdsa.PrivateKey:
  marshaled, err := x509.MarshalECPrivateKey(v)
  if err != nil {
  return "", nil, err
  }
- return "ECDSA PRIVATE KEY", marshaled, nil
+ return pmECPrivateKey, marshaled, nil
  case ed25519.PrivateKey:
  marshaled, err := x509.MarshalPKCS8PrivateKey(v)
  if err != nil {
@@ -316,19 +319,19 @@ func DecodePEM(src []byte) (interface{}, []byte, error) {
 
  switch block.Type {
  // Handle the semi-obvious cases
- case "RSA PRIVATE KEY":
+ case pmRSAPrivateKey:
  key, err := x509.ParsePKCS1PrivateKey(block.Bytes)
  if err != nil {
  return nil, nil, fmt.Errorf(`failed to parse PKCS1 private key: %w`, err)
  }
  return key, rest, nil
- case "RSA PUBLIC KEY":
+ case pmRSAPublicKey:
  key, err := x509.ParsePKCS1PublicKey(block.Bytes)
  if err != nil {
  return nil, nil, fmt.Errorf(`failed to parse PKCS1 public key: %w`, err)
  }
  return key, rest, nil
- case "EC PRIVATE KEY":
+ case pmECPrivateKey:
  key, err := x509.ParseECPrivateKey(block.Bytes)
  if err != nil {
  return nil, nil, fmt.Errorf(`failed to parse EC private key: %w`, err)

jwk/jwk_test.go

@@ -214,28 +214,65 @@ func VerifyKey(t *testing.T, def map[string]keyDef) {
  }
  })
  t.Run("Roundtrip", func(t *testing.T) {
- buf, err := json.Marshal(key)
- if !assert.NoError(t, err, `json.Marshal should succeed`) {
- return
+ var supportsPEM bool
+ switch key.KeyType() {
+ case jwa.OKP, jwa.OctetSeq:
+ default:
+ supportsPEM = true
  }
 
- newkey, err := jwk.ParseKey(buf)
- if !assert.NoError(t, err, `jwk.ParseKey should succeed`) {
- return
- }
+ for _, usePEM := range []bool{true, false} {
+ if usePEM && !supportsPEM {
+ continue
+ }
+ t.Run(fmt.Sprintf("WithPEM(%t)", usePEM), func(t *testing.T) {
+ var buf []byte
+ if usePEM {
+ pem, err := jwk.EncodePEM(key)
+ if !assert.NoError(t, err, `jwk.EncodePEM should succeed`) {
+ return
+ }
+ buf = pem
+ } else {
+ jsonbuf, err := json.Marshal(key)
+ if !assert.NoError(t, err, `json.Marshal should succeed`) {
+ return
+ }
+ buf = jsonbuf
+ }
 
- m1, err := key.AsMap(context.TODO())
- if !assert.NoError(t, err, `key.AsMap should succeed`) {
- return
- }
+  newkey, err := jwk.ParseKey(buf, jwk.WithPEM(usePEM))
+  if !assert.NoError(t, err, `jwk.ParseKey should succeed`) {
+  return
+  }
 
- m2, err := newkey.AsMap(context.TODO())
- if !assert.NoError(t, err, `key.AsMap should succeed`) {
- return
- }
+  m1, err := key.AsMap(context.TODO())
+  if !assert.NoError(t, err, `key.AsMap should succeed`) {
+  return
+  }
 
- if !assert.Equal(t, m1, m2, `keys should match`) {
- return
+ m2, err := newkey.AsMap(context.TODO())
+ if !assert.NoError(t, err, `key.AsMap should succeed`) {
+ return
+ }
+
+ // PEM does not preserve these keys
+ if usePEM {
+ delete(m1, `private`)
+ delete(m1, jwk.AlgorithmKey)
+ delete(m1, jwk.KeyIDKey)
+ delete(m1, jwk.KeyOpsKey)
+ delete(m1, jwk.KeyUsageKey)
+ delete(m1, jwk.X509CertChainKey)
+ delete(m1, jwk.X509CertThumbprintKey)
+ delete(m1, jwk.X509CertThumbprintS256Key)
+ delete(m1, jwk.X509URLKey)
+ }
+
+ if !assert.Equal(t, m1, m2, `keys should match`) {
+ return
+ }
+ })
  }
  })
  t.Run("Raw", func(t *testing.T) {
@@ -2070,3 +2107,26 @@ func TestGH730(t *testing.T) {
  require.NoError(t, set.AddKey(key), `first AddKey should succeed`)
  require.Error(t, set.AddKey(key), `second AddKey should fail`)
 }
+
+// This test was lifted from #875. See tests under Roundtrip/WithPEM(true) for other key types
+func TestECDSAPEM(t *testing.T) {
+ // go make an EC key at https://mkjwk.org/
+ key, err := jwk.ParseKey([]byte(`{
+ "kty": "EC",
+ "d": "zqYPTs5gMEwtidOqjlFJSk6L4BQSfhCJX6FTgbuuiE0",
+ "crv": "P-256",
+ "x": "AYwhwiE1hXWdfwu-HlBSsY5Chxycu-LyE6WsZ_w2DO4",
+ "y": "zumemGclMFkimMsKMXlLdKYWtLle58e4N9hDPcN7lig"
+ }`))
+ if err != nil {
+ t.Fatal(err)
+ }
+ pem, err := jwk.EncodePEM(key)
+ if err != nil {
+ t.Fatal(err)
+ }
+ _, err = jwk.ParseKey(pem, jwk.WithPEM(true))
+ if err != nil {
+ t.Fatal(err)
+ }
+}