Incorrect webkey thumbprint calculation

[anatol]

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.

[lestrrat]

Oh, it's fixed size against the curve width... hmmm.

Not to cast doubt or anything, but I'd like to check against the spec and/or an example to make sure what the algorithm should really be. I'm going to go research on my own too, but if you have pointers and could share it, I'd really appreciate it.

[lestrrat]

Okay, so the when the curve size is N bytes (in your sample, 66), but the number of bytes in the input number is less than N (in this case 65), go-jose pads this to match the curveSize, so that the input to base64 encoding is always at N bytes.

Implementation wise this seems straight forward, but so far I cannot find a reference that says that this padding is necessary. Part of me is nagging me to check if this is go-jose's definition of correct hashing, a defacto-standard, or part of a spec before matching this behavior.

I'm going to have to be off to $day_job now, but if you have pointers, that would be great.

[lestrrat]

Oops, I realize I made a mistake in the interpretation of the code. Anyways, gotta go..

[anatol]

I do not really know what jwx spec expects here.

In my case I was testing 2 different implementations go-jose and jose and they have the same implementation while jwx has different implementation.

And one more place where one needs to inspect the implementation is ECDHESDecrypt.Decrypt(). go-jose adds padding here as well.

[lestrrat]

righto. defacto-standard, I guess. Want to try out #208 ?

Can you please provide a test case for the ECDHESDecrypt

[anatol]

As of ECDHESDecrypt testing the best way would probably be to generate an encrypted message using jose tool and then try to decrypt it using jwx.

Here is a way to generate a message:

jose jwk gen -i '{"alg":"ECDH-ES"}' -o ec.jwk
jose jwk thp -i ec.jwk
echo -n hi | jose jwe enc -I- -k ec.jwk -o msg.jwe
jose jwe fmt -i msg.jwe -c

it gives following key:

{"alg":"ECDH-ES","crv":"P-521","d":"ARxUkIjnB7pjFzM2OIIFcclR-4qbZwv7DoC96cksPKyvVWOkEsZ0CK6deM4AC6G5GClR5TXWMQVC_bNDmfuwPPqF","key_ops":["wrapKey","unwrapKey"],"kty":"EC","x":"ACewmG5j0POUDQw3rIqFQozK_6yXUsfNjiZtWqQOU7MXsSKK9RsRS8ySmeTG14heUpbbnrC9VdYKSOUGkYnYUl2Y","y":"ACkXSOma_FP93R3u5uYX7gUOlM0LDkNsij9dVFPbafF8hlfYEnUGit2o-tt7W0Zq3t38jEhpjUoGgM04JDJ6_m0x"}

and message

{"ciphertext":"sp0cLt4Rx1p0Ax0Q1OZj7w","header":{"alg":"ECDH-ES","epk":{"crv":"P-521","kty":"EC","x":"APMKQpje5vu39-eS_LX_g15stqbNZ37GgYimW8PZf7d_OOuAygK2YlINYnPoUybrxkoaLRPhbmxc9MBWFdaY8SXx","y":"AMpq4DFi6w-pfnprO58CkfX-ncXtJ8fvox2Ej8Ey3ZY1xjVUtbDJCDCjY53snYaNCEjnFQPAn-IkAG90p2Xcm8ut"}},"iv":"Fjnb5uUWp9euqp1MK_hT4A","protected":"eyJlbmMiOiJBMjU2Q0JDLUhTNTEyIn0","tag":"6nhiy-vyqwVjpy08jrorTpWqvam66HdKxU36XsE3Z3s"}

And here is the Go code to test decrypt codepath

package main

import (
    "crypto"
    "crypto/ecdsa"
    "encoding/base64"
    "log"

    "github.com/lestrrat-go/jwx/jwa"
    "github.com/lestrrat-go/jwx/jwe"
    "github.com/lestrrat-go/jwx/jwk"
)

func main() {
    // generate encrypted message with
    // jose jwk gen -i '{"alg":"ECDH-ES"}' -o ec.jwk
    // jose jwk thp -i ec.jwk
    // echo -n hi | jose jwe enc -I- -k ec.jwk -o msg.jwe
    // jose jwe fmt -i msg.jwe -c

    webKeys, err := jwk.ParseString(`{"alg":"ECDH-ES","crv":"P-521","d":"ARxUkIjnB7pjFzM2OIIFcclR-4qbZwv7DoC96cksPKyvVWOkEsZ0CK6deM4AC6G5GClR5TXWMQVC_bNDmfuwPPqF","key_ops":["wrapKey","unwrapKey"],"kty":"EC","x":"ACewmG5j0POUDQw3rIqFQozK_6yXUsfNjiZtWqQOU7MXsSKK9RsRS8ySmeTG14heUpbbnrC9VdYKSOUGkYnYUl2Y","y":"ACkXSOma_FP93R3u5uYX7gUOlM0LDkNsij9dVFPbafF8hlfYEnUGit2o-tt7W0Zq3t38jEhpjUoGgM04JDJ6_m0x"}`)
    if err != nil {
        log.Fatal(err)
    }
    webKey := webKeys.Keys[0]
    thumbprint, err := webKey.Thumbprint(crypto.SHA1)
    if err != nil {
        log.Fatal(err)
    }

    expectedThumbprint := "0_6x6e2sZKeq3ka0QV0PEkJagqg"
    if base64.RawURLEncoding.EncodeToString(thumbprint) != expectedThumbprint {
        log.Fatalf("incorrect key thumbprint, expected %v got %v", expectedThumbprint, base64.RawURLEncoding.EncodeToString(thumbprint))
    }

    var key ecdsa.PrivateKey
    err = webKey.Raw(&key)
    if err != nil {
        log.Fatal(err)
    }

    data := `{"ciphertext":"sp0cLt4Rx1p0Ax0Q1OZj7w","header":{"alg":"ECDH-ES","epk":{"crv":"P-521","kty":"EC","x":"APMKQpje5vu39-eS_LX_g15stqbNZ37GgYimW8PZf7d_OOuAygK2YlINYnPoUybrxkoaLRPhbmxc9MBWFdaY8SXx","y":"AMpq4DFi6w-pfnprO58CkfX-ncXtJ8fvox2Ej8Ey3ZY1xjVUtbDJCDCjY53snYaNCEjnFQPAn-IkAG90p2Xcm8ut"}},"iv":"Fjnb5uUWp9euqp1MK_hT4A","protected":"eyJlbmMiOiJBMjU2Q0JDLUhTNTEyIn0","tag":"6nhiy-vyqwVjpy08jrorTpWqvam66HdKxU36XsE3Z3s"}`
    msg, err := jwe.ParseString(data)
    if err != nil {
        log.Fatal(err)
    }

    plaintext, err := msg.Decrypt(jwa.ECDH_ES, &key)
    if err != nil {
        log.Fatal(err)
    }

    expected := "hi"
    if string(plaintext) != expected {
        log.Fatalf("incorrect decrypted text, expected %v got %v", expected, string(plaintext))
    }
}

but running it gives me another error for some reason: no recipients, can not proceed with decrypt.

[lestrrat]

I was trying to figure out why it wasn't detecting any recipients, but during the course I got confused: Your JWE message doesn't have an encrypted key section. I was under the impression that a valid JWE message must have the parts described in https://tools.ietf.org/html/rfc7516#section-3, which are

• JOSE Header
• JWE Encrypted Key
• JWE Initialization Vector
• JWE AAD
• JWE Ciphertext
• JWE Authentication Tag

I don't see the aad and the encrypted key... do you know how the jose tool work there?

[anatol]

I am not an expert to answer this question, @npmccallum is a better source of truth here.

Also I found this comment here https://github.com/latchset/jose/blob/master/jose/jwe.h#L27

 * Thus there is (usually) one CEK and one or more JWKs. However, there are
 * some exceptions to this rule. Two such examples are the algorithms: "dir"
 * and "ECDH-ES". In the first case, the JWK is a symmetric key and is used
 * directly as the CEK. In the second case, an ECDH key exchange is performed
 * and the result is used directly as the CEK. But in general, the maxim holds.

so with ECDH-ES there is no encrypted_key in the jwk.

But if for example I generate ECDH-ES+A256KW key jose jwk gen -i '{"alg":"ECDH-ES+A256KW"}' -o ec.jwk (DH key derivation and then wrap key with AES) then the key is going to look like

{"alg":"ECDH-ES+A256KW","crv":"P-521","d":"APqo7hp2aa1gCJBaL-lUgfpl6TUh_jnrTWUwVr3JIjjbfH302cI7haiVCMauexenKYXmB7QEMlo-F6fXPPiaPVPn","key_ops":["wrapKey","unwrapKey"],"kty":"EC","x":"APNHmWpGhOeVqC0470Q6CCrpBPjd9EQ5sKedQwhXa1u2LlH4wrpXf20PVFVRNoIdJRagDGF_FpfZLY9lL1PYzxc-","y":"AV7wxzU-QSCbBs9N5iSxqYIsblivXvKKDvqwHMauhIadMgUSo3hz8TuEr_Aoa07IVroZWb1MBsQNe-7DxgwxQ5NT"}

and the message is going to be

{"ciphertext":"B_LmxZ0dpvXQ4W4uLvOUnw","encrypted_key":"enC-O1qIg5k1pQnlCn5YVbUoug-t_5ub2kgAwgAo94jjWmlhqe3WBx5ZkilRbBAdQo5Je4HywON1EBjB3Jafv-i5iCrYfxKZ","header":{"alg":"ECDH-ES+A256KW","epk":{"crv":"P-521","kty":"EC","x":"AcfOcKncapIK3UI0Iw1pbk4pvwpBCPt3zqf0-_IJGb9Q613_K7nyKX2w23vTldrJhJctgb3GtlwSHff8-0o4xpq0","y":"AMw51ccFh2XPrQN_11n7BuDq30GkejvHwcUKmMBF0rmaRM4Vp0GSBhUo0yJACOzvqmfvHC1wsm_3inofPAJKO0et"}},"iv":"P3dlfT0nZ3ZVlll_r9a-bw","protected":"eyJlbmMiOiJBMjU2Q0JDLUhTNTEyIn0","tag":"tblk7AGF8lw8jVmJUCbWREhA-u9SFbWgTemkpMSnDFM"}

[anatol]

And here is an updated example with ECDH-ES+A256KW algo:

package main

import (
    "crypto"
    "crypto/ecdsa"
    "encoding/base64"
    "log"

    "github.com/lestrrat-go/jwx/jwa"
    "github.com/lestrrat-go/jwx/jwe"
    "github.com/lestrrat-go/jwx/jwk"
)

