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

decode() should not require a key? #3

Closed
FGasper opened this issue Apr 26, 2016 · 14 comments
Closed

decode() should not require a key? #3

FGasper opened this issue Apr 26, 2016 · 14 comments

Comments

@FGasper
Copy link
Contributor

FGasper commented Apr 26, 2016

If I have the following JWT: eyJqd2siOnsiZSI6IkFRQUIiLCJuIjoieVFJdnpEU3h2a2EzQTNhVFYzS2Yza29PeElWMjNqZGlaa1BkOU8xb3RsN0JYLWZJS2dEYk00QnBHSkxZLUhrTG5aZUxpcXgwSFpKaF94U09IVXhWNnVpLUpIU00yZkFrTnEzMHd4QzMycDZmVDk2b3RuT3ZsTEhPTVVpNEZwUFR0NDVFQmcyemlqRXRfRWNFM3g0OFJjT2ZQVGk3SDBmWnhBdXVYcmJrYmU1SHFqczVxVWp2bDFKWUdKdTA1TlItdnE2NUwyUC1oOFA5eUJBT1pRZjhRMVhBSGg1RlFQd08tQjZ3T1p6aTNjeTEtRUhXZkhpWXpxeTMxWU01ZmxIaFZ4QndWRmUyMUlINEh3WWp2SE5KMURFaEl2R2FSQTBWc09ZNlFqVUxPS19XTVlQVnExc211TmdEZThlZ1V4RnV2R2N4aWJ4NTUydHJkSHVBaWFUVGlRIiwia3R5IjoiUlNBIn0sImFsZyI6IlJTMjU2Iiwibm9uY2UiOm51bGx9.eyJjb250YWN0IjpbIm1haWx0bzpmQGcudGxkIl0sInJlc291cmNlIjoibmV3LXJlZyJ9.wrY6y0kvA3qgR38ZuAA471ygN9fmSHdfWDIayjkBKGmeGbn0f30_LQBC9FiFDFgFJ8Owyy3bOkPWvHx7yhRnP5XnEYdzNtKy4t2LyKq_JSEVQf6p1zycsVaxVLCmZ6ZbRidxIFLhbkcmu2uc4BEVGQQEj3UesccIv-AS2JCQFqK5ca-HQeaLEMntXOz5p7DYZtauYjHuXQ60i25mClm51jScJfP-wk7yYnnohGYKDinwiYlH4Nw8p4yElzWL1dI-U8fiFoxnduGaflPIZ80hkk0p7delrZt3RvmaDdu4cLJ16TgrMw_nMZfbAK0IJXByAsbej78HwIAchdzHyRPmgA

… and I put that into http://jwt.io, it parses into something useful.

I don’t see anything in this module that does this … am I missing something? (JSON::WebToken has the same issue.)

@karel-m
Copy link
Contributor

karel-m commented Apr 27, 2016

it has has a key in its header

{"jwk":{"e":"AQAB","n":"yQIvzDSxvka3A3aTV3Kf3koOxIV23jdiZkPd9O1otl7BX-fIKgDbM4BpGJLY-HkLnZeLiqx0HZJh_xSOHUxV6ui-JHSM2fAkNq30wxC32p6fT96otnOvlLHOMUi4FpPTt45EBg2zijEt_EcE3x48RcOfPTi7H0fZxAuuXrbkbe5Hqjs5qUjvl1JYGJu05NR-vq65L2P-h8P9yBAOZQf8Q1XAHh5FQPwO-B6wOZzi3cy1-EHWfHiYzqy31YM5flHhVxBwVFe21IH4HwYjvHNJ1DEhIvGaRA0VsOY6QjULOK_WMYPVq1smuNgDe8egUxFuvGcxibx552trdHuAiaTTiQ",
"kty":"RSA"}

If you find a RFC which says that jwk field from header should be used as a key I can implement it. I only do not know if it is implied by some standard,

@FGasper
Copy link
Contributor Author

FGasper commented Apr 27, 2016

I don’t meant to use the key.

I mean just a simple parse without requiring the key--the same as jwt.io is doing.

Right now, if there were no http://jwt.io, would Crypt::JWT be able to do anything with that JWT without its key?

I’m working on an ACME (Let’s Encrypt) client, and that service uses public keys as registration. The key is sent as in that JWT; the server parses it and extracts the key as JWK. This parse happens without first having the key.

-FG

On 27 Apr 2016, at 5:09 AM, karel-m notifications@github.com wrote:

it has has a key in its header

{"jwk":{"e":"AQAB","n":"yQIvzDSxvka3A3aTV3Kf3koOxIV23jdiZkPd9O1otl7BX-fIKgDbM4BpGJLY-HkLnZeLiqx0HZJh_xSOHUxV6ui-JHSM2fAkNq30wxC32p6fT96otnOvlLHOMUi4FpPTt45EBg2zijEt_EcE3x48RcOfPTi7H0fZxAuuXrbkbe5Hqjs5qUjvl1JYGJu05NR-vq65L2P-h8P9yBAOZQf8Q1XAHh5FQPwO-B6wOZzi3cy1-EHWfHiYzqy31YM5flHhVxBwVFe21IH4HwYjvHNJ1DEhIvGaRA0VsOY6QjULOK_WMYPVq1smuNgDe8egUxFuvGcxibx552trdHuAiaTTiQ",
"kty":"RSA"}

If you find a RFC which says that jwk field from header should be used as a key I can implement it. I only do not know if it is implied by some standard,


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@karel-m
Copy link
Contributor

karel-m commented Apr 27, 2016

According to https://tools.ietf.org/html/rfc7515#section-4.1.3

jwk value from header (if present) should be used as a key for signature validation. Which Crypt::JWT does not follow, so it is a bug worth fixing

@FGasper
Copy link
Contributor Author

FGasper commented Apr 27, 2016

Ah, there it is! :)

-FG

On 27 Apr 2016, at 7:55 AM, karel-m notifications@github.com wrote:

According to https://tools.ietf.org/html/rfc7515#section-4.1.3

jwk value from header (if present) should be used as a key for signature validation. Which Crypt::JWT does not follow, so it is a bug worth fixing


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@FGasper
Copy link
Contributor Author

FGasper commented Apr 27, 2016

I’m sorry I’m not being more prompt about testing changes … I literally am working nonstop currently, managing a large number of pressing obligations at work and outside.

Hopefully next week things will lighten up a bit.

Do you need tests/PR from me prior to releasing versions of Crypt::JWT and CryptX that incorporate the changes we’ve been discussing (thumbprints, etc.)?

-FG

On 27 Apr 2016, at 7:55 AM, karel-m notifications@github.com wrote:

According to https://tools.ietf.org/html/rfc7515#section-4.1.3

jwk value from header (if present) should be used as a key for signature validation. Which Crypt::JWT does not follow, so it is a bug worth fixing


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@karel-m
Copy link
Contributor

karel-m commented Apr 27, 2016

initial support for using jwk header value for verification 5af5f32 (only for JWS decoding, only works with RSA/ECC, no tests, no doc)

@karel-m
Copy link
Contributor

karel-m commented Apr 27, 2016

Ad tests/PR - yes it will speed up the release if you can help me by submitting some tests & doc fixes.

I will add a comment what I would like to have to each individual issue.

@FGasper
Copy link
Contributor Author

FGasper commented Apr 27, 2016

I’ll see what I can do. These are directly useful in the project I’m doing at work, so I may actually be able to swing it as part of normal work day stuff.

Thank you!

-FG

On 27 Apr 2016, at 8:08 AM, karel-m notifications@github.com wrote:

Ad tests/PR - yes it will speed up the release if you can help me by submitting some tests & doc fixes.

I will add a comment what I would like to have to each individual issue.


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub

@FGasper
Copy link
Contributor Author

FGasper commented Apr 27, 2016

#4 @karel-m

@karel-m
Copy link
Contributor

karel-m commented May 1, 2016

now fixed in master

@karel-m karel-m closed this as completed May 1, 2016
@karel-m
Copy link
Contributor

karel-m commented Sep 2, 2018

@FGasper just FYI:

since v0.23 you have to to explicitly set newly added param key_from_jwk_header to use the key from jwk token header for signature verification:

my ($header, $payload) = decode_jwt(token => $jws, decode_header => 1, key_from_jwk_header => 1);

@FGasper
Copy link
Contributor Author

FGasper commented Sep 2, 2018

Just to clarify: is that a breaking change?

@karel-m
Copy link
Contributor

karel-m commented Sep 2, 2018

Yes, it is a backwards incompatible change. Now you have to explicitly say that you want to use jwk value from token header as the signature validation key (by key_from_jwk_header => 1).

The old behaviour had some nasty security implications.

@FGasper
Copy link
Contributor Author

FGasper commented Sep 2, 2018

Are these documented somewhere? They may be relevant to some other work I’ve done.

Anyway, thank you for the heads-up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants