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

b64 InvalidCastException #121

Open
fdres opened this issue Nov 28, 2019 · 9 comments
Open

b64 InvalidCastException #121

fdres opened this issue Nov 28, 2019 · 9 comments

Comments

@fdres
Copy link

fdres commented Nov 28, 2019

Hi.
Inside private static byte[] DecodeBytes(...) in JWT.cs there is this piece of code:
object value; if (headerData.TryGetValue("b64", out value)) { b64 = (bool) value; }

If value is "false" or "true" this will throw an InvalidCastException.
Would you consider the change to
b64 = Convert.ToBoolean(value);

@dvsekhvalnov
Copy link
Owner

Sure can absolutely do it.

Are you facing this issue in real world? Mind unit test or sample data?

I also happy to accept PR :)

@fdres
Copy link
Author

fdres commented Nov 29, 2019

Thanks for your response.
I'm using the nuget in an ASP.NET Core API for decoding.
I use postman as a client with javascript pre-request script to sign the request body

Client Code Sample
var oHeader = { "alg": "PS256", "typ": "JOSE", "cty": "application/json", "b64": "true" }; var sHeader = JSON.stringify(oHeader); var token = KJUR.jws.JWS.sign("PS256", sHeader, sPayload, privateKey);

@dvsekhvalnov
Copy link
Owner

@fdres thanks for example.

May i ask why you putting string value for b64 header instead of boolean?

RFC7797 pretty clear defines (https://tools.ietf.org/html/rfc7797#section-3):

The "b64" value is a JSON boolean, with a default value of "true".

Obviously i don't mind to add fix to support string values as well if it helps in real world. Just trying to understand what is real use case for it?

@fdres
Copy link
Author

fdres commented Dec 2, 2019

I started using the nuget in version 2.4.0. When I sent the b64 header as boolean I got an IntegrityException with the message "Invalid Signature". When I changed the value to a string it validated the payload.
I don't know if the problem with the boolean value is with the js framework I'm using to sign the request ( I am not a javascript developer and I was searching for something to use with postman).

@dvsekhvalnov
Copy link
Owner

I see. Are you using https://kjur.github.io/jsrsasign/ ?

Probably i need to craft some example.

@fdres
Copy link
Author

fdres commented Dec 3, 2019

"Are you using https://kjur.github.io/jsrsasign/ ?"
Yes

@dvsekhvalnov
Copy link
Owner

dvsekhvalnov commented Dec 3, 2019

@fdres i'm not super sure but i can't find that jsrsasign supports unencoded payload.
It seems to match what you are observing:

  • when you provide boolean b64 value, jsrsasign not handling it and still applies base64 encoding for payload. On verification part if it obviously fails as jose-jwt expects unencoded payload

Pretty simple to verify, just generate 2 tokens via jsrsasign: one with b64=true, another with b64=false and post here or compare yourself. Pretty sure you'll get identical payload part within.

@fdres
Copy link
Author

fdres commented Dec 5, 2019

@dvsekhvalnov
That's true. jsrasign does not support unencoded payload (yet?).
My original issue was that I sent a b64 header with value "true" and it couldn't be parsed. That caused my api to throw a 500 http status code.
The problem I was having with jose-jwt and lead me to send string value instead of boolean does not seem to occur any more (probably I was misusing the library).
Anyway. I think is a good practice to either try parse any value sent at the header, or throw a proper exception.
Thanks for your time

@dvsekhvalnov
Copy link
Owner

Yeah, for sure i agree we should improve error handling, InvalidCastException is terrible )

I'm slightly disagree with accepting "string" values for b64 header as it against spec and no real use case.

Will publish fix in next minor version.

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