func main() {
    // generate encrypted message with
    // jose jwk gen -i '{"alg":"ECDH-ES+A256KW"}' -o ec.jwk
    // jose jwk thp -i ec.jwk
    // echo -n hi | jose jwe enc -I- -k ec.jwk -o msg.jwe
    // jose jwe fmt -i msg.jwe -c

    webKeys, err := jwk.ParseString(`{"alg":"ECDH-ES+A256KW","crv":"P-521","d":"AcH8h_ctsMnopTiCH7wiuM-nAb1CNikC0ubcOZQDLYSVEw93h6_D57aD7DLWbjIsVNzn7Qq8P-kRiTYVoH5GTQVg","key_ops":["wrapKey","unwrapKey"],"kty":"EC","x":"AAQoEbNeiG3ExYj9bJLGFn4h_bFjERfIcmpQMW5KWlFhqcXTFg0g8-5YWjdJXdNmO_2EuaKe7zOvEq8dCFCb12-R","y":"Ad8E2jp6FSCSd8laERqIt67A2T-MIqQE5301jNYb5SMsCSV1rs1McyvhzHaclYcqTUptoA-rW5kNS9N5124XPHky"}`)
    if err != nil {
        log.Fatal(err)
    }
    webKey := webKeys.Keys[0]
    thumbprint, err := webKey.Thumbprint(crypto.SHA1)
    if err != nil {
        log.Fatal(err)
    }

    expectedThumbprint := "G4OtKQL_qr9Q57atNOU6SJnJxB8"
    if base64.RawURLEncoding.EncodeToString(thumbprint) != expectedThumbprint {
        log.Printf("incorrect key thumbprint, expected %v got %v", expectedThumbprint, base64.RawURLEncoding.EncodeToString(thumbprint))
    }

    var key ecdsa.PrivateKey
    err = webKey.Raw(&key)
    if err != nil {
        log.Fatal(err)
    }

    data := `{"ciphertext":"evXmzoQ5TWQvEXdpv9ZCBQ","encrypted_key":"ceVsjF-0LhziK75oHRC-C539hlFJMSbub015a3YtIBgCt7c0IRzkzwoOvo_Jf44FXZi0Vd-4fvDjRkZDzx9DcuDd4ASYDLvW","header":{"alg":"ECDH-ES+A256KW","epk":{"crv":"P-521","kty":"EC","x":"Aad7PFl9cct7WcfM3b_LNkhCHfCotW_nRuarX7GACDyyZkr2dd1g6r3rz-8r2-AyOGD9gc2nhrTEjVHT2W7eu65U","y":"Ab0Mj6BK8g3Fok6oyFlkvKOyquEVxeeJOlsyXKYBputPxFT5Gljr2FoJdViAxVspoSiw1K5oG1h59UBJgPWG4GQV"}},"iv":"KsJgq2xyzE1dZi2BM2xf5g","protected":"eyJlbmMiOiJBMjU2Q0JDLUhTNTEyIn0","tag":"b6m_nW9vfk6xJugm_-Uuj4cbAQh9ECelLc1ZBfO86L0"}`
    msg, err := jwe.ParseString(data)
    if err != nil {
        log.Fatal(err)
    }

    plaintext, err := msg.Decrypt(jwa.ECDH_ES, &key)
    if err != nil {
        log.Fatal(err)
    }

    expected := "hi"
    if string(plaintext) != expected {
        log.Fatalf("incorrect decrypted text, expected %v got %v", expected, string(plaintext))
    }
}

[lestrrat]

Oh wow, I haven't groked all of this yet, but this is what I get from working strictly out of the RFCs I swear there's no mention of this in the RFC :/ Thank you for digging into this, but this just means I have some catching up to do

[lestrrat]

Ah, no wonder I don't remember doing this stuff about plain ECDH_ES... I never implemented it yet. Okay, well, at least it's not a "bug" that ECDH-ES doesn't work, albeit that it lacks clear documentation and error reporting.

[lestrrat]

Okay, for the updated example, the same deal as before, no recipient error... will go and research

[lestrrat]

@anatol I think, per RFC, the jose tool generates an invalid JWE message. The RFC explicitly states that the "header" is a per-recipient element, and it should not appear directly in the main object.

   In summary, the syntax of a JWE using the general JWE JSON
   Serialization is as follows:

     {
      "protected":"<integrity-protected shared header contents>",
      "unprotected":<non-integrity-protected shared header contents>,
      "recipients":[
       {"header":<per-recipient unprotected header 1 contents>,
        "encrypted_key":"<encrypted key 1 contents>"},
       ...
       {"header":<per-recipient unprotected header N contents>,
        "encrypted_key":"<encrypted key N contents>"}],
      "aad":"<additional authenticated data contents>",
      "iv":"<initialization vector contents>",
      "ciphertext":"<ciphertext contents>",
      "tag":"<authentication tag contents>"
     }

[anatol]

@lestrrat it makes sense. I filed a ticket to jose project https://github.com/latchset/jose/issues/85

[lestrrat]

Yeah, I so my bad. back to coding...

[anatol]

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.

[npmccallum]

Implementation wise this seems straight forward, but so far I cannot find a reference that says that this padding is necessary. Part of me is nagging me to check if this is go-jose's definition of correct hashing, a defacto-standard, or part of a spec before matching this behavior.

RFC 7518 Section 6.2.1.2 says:

The length of this octet string MUST be the full size of a coordinate for the curve specified in the "crv" parameter. For example, if the value of "crv" is "P-521", the octet string must be 66 octets long.

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.

[npmccallum]

FYI, jose calculates the length of the encoded values here. We then pass that length to the encoding function here. This should zero-pad to the width of the field here. But I'm neither an infallible coder nor reader.

cc @sergio-correia

[anatol]

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.

[lestrrat]

fixed by #207, #208, #212