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

fix(claims): deser exact integer timestamp values from claims #106

Closed
wants to merge 1 commit into from

Conversation

ocn
Copy link

@ocn ocn commented Feb 19, 2024

When deserializing JWT claims from JSON inputs, the hardcoded values for issued_at, expires_at, invalid_before, etc. are bit-shifted to values which do not match the input. This PR uses UnixTimestamp methods which passthrough the values from the deserializer's input so that the resultant claims match the expected values from the input.

When deserializing JWT claims from JSON inputs, the hardcoded values for issued_at, expires_at, invalid_before, etc. are bit-shifted to values which do not match the input. This PR uses UnixTimestamp methods which passthrough the values from the deserializer's input so that the resultant claims match the expected values from the input.
@ocn
Copy link
Author

ocn commented Feb 19, 2024

e.g. when deserializing the following input claims, the resultant issued_at time becomes 858993459200 due to the internal bit-shifting, rather than the desired test data.

    pub const TEST_CLAIMS: &str = r#"
    {
        "name": "Ada Lovelace",
        "iss": "https://chronogears.com/test",
        "aud": "test",
        "auth_time": 100,
        "user_id": "uid123",
        "sub": "sbu123",
        "iat": 200,
        "exp": 500,
        "nbf": 300,
        "email": "alovelace@chronogears.com"
    }"#;

    #[derive(Serialize, Deserialize, Debug)]
    pub struct CustomClaims {
        auth_time: i64,
        name: String,
        user_id: String,
        email: String,
    }

    #[test]
    fn my_cool_fn() {
        let claims: JWTClaims<CustomClaims> = serde_json::from_str(TEST_CLAIMS).unwrap();
        assert_eq!(claims.issued_at.unwrap(), 858993459200);
    }

With these changes, the claims are deserialized precisely to match the input data.

If there is some non-obvious reason why this should not be done and the input should in fact be bit-shifted, I would appreciate any insight you can share as to the reason why that is so.

@ocn ocn changed the title fix(claims): deser exact values from claims fix(claims): deser integer timestamp exact values from claims Feb 19, 2024
@ocn ocn changed the title fix(claims): deser integer timestamp exact values from claims fix(claims): deser exact integer timestamp values from claims Feb 19, 2024
@jedisct1
Copy link
Owner

    assert_eq!(claims.issued_at.unwrap(), 858993459200);

That should not even compile. issued.at is a Duration value, so if you want to get it as a number of seconds, use .as_secs():

    let claims: JWTClaims<NoCustomClaims> = serde_json::from_str(json).unwrap();
    assert_eq!(claims.issued_at.unwrap().as_secs(), 200);

@jedisct1 jedisct1 closed this in 12a73be Feb 19, 2024
@ocn
Copy link
Author

ocn commented Feb 20, 2024

That example will compile with an .into() call appended, which I accidentally omitted from my comment.

The issue is not that the deserialization does not directly store the input-values, but that JWTs created with the claims provided above cannot be verified due to "clock drift".

The following JWT and RS256 public-key pair cannot be verified, even though they are valid. I have provided a reproducible example below, with the imports omitted.

#[test_log::test]
fn should_verify() {
    #[derive(Serialize, Deserialize, Debug)]
    pub struct CustomClaims {
        auth_time: i64,
        name: String,
        user_id: String,
        email: String,
    }
    let key = RS256PublicKey::from_pem(r#"-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAt5N44H1mpb5Wlx/0e7Cd
oKTY8xt+3yMby8BgNdagVNkeCkZ4pRbmQXRWNC7qn//Zaxx9dnzHbzGCul5W0RLf
d3oB3PESwsrQh+oiXVEPTYhvUPQkX0vBfCXJtg/zY2mY1DxKOIiXnZ8PaK/7Sx0a
MmvR//0Yy2a5dIAWCmjPsxn+PcGZOkVUm+D5bH1+ZStcA/68r4ZSPix7Szhgl1Ro
Hb9Q6JSekyZqM0Qfwhgb7srZVXC/9/m5PEx9wMVNYpYJBrXhD5IQm9RzE9oJS8T+
Ai+4/5mNTNXI8f1rrYgffWS4wf9cvsEihrvEg9867B2f98L7ux9Llle7jsHCtwgV
1wIDAQAB
-----END PUBLIC KEY-----"#).unwrap();
    let jwt = "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6IjEifQ.eyJuYW1lIjoiQWRhIExvdmVsYWNlIiwiaXNzIjoiaHR0cHM6Ly9jaHJvbm9nZWFycy5jb20vdGVzdCIsImF1ZCI6InRlc3QiLCJhdXRoX3RpbWUiOjEwMCwidXNlcl9pZCI6InVpZDEyMyIsInN1YiI6InNidTEyMyIsImlhdCI6MjAwLCJleHAiOjUwMCwibmJmIjozMDAsImVtYWlsIjoiYWxvdmVsYWNlQGNocm9ub2dlYXJzLmNvbSJ9.eTQnwXrri_uY55fS4IygseBzzbosDM1hP153EZXzNlLH5s29kdlGt2mL_KIjYmQa8hmptt9RwKJHBtw6l4KFHvIcuif86Ix-iI2fCpqNnKyGZfgERV51NXk1THkgWj0GQB6X5cvOoFIdHa9XvgPl_rVmzXSUYDgkhd2t01FOjQeeT6OL2d9KdlQHJqAsvvKVc3wnaYYoSqv2z0IluvK93Tk1dUBU2yWXH34nX3GAVGvIoFoNRiiFfZwFlnz78G0b2fQV7B5g5F8XlNRdD1xmVZXU8X2-xh9LqRpnEakdhecciFHg0u6AyC4c00rlo_HBb69wlXajQ3R4y26Kpxn7HA";
    let verification = VerificationOptions {
        allowed_issuers: Some(HashSet::from(["https://chronogears.com/test".to_owned()])),
        artificial_time: Some(UnixTimeStamp::from_ticks(400)),
        time_tolerance: Some(coarsetime::Duration::from_secs(0)),
        ..Default::default()
    };
    key.verify_token::<CustomClaims>(jwt, Some(verification)).unwrap();
}

This results in an error/panic with the following message:

called `Result::unwrap()` on an `Err` value: Clock drift detected

When I use the changes I submitted in this PR, it does correctly deserialize the values as they are in the input data, which allows for the verification to proceed as expected.

@jedisct1
Copy link
Owner

You're trying to verify a token that was issued in year 1970.

UnixTimeStamp::from_ticks(400)

If 400 is a number of seconds, use from _secs().

ticks is just an internal, unstable, undocumented, non-portable representation.

jedisct1 added a commit that referenced this pull request Feb 20, 2024
jspspike pushed a commit to jspspike/rust-jwt-simple that referenced this pull request Apr 16, 2024
jspspike pushed a commit to jspspike/rust-jwt-simple that referenced this pull request Apr 16, 2024
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

Successfully merging this pull request may close these issues.

2 participants