nov / json-jwt

JSON Web Token and its family (JSON Web Signature, JSON Web Encryption and JSON Web Key) in Ruby
MIT License
299 stars 81 forks source link

Serialization of valid JW* not always valid #97

Closed jphastings closed 2 years ago

jphastings commented 2 years ago

JSON supports optional escaping of characters which means that there are multiple representations of the same decoded object.

When JSON::JWT#to_s is called (similarly JSON:JWE#to_s and others) the JSON is re-serialized using ruby's default JSON encoding.

Because re-serializing the JWT is never guaranteed to be bit-perfect identical to the original serialized JSON, but that is depended upon for the signature to be accurate, I recommend that the decode_compact_serialized methods also keep track of the original JWT string, and return it in place of the generated versions if present/appropriate.

Trivial example

jwt = 'e30=.eyJpc3MiOiJodHRwczpcL1wvZXhhbXBsZS5jb21cLyJ9.whatever'
obj = JSON::JWT.decode(jwt, :skip_verification)
obj.to_s
#     "e30.eyJpc3MiOiJodHRwczovL2V4YW1wbGUuY29tLyJ9.whatever"
obj.to_s == jwt
# false

puts Base64.decode64(jwt.split('.')[1])
# {"iss":"https:\/\/example.com\/"}
puts Base64.decode64(obj.to_s.split('.')[1])
# {"iss":"https://example.com/"}
obj.to_h
# {"iss"=>"https://example.com/"}
nov commented 2 years ago

what is your problem? you can verify the signature against the original octets with current implementation.

jphastings commented 2 years ago

Apologies; the problem is calling to_s on a JSON::JWT that has been decoded from a string produced by a different library (with different JSON serialisation options) will return an erroneously invalid JWT string.

Here is an example with a full JWT:

original = "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzI1NiJ9.eyJpc3MiOiJodHRwczpcL1wvZXhhbXBsZS5jb21cLyIsImV4cCI6MTY0MDYwMTY1OCwibmJmIjoxNjM5OTk2ODU4fQ.TlLhxMB8uo-HasSVfEkyJ7UcNBqq1Er8YJ5chkbHQgSjY8AiK-SjQYHlzl_Bbns6LApQZvw6zTKphsa4YM866Ag_Nc4_HZ4mjHjRGDIPlzgq72MA3WUiBzlo8ES0y3PaB6sXCEdd_G18lNXXYWV3WPnRWka4vPWmvIOW8pdtNvn4naLfROY8gPpulxLYx_KYyXk01U71f_rPlOA0R6456SW-V1Aw8U4rBbDkjrfepVNOLBoCOWt-F_qcAUz77fCXxCnydf90_RF2Lz558v6Ux9dc3w2OOT3vmn9PuQcZH6SdwYCHVVn3CxReaMjXvdtjSSOsBSPlq-Pj2y_an74EWQ"

public_key = OpenSSL::PKey::RSA.new <<-PEM
-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtj5hknUzY1cqcJ+5wlas
mxUDLNxiMbO90gx6C24RahuutS3XwYQ98eTnbCUQHcYQS/RGdGPm0BNPBnJZm4SQ
CrUJinDwAso8d7So97sItRcacNSnLl3kXepDWrUBZdAWikaS9khU9EEV+E0cEIVI
PsQrz5u0RC1bwSi7EdxrhZr9PiQxN7tQGDzZa50P3OtllCFR9Fj2uNBASbpdlR7s
DDI0Hiv24PjqOXvAeM7CrDMjKQeV4N9Irit3iKQh34ouX9oN0PODswsBzEsiOBHM
zWRewTPww3h4yp5741tOj7ZpO62Rg2wDyJinfxAW45g0t5sMzQgH7RAMH6XqOOew
HQIDAQAB
-----END PUBLIC KEY-----
PEM

# Verifies & decodes correctly:
p jwt_obj = JSON::JWT.decode(original, public_key)
# => {"iss"=>"https://example.com/", "exp"=>1640601658, "nbf"=>1639996858}

# But the string-rendering from this library's object form is not consistent with the given input
p original == jwt_obj.to_s
# => false

# Which means it is no longer valid
p JSON::JWT.decode(jwt_obj.to_s, public_key)
# => JSON::JWS::VerificationFailed (JSON::JWS::VerificationFailed)

I absolutely can use the original octets, but the implication of JSON::JWT#to_s is that it creates a valid JWT. In this scenario it does not.

nov commented 2 years ago

I don't understand why you need second decoding & verification step nor why you need to call to_s there. to_s is for generating JWT from json object, not for keeping original JWT string.

jphastings commented 2 years ago

Before I delved into the code here I assumed that a JSON::JWT could represent both a parsed JWT or a generated JWT, and that the output of to_s wouldn't be dependent on where the JWT had come from.

One of the libraries I work on uses the JSON::JWT object to hold the information about a received JWT as it's passed through other logic in the library — this so we don't have to send the raw string instead, and make the 3 different call sites perform JSON::JWT.decode to access assertions and JSON::JWT's other helpful methods.

Because JSON::JWT.decode(jwt_string).to_s != jwt_string in every case (or any cases with JWTs generated by the library we use in Scala) we now need to have method arguments like: def do_thing(raw_jwt, parsed_jwt) so that when the JWT needs to be passed on, or wrapped (for delegated authentication), we have the original json encoding (which matches the signature).

Would you be interested in my proposing a change to address this?