jwt: add functions to parse a token #75924

blemouzy · 2024-07-16T08:48:07Z

Add a new function jwt_parse_payload to parse token payload
Add a new function jwt_verify to verify token header and signature

d3zd3z

This is largely reasonable, but the strn* usages are not correct.

d3zd3z · 2024-10-04T20:44:11Z

subsys/jwt/jwt.c

+	}
+	builder_header = builder.base;
+	builder_header_len = builder.buf - builder.base;
+	res = strncmp(parser->header, builder_header, builder_header_len);


This is not correct, the strncmp will only check that the builder_header matches a prefix of parser->header. Is there a concern that the builder somehow didn't get a terminating nul?

Frankly, in this case, just use strcmp, since you are controlling the builder, and the buffer should always have the null termination.

Is there a concern that the builder somehow didn't get a terminating nul?

Yes, there is.

jwt_init_parser uses strchr to find payload and signature positions in the token but . (dot separators) are not overwrittent by \0 to avoid modifying the original jwt input string.

So in tests/subsys/jwt/src/main.c example:

jwt = "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJpb3Qtd29yay0xOTk0MTkiLCJleHAiOjE1MzAzMTIwMjYsImlhdCI6MTUzMDMwODQyNn0.VHKDT-dvWW3UDBGokkttP7P6DNDnd4jjz_wLsOvn9RVBoDAAVG1mQnlROPtOHyRTotWzFYf2g02-VyZoh326yDFXM-66U7ycqyUsJdWBZWYqub6iZ17l0hjodoSpZBJqPB6F9c8Q5u9x1gWVw2l2XNASQyTDIZ3db_GiQbx1SYnNtyxsospoKClIebee-EQkcut9jBPNyiKrzZRPuX0xCiPJYTumHdHSp0UPIDIij2hJl_tacXs-hz_ckqT5HZvJPsxgJCeXz3tP7IlgbeGbPFMFmaYNX4UxYD4XS10MPXUK4_qp8wK4EqjT2YBNF7xXi_sjWMj7isc0i0YSIl6_nQ" | | | header payload sign parser->header = "eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCJ9.eyJhdWQiOiJpb3Qtd29yay0xOTk0MTkiLCJleHAiOjE1MzAzMTIwMjYsImlhdCI6MTUzMDMwODQyNn0.VHKDT-dvWW3UDBGokkttP7P6DNDnd4jjz_wLsOvn9RVBoDAAVG1mQnlROPtOHyRTotWzFYf2g02-VyZoh326yDFXM-66U7ycqyUsJdWBZWYqub6iZ17l0hjodoSpZBJqPB6F9c8Q5u9x1gWVw2l2XNASQyTDIZ3db_GiQbx1SYnNtyxsospoKClIebee-EQkcut9jBPNyiKrzZRPuX0xCiPJYTumHdHSp0UPIDIij2hJl_tacXs-hz_ckqT5HZvJPsxgJCeXz3tP7IlgbeGbPFMFmaYNX4UxYD4XS10MPXUK4_qp8wK4EqjT2YBNF7xXi_sjWMj7isc0i0YSIl6_nQ" parser->header_len = 36 parser->payload = "eyJhdWQiOiJpb3Qtd29yay0xOTk0MTkiLCJleHAiOjE1MzAzMTIwMjYsImlhdCI6MTUzMDMwODQyNn0.VHKDT-dvWW3UDBGokkttP7P6DNDnd4jjz_wLsOvn9RVBoDAAVG1mQnlROPtOHyRTotWzFYf2g02-VyZoh326yDFXM-66U7ycqyUsJdWBZWYqub6iZ17l0hjodoSpZBJqPB6F9c8Q5u9x1gWVw2l2XNASQyTDIZ3db_GiQbx1SYnNtyxsospoKClIebee-EQkcut9jBPNyiKrzZRPuX0xCiPJYTumHdHSp0UPIDIij2hJl_tacXs-hz_ckqT5HZvJPsxgJCeXz3tP7IlgbeGbPFMFmaYNX4UxYD4XS10MPXUK4_qp8wK4EqjT2YBNF7xXi_sjWMj7isc0i0YSIl6_nQ" parser->payload_len = 79 parser->sign = "VHKDT-dvWW3UDBGokkttP7P6DNDnd4jjz_wLsOvn9RVBoDAAVG1mQnlROPtOHyRTotWzFYf2g02-VyZoh326yDFXM-66U7ycqyUsJdWBZWYqub6iZ17l0hjodoSpZBJqPB6F9c8Q5u9x1gWVw2l2XNASQyTDIZ3db_GiQbx1SYnNtyxsospoKClIebee-EQkcut9jBPNyiKrzZRPuX0xCiPJYTumHdHSp0UPIDIij2hJl_tacXs-hz_ckqT5HZvJPsxgJCeXz3tP7IlgbeGbPFMFmaYNX4UxYD4XS10MPXUK4_qp8wK4EqjT2YBNF7xXi_sjWMj7isc0i0YSIl6_nQ" parser->sign_len = 342

That may be the case, but the strncmp is still incorrect, as it only compares the prefix. e.g. strncmp("Hello", "HelloWorld", 5) == 0.

Two possibilities. Change the dot to a \0 and then put it back. Or do something complicated that does the comparison. If you know where the dot was you could check the length manually, and if equal, just do a memcmp.

That may be the case, but the strncmp is still incorrect, as it only compares the prefix. e.g. strncmp("Hello", "HelloWorld", 5) == 0.

Understood.

If you know where the dot was you could check the length manually, and if equal, just do a memcmp.

I added this check.

d3zd3z · 2024-10-04T20:46:19Z

subsys/jwt/jwt.c

+		return -ENOSPC;
+	}
+	/* -1/+1 to also copy the dot */
+	strncpy(builder.buf, parser->payload - 1, parser->payload_len + 1);


Likewise here, strncpy is definitely not the right thing to use, as it can leave the buffer not terminated. In this particular case, it always leave the buffer non-terminated. I'd just use memcpy of the given length, and place the terminating nul manually.

OK, changed

d3zd3z · 2024-10-04T20:46:57Z

subsys/jwt/jwt.c

+		return res;
+	}
+	builder_sign_len = builder.buf - builder_sign;
+	res = strncmp(parser->sign, builder_sign, builder_sign_len);


Likewise here, we have control over the builder, so we know if is terminated (you would return errors if it doesn't fit), so just use strcmp.

OK, changed

d3zd3z · 2024-10-08T19:13:39Z

subsys/jwt/jwt.c

+	}
+	builder_header = builder.base;
+	builder_header_len = builder.buf - builder.base;
+	res = strncmp(parser->header, builder_header, builder_header_len);


Two possibilities. Change the dot to a \0 and then put it back. Or do something complicated that does the comparison. If you know where the dot was you could check the length manually, and if equal, just do a memcmp.

- Use base64url instead of base64 to explicitly show that Base64URL is used and not Base64 - Add encode suffix to prepare JWT verify function introduction Signed-off-by: Benjamin Lemouzy <blemouzy@centralp.fr>

- Add a new function jwt_parse_payload to parse token payload - Add a new function jwt_verify to verify token header and signature Signed-off-by: Benjamin Lemouzy <blemouzy@centralp.fr>

Fix 'WARNING: Violation to rule 21.2 (Should not used a reserved identifier) - exp' Signed-off-by: Benjamin Lemouzy <blemouzy@centralp.fr>

blemouzy · 2024-10-09T13:09:49Z

jwt_verify now works with RSA but not with ECDSA (as signature changes at each call).

I'll try to do it another way.

zephyrbot added the area: JSON label Jul 16, 2024

zephyrbot requested review from d3zd3z, mrfuchs and Sir-Branch July 16, 2024 08:48

zephyrbot assigned d3zd3z Jul 16, 2024

blemouzy force-pushed the jwt-add-verify branch 2 times, most recently from 0aa1230 to 1aa0cab Compare July 16, 2024 11:22

blemouzy force-pushed the jwt-add-verify branch from 1aa0cab to 2cce404 Compare August 23, 2024 14:56

d3zd3z requested changes Oct 4, 2024

View reviewed changes

blemouzy force-pushed the jwt-add-verify branch 2 times, most recently from 6636ea3 to 84f3fd8 Compare October 7, 2024 13:21

d3zd3z requested changes Oct 8, 2024

View reviewed changes

jwt: rename base64 functions to base64url_encode

f1472f3

- Use base64url instead of base64 to explicitly show that Base64URL is used and not Base64 - Add encode suffix to prepare JWT verify function introduction Signed-off-by: Benjamin Lemouzy <blemouzy@centralp.fr>

blemouzy force-pushed the jwt-add-verify branch from 84f3fd8 to 34de574 Compare October 9, 2024 10:03

blemouzy added 2 commits October 9, 2024 14:55

jwt: add functions to parse a token

939932a

- Add a new function jwt_parse_payload to parse token payload - Add a new function jwt_verify to verify token header and signature Signed-off-by: Benjamin Lemouzy <blemouzy@centralp.fr>

jwt: rename exp variables to expt

70d0107

Fix 'WARNING: Violation to rule 21.2 (Should not used a reserved identifier) - exp' Signed-off-by: Benjamin Lemouzy <blemouzy@centralp.fr>

blemouzy force-pushed the jwt-add-verify branch from 34de574 to 70d0107 Compare October 9, 2024 13:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

jwt: add functions to parse a token #75924

jwt: add functions to parse a token #75924

blemouzy commented Jul 16, 2024

d3zd3z left a comment

d3zd3z Oct 4, 2024

blemouzy Oct 7, 2024 •

edited

Loading

d3zd3z Oct 8, 2024

d3zd3z Oct 8, 2024

blemouzy Oct 9, 2024

d3zd3z Oct 4, 2024

blemouzy Oct 7, 2024

d3zd3z Oct 4, 2024

blemouzy Oct 7, 2024

d3zd3z Oct 8, 2024

blemouzy commented Oct 9, 2024 •

edited

Loading

jwt: add functions to parse a token #75924

Are you sure you want to change the base?

jwt: add functions to parse a token #75924

Conversation

blemouzy commented Jul 16, 2024

d3zd3z left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blemouzy Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blemouzy commented Oct 9, 2024 • edited Loading

blemouzy Oct 7, 2024 •

edited

Loading

blemouzy commented Oct 9, 2024 •

edited

Loading