Skip to content

Conversation

@russellb
Copy link
Member

Signed-off-by: rentianyue-jk <rentianyue-jk@360shuke.com>
@mergify mergify bot added the frontend label Sep 26, 2025
@russellb russellb added the ready ONLY add when PR is ready to merge/full CI is needed label Sep 26, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request addresses a security vulnerability by using secrets.compare_digest for API token validation to prevent timing attacks. The implementation is a significant improvement. However, I've identified a potential residual timing vulnerability related to token length and have suggested a further hardening measure by hashing the tokens before comparison. This will ensure the comparison is always done on fixed-length inputs, fully mitigating timing attacks on both token value and length.

@russellb russellb added this to the v0.11.0 Cherry Picks milestone Sep 26, 2025
This is based on gemini-code-assist's recommendation:

> While using secrets.compare_digest is a great improvement to prevent timing
> attacks on the token's value, this implementation might still be vulnerable to a
> timing attack that could reveal the length of the valid API tokens.
>
> The documentation for secrets.compare_digest notes that if the two strings being
> compared have different lengths, a timing attack could theoretically reveal
> information about their lengths. An attacker could use this to determine the
> length of valid tokens, which reduces the search space for a brute-force attack.
>
> To fully mitigate this, you can hash the API tokens to a fixed length before
> comparison. This ensures that secrets.compare_digest always operates on inputs
> of the same length. This also has the benefit of allowing non-ASCII characters
> in tokens if needed, as they will be encoded to UTF-8 bytes before hashing.

Signed-off-by: Russell Bryant <rbryant@redhat.com>
@Isotr0py Isotr0py merged commit 3f5d902 into vllm-project:main Sep 27, 2025
43 checks passed
simon-mo pushed a commit that referenced this pull request Sep 28, 2025
Signed-off-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Co-authored-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: simon-mo <simon.mo@hey.com>
pdasigi pushed a commit to pdasigi/vllm that referenced this pull request Oct 2, 2025
Signed-off-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Co-authored-by: rentianyue-jk <rentianyue-jk@360shuke.com>
yewentao256 pushed a commit that referenced this pull request Oct 3, 2025
Signed-off-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Co-authored-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: yewentao256 <zhyanwentao@126.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 10, 2025
Signed-off-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Co-authored-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
choprahetarth pushed a commit to Tandemn-Labs/vllm that referenced this pull request Oct 11, 2025
Signed-off-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Co-authored-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: simon-mo <simon.mo@hey.com>
shyeh25 pushed a commit to shyeh25/vllm that referenced this pull request Oct 14, 2025
Signed-off-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Co-authored-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: simon-mo <simon.mo@hey.com>
lywa1998 pushed a commit to lywa1998/vllm that referenced this pull request Oct 20, 2025
Signed-off-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Co-authored-by: rentianyue-jk <rentianyue-jk@360shuke.com>
alhridoy pushed a commit to alhridoy/vllm that referenced this pull request Oct 24, 2025
Signed-off-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Co-authored-by: rentianyue-jk <rentianyue-jk@360shuke.com>
xuebwang-amd pushed a commit to xuebwang-amd/vllm that referenced this pull request Oct 24, 2025
Signed-off-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Co-authored-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: xuebwang-amd <xuebwang@amd.com>
rtourgeman pushed a commit to rtourgeman/vllm that referenced this pull request Nov 10, 2025
Signed-off-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Signed-off-by: Russell Bryant <rbryant@redhat.com>
Co-authored-by: rentianyue-jk <rentianyue-jk@360shuke.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

frontend ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants