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

[Bug]: JWK thumbprint includes optional members when calculated #7

Closed
1 task done
trew opened this issue Nov 16, 2023 · 1 comment · Fixed by #11
Closed
1 task done

[Bug]: JWK thumbprint includes optional members when calculated #7

trew opened this issue Nov 16, 2023 · 1 comment · Fixed by #11
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@trew
Copy link

trew commented Nov 16, 2023

Contact Details

No response

Describe the bug

When a DPoP is parsed, the JWK thumbprint should only include required members when calculated. See https://datatracker.ietf.org/doc/html/rfc7638#section-3.2

Related lines of code:

go-dpop/parse.go

Lines 124 to 137 in e3feea2

// Extract the public key from the proof and hash it.
// This is done in order to store the public key
// without the need for extracting and hashing it again.
jwkHeaderJSON, err := json.Marshal(dpopToken.Header["jwk"])
if err != nil {
// keyFunc used with parseWithClaims should ensure that this can not happen but better safe than sorry.
return nil, errors.Join(ErrInvalidProof, err)
}
h := sha256.New()
_, err = h.Write([]byte(jwkHeaderJSON))
if err != nil {
return nil, errors.Join(ErrInvalidProof, err)
}
b64URLjwkHash := base64.RawURLEncoding.EncodeToString(h.Sum(nil))

Steps to reproduce the behavior

Create a proof with a JWK containing optional fields, for example:

{
  "crv": "P-256",
  "ext": true,
  "kty": "EC",
  "x": "KDVM3aXrYGUgmKyo0q__DdIDVS0AV139ZTba04SKqFQ",
  "y": "SC-oR4qLdhmwrjr6PjJ7_LfFpC_BSFUT14LnMtgjHPs"
}

In this example, "ext" should be excluded.

OS

No response

Relevant log output

No response

Additional Notes

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct
@trew trew added the bug Something isn't working label Nov 16, 2023
@SalladinBalwer SalladinBalwer added the good first issue Good for newcomers label Nov 17, 2023
@SalladinBalwer
Copy link
Contributor

This is indeed a bug and should be fixed.
I will look at it when I have time unless any one else claims it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants