jwx: Incorrect webkey thumbprint calculation

Hi

I am porting some jwk functionality to jwx v 1.0.5 library (currently the latest release).

I found an EC thumbprint calculation incompatibility with go-jose and C jose libraries. Here is a webkey:

{"kty":"EC","alg":"ECMR","crv":"P-521","key_ops":["deriveKey"],"x":"AJwCS845x9VljR-fcrN2WMzIJHDYuLmFShhyu8ci14rmi2DMFp8txIvaxG8n7ZcODeKIs1EO4E_Bldm_pxxs8cUn","y":"ASjz754cIQHPJObihPV8D7vVNfjp_nuwP76PtbLwUkqTk9J1mzCDKM3VADEk-Z1tP-DHiwib6If8jxnb_FjNkiLJ"}

C jose library gives following thumbprint:

jose jwk thp -i- <<< '{"kty":"EC","alg":"ECMR","crv":"P-521","key_ops":["deriveKey"],"x":"AJwCS845x9VljR-fcrN2WMzIJHDYuLmFShhyu8ci14rmi2DMFp8txIvaxG8n7ZcODeKIs1EO4E_Bldm_pxxs8cUn","y":"ASjz754cIQHPJObihPV8D7vVNfjp_nuwP76PtbLwUkqTk9J1mzCDKM3VADEk-Z1tP-DHiwib6If8jxnb_FjNkiLJ"}'
2Mc_43O_BOrOJTNrGX7uJ6JsIYE

But jxk gives ZNIw9ANrxjuqoiqZjzJD1pGr43w thumbprint.

I started comparing how the values are computed in jwk vs go-jose and found that go-jose performs x and y padding to the size of the EC curve. Such padding adds one extra 0 in the front of key.X and key.Y.

As a proof of concept I ported code from go-jose and with the patch below the thumbprints become identical between different implementation:

diff --git a/jwk/ecdsa.go b/jwk/ecdsa.go
index 61736b0..272eacb 100644
--- a/jwk/ecdsa.go
+++ b/jwk/ecdsa.go
@@ -157,11 +157,14 @@ func (k ecdsaPublicKey) Thumbprint(hash crypto.Hash) ([]byte, error) {
        if err := k.Raw(&key); err != nil {
                return nil, errors.Wrap(err, `failed to materialize ecdsa.PublicKey for thumbprint generation`)
        }
+
+       curveSize := curveSize(key.Curve)
+
        return ecdsaThumbprint(
                hash,
                key.Curve.Params().Name,
-               base64.EncodeToString(key.X.Bytes()),
-               base64.EncodeToString(key.Y.Bytes()),
+               base64.EncodeToString(newFixedSizeBuffer(key.X.Bytes(), curveSize)),
+               base64.EncodeToString(newFixedSizeBuffer(key.Y.Bytes(), curveSize)),
        ), nil
 }
 
@@ -179,3 +182,25 @@ func (k ecdsaPrivateKey) Thumbprint(hash crypto.Hash) ([]byte, error) {
                base64.EncodeToString(key.Y.Bytes()),
        ), nil
 }
+
+// Get size of curve in bytes
+func curveSize(crv elliptic.Curve) int {
+       bits := crv.Params().BitSize
+
+       div := bits / 8
+       mod := bits % 8
+
+       if mod == 0 {
+               return div
+       }
+
+       return div + 1
+}
+
+func newFixedSizeBuffer(data []byte, length int) []byte {
+       if len(data) > length {
+               panic("jwx/jwk: invalid call to newFixedSizeBuffer (len(data) > length)")
+       }
+       pad := make([]byte, length-len(data))
+       return append(pad, data...)
+}

ecdsaPrivateKey.Thumbprint() need to be patched as well.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 20 (7 by maintainers)

Commits related to this issue

Most upvoted comments

Do I understand correctly that the jose project is correctly following the spec? A comment says that go-jose pads and jose doesn’t, but the patch implies it is the other way around. I reviewed our code and I think we are correct.

Both jose and go-jose do padding (and it is correct as per RFC you pointed). jwx does not do padding and it is what we try to clarify/fix.

jose folks pointed to Flattened JWE JSON Serialization Syntax https://tools.ietf.org/html/rfc7516#section-7.2.2

Also I think this no recipients error should be tracked as a separate issue.