`jwt_get_alg` return `JWT_ALG_NONE` after `jwt_decode` call without key

[dafanasiev]

#include <stdio.h>

#include <jwt.h>

int main() {
    jwt_t* jwt;
    jwt_new(&jwt);

    // HS256 token with secret 123
    int ret = jwt_decode(&jwt, "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIn0.YW1752G5KvXQJ97MN3fzwLPhVYq2vONmBjQ1c5OEMag", NULL, 0);
    printf("jwt_decode result: %d\n", ret);   // returns 0 == no errors (decoded, not validated)

    jwt_alg_t alg = jwt_get_alg(jwt);
    printf("jwt_get_alg result: %d\n", (int)alg);      // returns 0 == JWT_ALG_NONE

    return 0;
}

I think that jwt_decode must fill jwt->alg in this case.

[benmcollins]

I believe this is obsoleted by:

JWT_EXPORT int jwt_decode_2(jwt_t **jwt, const char *token, jwt_key_p_t key_provider);

[benmcollins]

Apologies, wrong issue.

[mrryanjohnston]

I have run into some undefined behavior that is perhaps a result of this. Using the latest master branch:

#include <stdio.h>
#include <stdlib.h>
#include <jwt.h>
#include <string.h>
#include <time.h>

int main() {
    jwt_t *jwt = NULL;
    char *out = NULL;
    jwt_t *decoded_jwt = NULL;
    int ret;
    const char *secret = "YOUR_SECRET_KEY_HERE";

    // Create a new JWT object
    ret = jwt_new(&jwt);
    if (ret != 0) {
        fprintf(stderr, "Failed to create JWT object: %s\n", strerror(ret));
        return 1;
    }

    // Set the algorithm for the JWT
    jwt_set_alg(jwt, JWT_ALG_HS256, (const unsigned char *)secret, strlen(secret));

    // Add a custom grant
    jwt_add_grant(jwt, "foo", "bar");

    // Export the JWT as a string
    out = jwt_encode_str(jwt);
    if (!out) {
        fprintf(stderr, "Failed to encode JWT\n");
        jwt_free(jwt);
        return 1;
    }

    // Output the generated JWT string
    printf("Generated JWT: %s\n", out);

    // Decode the JWT
    // ret = jwt_decode(&decoded_jwt, out, (const unsigned char *)secret, strlen(secret));
    ret = jwt_decode(&decoded_jwt, out, NULL, 0);
    if (ret != 0) {
        fprintf(stderr, "Failed to decode JWT");
        free(out);
        jwt_free(jwt);
        return 1;
    }

    // Access and print the "foo" grant
    const char *foo = jwt_get_grant(decoded_jwt, "foo");
    if (foo) {
        printf("Claim foo: %s\n", foo); // Should print "bar"
    } else {
        fprintf(stderr, "Claim 'foo' not found\n");
    }

    // Clean up
    free(out);
    jwt_free(jwt);
    jwt_free(decoded_jwt);

    return 0;
}

Running this produces:

Generated JWT: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJmb28iOiJiYXIifQ.S_fQAxWX7eCbvrBlN0wfbxmXoZxFL9EWaeJGj5gj4yE
Claim foo: bar

I believe this happens because, when we do json_decode, we overwrite (*jwt)->headers with a fresh json_object due to a call to jwt_parse in jwt_decode which calls jwt_new: https://github.com/benmcollins/libjwt/blob/master/libjwt/jwt.c#L663 and https://github.com/benmcollins/libjwt/blob/master/libjwt/jwt.c#L619

[benmcollins]

Can you give me insight on what you're expecting to happen vs. what is actually happening?

[mrryanjohnston]

Expected: jwt_decode should correctly set decoded_jwt->alg to jwt->alg based on the alg header in the encoded jwt string out. This was specified by jwt_set_alg(jwt.

Actual: jwt_decode discards the alg header when it calls jwt_parse because jwt_parse calls jwt_new before it calls jwt_parse_head which then sets ->alg from the "new" jwt whcih has a blank alg header property: https://github.com/benmcollins/libjwt/blob/master/libjwt/jwt.c#L557-L559

[benmcollins]

It's not apparent in your example code where this is happening. One thing I can say is that result of jwt_decode() is not meant to be reused to create a new JWT without first calling jwt_set_alt() on the result. The alg and secrets are thrown away on purpose to preserve cryptographic cleanliness. One would expect that since you decoded it, you already know the ALG since you had to know it to decode it.

[mrryanjohnston]

It's not apparent in your example code where this is happening. One thing I can say is that result of jwt_decode() is not meant to be reused to create a new JWT without first calling jwt_set_alt() on the result. The alg and secrets are thrown away on purpose to preserve cryptographic cleanliness. One would expect that since you decoded it, you already know the ALG since you had to know it to decode it.

It is not guaranteed that jwts being sent to a server were produced by that server.

Additionally, running jwt_encode_str on the decoded jwt produces a different jwt string. For reference, the string generated with the alg header set is eyJhbGciOiJIUzI1NiIsImZvbyI6ImJhciIsInR5cCI6IkpXVCJ9.e30.d9cuDlBG72AqPLBKBYNku2CHMNMD3iany8FAhXYB3Bg, while the jwt string without the alg header is eyJhbGciOiJub25lIiwidHlwIjoiSldUIn0.eyJmb28iOiJiYXIifQ.. If this is expected behavior, it is important to note for users using libjwt that decoding/encoding jwts is not an idempotent process.

Finally, it is not a guarantee that a program generating jwts with libjwt will only ever issue jwts with one algorithm.

Edit: Additionally, this occurs in jwt_parse when we first run jwt_new, then we attempt to jwt_parse_head on the "new" jwt https://github.com/benmcollins/libjwt/blob/master/libjwt/jwt.c#L619-L624

jwt_parse_head attempts to set jwt->alg https://github.com/benmcollins/libjwt/blob/master/libjwt/jwt.c#L558, but it tries to set it from the headers property of the "new" jwt: https://github.com/benmcollins/libjwt/blob/master/libjwt/jwt.c#L557

[benmcollins]

First off, encoded strings are not guaranteed to be the same. This requires sorting all of the claims and other fields in the JWT, which is neither required nor needed by the JWT spec. Relying on this to be guaranteed is broken. Even if you sort the grants and header fields, some algorithms do not produce the same result even for the same input.

I'm still not understanding your workflow here enough to know how to help you with this, but maybe this function can help:

/**
 * Like jwt_decode(), but the key will be obtained via the key provider.
 * Key providers may use all sorts of key management techniques, e.g.
 * can check the "kid" header parameter or download the key pointed to
 * in "x5u"
 *
 * @param jwt Pointer to a JWT object pointer. Will be allocated on
 *     success.
 * @param token Pointer to a valid JWT string, null terminated.
 * @param key_provider Pointer to a function that will obtain the key for the given JWT.
 *      Returns 0 on success or any other value on failure.
 *      In the case of an error, the same error value will be returned to the caller.
 * @return 0 on success, valid errno otherwise.
 *
 * @remark See jwt_decode()
 */
JWT_EXPORT int jwt_decode_2(jwt_t **jwt, const char *token, jwt_key_p_t key_provider);

It will allow you to inspect the jwt_t (e.g. the algorithm) and determine what key needs to be used to validate the token.

[mrryanjohnston]

It appears that RFC 7519 specifies that the header "alg" is used to specify the algorithm used: https://datatracker.ietf.org/doc/html/rfc7519#section-3.1

Are you sure it makes sense not to set the alg property on the decoded jwt_t from the encoded jwt string? This would mean libjwt loses data integrity.

Edit: As a workaround, code using libjwt to decode jwts could conceivably do jwt->alg = jwt_str_alg(jwt_get_header(decoded_jwt, "alg")) after they run jwt_decode.

Edit edit: Consider also that "downstream" code shouldn't need to know whether a jwt_t was produced using jwt_new+jwt_set_alg vs json_decode.

[mrryanjohnston]

I have found where this happens. Inside of jwt_verify_head, we run jwt_scrub_head (which sets the alg to none https://github.com/benmcollins/libjwt/blob/master/libjwt/jwt.c#L191) if key is not present on the jwt_t. Since we are decoding the jwt, the key will not be present on the object. https://github.com/benmcollins/libjwt/blob/master/libjwt/jwt.c#L574

[benmcollins]

I've explained that the ALG is not set unless there is a key with it. We do not store the key in a decoded jwt_t because that would mean we need to copy the secret, and could possibly leak cryptographic information.

Can you explain a workflow that would need this to be kept in the jwt_t after decoding? If not, I can't help you out.

[benmcollins]

It appears that RFC 7519 specifies that the header "alg" is used to specify the algorithm used:

That's correct. HOWEVER, a jwt_t is NOT a JWT. It is an abstract object used to store data until a JWT is created. You can't apply RFC requirements to an abstract programming object.

[mrryanjohnston]

I understand. Thank you for your time and for providing this library. One final thought: since you only ever use jwt_verify_head in the two decode functions, there will never be a key in the jwt header. Thus, you could potentially remove that if inside of the if as well as the else in jwt_verify_head so that it looks like this:

static int jwt_verify_head(jwt_t *jwt)
{
	int ret = 0;

	if (jwt->alg != JWT_ALG_NONE) {
			jwt_scrub_key(jwt);
        }

	return ret;
}