fix(envd): use constant-time comparison for signature validation#3145
Merged
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
tiago-mitralab
added a commit
to mitralab-dev/e2b-infra
that referenced
this pull request
Jul 9, 2026
…0.6.8 e2b-dev#3146 as-is would panic at boot on our host (default route has non-nil Dst=0.0.0.0/0); accept both netlink representations. Bump envd to 0.6.8 for the behavior changes in cherry-picked e2b-dev#3099/e2b-dev#3145 (version bumps dropped in cherry-pick).
tiago-mitralab
added a commit
to mitralab-dev/e2b-infra
that referenced
this pull request
Jul 9, 2026
…0.6.9 e2b-dev#3146 as-is panics at boot on our host (default route has non-nil Dst=0.0.0.0/0). Bump envd for cherry-picked e2b-dev#3099/e2b-dev#3145 envd changes.
Replace string inequality operator (!=) with crypto/subtle.ConstantTimeCompare for signature validation in auth.go. The previous implementation was vulnerable to timing attacks, where an attacker could potentially determine the correct signature byte-by-byte by measuring response times. This is a standard security best practice for comparing cryptographic values such as HMAC signatures, tokens, and hashes.
Add 6 test cases covering validateSigning: - Accepts correct signature with expiration - Rejects wrong signature - Rejects expired signature - Rejects missing signature parameter - Accepts valid access token from header - Rejects invalid access token from header Also refactor existing tests to use a shared helper.
tvi
force-pushed
the
fix/envd-auth-timing-attack
branch
from
July 9, 2026 07:05
69cc233 to
19f58f6
Compare
tiago-mitralab
added a commit
to mitralab-dev/e2b-infra
that referenced
this pull request
Jul 9, 2026
…dening) (#3) * fix(orchestrator): prevent nil panic when finding default gateway In Linux, the default route (0.0.0.0/0) has a nil Dst field in the netlink.Route struct. The previous code called route.Dst.String() without a nil check, which would panic on systems where the default route's Dst is nil. Fix by checking route.Dst == nil directly, which is the canonical way to identify the default route. (cherry picked from commit a2304dc) * fix(envd): use constant-time comparison for signature validation Replace string inequality operator (!=) with crypto/subtle.ConstantTimeCompare for signature validation in auth.go. The previous implementation was vulnerable to timing attacks, where an attacker could potentially determine the correct signature byte-by-byte by measuring response times. This is a standard security best practice for comparing cryptographic values such as HMAC signatures, tokens, and hashes. (cherry picked from commit 85cce6c) * test(envd): add validateSigning tests for signature validation Add 6 test cases covering validateSigning: - Accepts correct signature with expiration - Rejects wrong signature - Rejects expired signature - Rejects missing signature parameter - Accepts valid access token from header - Rejects invalid access token from header Also refactor existing tests to use a shared helper. (cherry picked from commit 69cc233) * fix(uffd): log error and remove stale TODO on handle failure The UFFD handle goroutine had a TODO comment stating that the sandbox should be killed when handle() fails. In fact, this is already implemented: u.exit.SetError() triggers the sandbox exit watcher in sandbox.go (line ~1093) which calls sbx.Stop(). - Remove the stale TODO comment - Add error logging when handle() fails for better observability - Add unit tests covering handle failure behavior (exit error propagation, readyCh closure, handler error state, initial state, socket creation) (cherry picked from commit 3e30b03) * fix(orchestrator): fix typo in TODO comment (to to → to) (cherry picked from commit c87310c) * fix: add t.Parallel() to all test functions (paralleltest) (cherry picked from commit 7b54ed7) * fix(orchestrator): fix race condition in Cleanup causing missed cleanup functions There was a TOCTOU race between Add()/AddPriority() and run(): 1. Add() checks hasRun (false) outside the lock 2. Add() blocks waiting for mu.Lock() 3. run() acquires lock, sets hasRun=true, executes all cleanups, unlocks 4. Add() acquires lock, appends f — but run() already finished Result: f is never executed, potentially leaking resources (network namespaces, cgroups, file descriptors, etc.). Fix: - Move hasRun.Store(true) inside the lock in run() - Add double-checked locking in Add()/AddPriority(): re-check hasRun after acquiring the lock and execute f inline if cleanup already ran Add race-condition tests that reliably reproduce the bug with -race. (cherry picked from commit d7b2551) * refactor: release lock before executing cleanup in double-check path Avoid holding the mutex while executing cleanup functions in the double-check branch. This prevents potential deadlocks if a cleanup function performs blocking operations or re-enters the lock. (cherry picked from commit 97336fd) * fix: add t.Parallel() to all test functions (paralleltest) (cherry picked from commit 619dd88) * feat(orchestrator): add capacity limit and memory-pressure eviction to template mmap cache (cherry picked from commit 1f1a5a1) * fix: use MemAvailable instead of Freeram, evict one entry per tick, add unit tests - Replace unix.Sysinfo Freeram with /proc/meminfo MemAvailable to get true available memory (includes reclaimable page cache) - Evict only one LRU entry per 1s tick instead of looping until threshold is met; mmap.Unmap is not instantaneous so the OS stats lag behind - Add 6 unit tests covering WithCapacity LRU eviction, MemAvailable parser correctness/error handling, and pressure-eviction logic (cherry picked from commit cab034c) * fix(orchestrator): enforce sandbox TTL on the node via WaitForExit WaitForExit was defined but never called, leaving the API-layer evictor as the sole mechanism to kill expired sandboxes. If the API service restarts the evictor goroutine stops and VMs accumulate indefinitely on nodes. Two changes: 1. setupSandboxLifecycle (sandboxes.go): replace sbx.Wait with sbx.WaitForExit so the lifecycle goroutine races against the sandbox TTL. On timeout, Stop() is called explicitly before the normal Close/cleanup path runs. 2. WaitForExit (sandbox.go): replace the one-shot time.After with a time.NewTimer loop that re-checks endAt after each fire. This means a KeepAlive that calls SetEndAt mid-flight correctly resets the node-side deadline instead of being silently ignored. Fixes e2b-dev#3193 (cherry picked from commit 0a823f1) * test(orchestrator): unit tests for WaitForExit TTL enforcement 5 cases covering the two code changes in the parent commit: - AlreadyExpired: endAt in the past returns error immediately - ExitBeforeTTL: clean FC exit before TTL returns nil - ExitWithError: FC exits with error, error is wrapped and returned - KeepAliveExtendsTTL: SetEndAt extension resets the timer loop; sandbox is NOT killed at the original deadline - ContextCancelled: ctx cancel returns nil without killing All pass with -race. (cherry picked from commit 4826a8e) * fix: propagate Docker image ENV vars to runtime sandbox processes When creating a sandbox, merge the template metadata's Context.EnvVars (which contains Docker image ENV directives) into the sandbox config's Envd.Vars before sending them to envd via POST /init. User-provided env vars from the SDK create() call take precedence over template defaults. Per-command env vars from process.start() still override both. This fixes both the standard resume path and the filesystem-only cold boot (reboot) path, where DefaultUser and DefaultWorkdir were already restored from template metadata but EnvVars was not. Closes e2b-dev#2268 (cherry picked from commit 5bfd55e) * perf(orchestrator): replace ip-netns-exec with nsenter (cherry-pick e2b-dev#3037, process.go conflict resolved) * fix(envd): correct tag-based process lookup (cherry-pick e2b-dev#3099, version bump deferred) * fix(orchestrator): accept nil or 0.0.0.0/0 default route + bump envd 0.6.9 e2b-dev#3146 as-is panics at boot on our host (default route has non-nil Dst=0.0.0.0/0). Bump envd for cherry-picked e2b-dev#3099/e2b-dev#3145 envd changes. --------- Co-authored-by: AdaAibaby <shaolila@buaa.edu.cn>
jakubno
previously requested changes
Jul 9, 2026
jakubno
left a comment
jakubno
left a comment
Member
There was a problem hiding this comment.
@AdaAibaby , could you please fix the lint issue?
Contributor
Author
Done. Does this look OK? |
- Add blank line before return (nlreturn) - Remove unnecessary int64() conversions (unconvert) - Replace httptest.NewRequest with NewRequestWithContext (noctx)
arkamar
requested changes
Jul 9, 2026
Contributor
Author
Done. Does this look OK? |
arkamar
approved these changes
Jul 9, 2026
arkamar
enabled auto-merge (squash)
July 9, 2026 14:35
arkamar
dismissed
jakubno’s stale review
July 9, 2026 14:45
issue was addressed and we need to move on.
tiago-mitralab
added a commit
to mitralab-dev/e2b-infra
that referenced
this pull request
Jul 21, 2026
…0.6.9 e2b-dev#3146 as-is panics at boot on our host (default route has non-nil Dst=0.0.0.0/0). Bump envd for cherry-picked e2b-dev#3099/e2b-dev#3145 envd changes.
tiago-mitralab
added a commit
to mitralab-dev/e2b-infra
that referenced
this pull request
Jul 24, 2026
…mote upstream e2b-dev#3145) (#6) * fix(envd): use constant-time comparison for signature validation Replace string inequality operator (!=) with crypto/subtle.ConstantTimeCompare for signature validation in auth.go. The previous implementation was vulnerable to timing attacks, where an attacker could potentially determine the correct signature byte-by-byte by measuring response times. This is a standard security best practice for comparing cryptographic values such as HMAC signatures, tokens, and hashes. (cherry picked from commit 85cce6c) * test(envd): add validateSigning tests for signature validation Add 6 test cases covering validateSigning: - Accepts correct signature with expiration - Rejects wrong signature - Rejects expired signature - Rejects missing signature parameter - Accepts valid access token from header - Rejects invalid access token from header Also refactor existing tests to use a shared helper. (cherry picked from commit 69cc233) --------- Co-authored-by: AdaAibaby <shaolila@buaa.edu.cn>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace string inequality operator (!=) with crypto/subtle.ConstantTimeCompare for signature validation in auth.go. The previous implementation was vulnerable to timing attacks, where an attacker could potentially determine the correct signature byte-by-byte by measuring response times.
This is a standard security best practice for comparing cryptographic values such as HMAC signatures, tokens, and hashes.
/cc @jakubno @dobrac @ValentaTomas Would appreciate your review on this change, thanks!