Skip to content

Fix: jwt constant time fixed time#60

Merged
Marsup merged 1 commit intohapijs:masterfrom
damusix:fix-jwt-constant-time-fix
Dec 9, 2025
Merged

Fix: jwt constant time fixed time#60
Marsup merged 1 commit intohapijs:masterfrom
damusix:fix-jwt-constant-time-fix

Conversation

@damusix
Copy link
Contributor

@damusix damusix commented Dec 9, 2025

No description provided.

@damusix damusix force-pushed the fix-jwt-constant-time-fix branch from f976e25 to 39441a0 Compare December 9, 2025 16:55
@Marsup Marsup force-pushed the fix-jwt-constant-time-fix branch from 39441a0 to 23ea282 Compare December 9, 2025 17:04
@Marsup Marsup self-assigned this Dec 9, 2025
@Marsup Marsup added the bug Something isn't working label Dec 9, 2025
@Marsup Marsup added this to the 3.2.2 milestone Dec 9, 2025
@Marsup Marsup merged commit aa2b12b into hapijs:master Dec 9, 2025
20 checks passed
Copy link

@kanongil kanongil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand this PR is coupled with hapijs/cryptiles#63, but I fail to understand why it needs to artificially spend time on structurally invalid signatures.

I'm guessing it is a security bit, but any attacker can trivially know the correct length of the signature. And for regular timing attacks, they require the actual content of the signature to be compared, so this makes no difference.

@damusix
Copy link
Contributor Author

damusix commented Dec 11, 2025

@kanongil the built-in crypto timesafe check throws on mismatching buffer lengths and reveals a mismatch. This was submitted as a security review. I swapped these arguments so it validates on our internally generated signature vs theirs, but youre right. Byte generation overhead reveals a timing difference due to length anyway. At least it hides dramatic time difference it had before.

@kanongil
Copy link

I guess you missed the OOB discussion. There is no point in hiding it. Even if it worked perfectly, it would only hide the length of the signature. Something that can trivially be derived from the used algorithm, or an existing non-forged token.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants