Skip to content

script: Require HMAC comparison to be constant time#43773

Merged
simonwuelker merged 1 commit into
servo:mainfrom
kkoyung:hmac-constant-time
Mar 31, 2026
Merged

script: Require HMAC comparison to be constant time#43773
simonwuelker merged 1 commit into
servo:mainfrom
kkoyung:hmac-constant-time

Conversation

@kkoyung

@kkoyung kkoyung commented Mar 30, 2026

Copy link
Copy Markdown
Member

We should compare HMAC signatures in constant time when validating user-provided signatures, to prevent leaking timing information proportional to the number of matching bytes. The WebCrypto specification has also updated to require to use constant-time comparison in HMAC signatures.

We update our implementation accordingly. Since we are still using the aws-lc-rs crate for our HMAC implementation, we use the function verify_slices_are_equal provided by aws_lc_rs::constant_time to guarantees the comparison is constant-time.

Specification Update: w3c/webcrypto@c962bc7
Testing: Existing tests suffice.

We should compare HMAC signatures when validating user-provided
signatures in constant time, to prevent leaking timing information
proportional to the number of matching bytes. The WebCrypto
specification has also updated to require to use constant-time
comparison in HMAC signatures.

We update our implementation accordingly. Since we are still using
the `aws-lc-rs` crate for our HMAC implementation, we use the function
`verify_slices_are_equal` provided by `aws_lc_rs::constant_time` to
guarantees the comparison is constant-time.

Specification Update: w3c/webcrypto@c962bc7
Testing: Existing tests suffice.

Signed-off-by: Kingsley Yung <kingsley@kkoyung.dev>
@kkoyung kkoyung requested a review from gterzian as a code owner March 30, 2026 10:23
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 30, 2026
let mac = hmac::sign(&sign_key, message);

// Step 2. Return true if mac is equal to signature and false otherwise.
Ok(mac.as_ref() == signature)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we should cover this with WPT test

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hrm. Is it possible to test that an algorithm is run in constant time via a WPT test?

@kkoyung kkoyung Mar 30, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think it is pretty hard to do it as a WPT test. The time difference between constant-time comparison and naive comparison of less than a hundred bytes can be as small as a few nanosecond. Detecting the difference may need fine-tuned device and/or well-designed framework.

@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Mar 30, 2026
@simonwuelker simonwuelker added this pull request to the merge queue Mar 31, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 31, 2026
Merged via the queue into servo:main with commit 4d0c29c Mar 31, 2026
33 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Mar 31, 2026
@kkoyung kkoyung deleted the hmac-constant-time branch April 17, 2026 10:04
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.

5 participants