Skip to content

Fix misc findings from reaudit - #376

Merged
jclapis merged 13 commits into
sigp-audit-fixesfrom
misc-reaudit-fixes
Sep 30, 2025
Merged

Fix misc findings from reaudit#376
jclapis merged 13 commits into
sigp-audit-fixesfrom
misc-reaudit-fixes

Conversation

@jclapis

@jclapisjclapis commented Sep 18, 2025

Copy link
Copy Markdown
Contributor

Work In Progress

This fixes several smaller miscellaneous findings from the reaudit assessment.

CBST2-01

  • Added path / route to the JWT claims to improve resilience against replaying.

CBST2-04

  • Added rate-limit flagging to admin route JWT auth checking.
  • handle_reload() now updates the state atomically once all of the code has succeeded, and doesn't update it at all if any failures occur.

CBST2-06

  • JWT auth checking now attempts to retrieve the rightmost client IP from various proxy forwarding headers, then the falls back to the direct connection IP.
  • Fixed a race that occurred due to differing locks on JWT auth failure reads and writes. This was reverted due to performance considerations with locking the auth check for each request.
  • (Done in Add a task to clean up expired JWT failure rate limits  #380) Added a task to periodically sweep and clean up expired JWT authorization failures.

CBST2-14

  • Renamed a misnamed variable.

@jclapisjclapis self-assigned this Sep 18, 2025
@jclapisjclapis added pbs Pbs module / Builder API core Core part of the repo (signer, modules interface) signer Signer module labels Sep 18, 2025
@jclapis
jclapis marked this pull request as ready for review September 22, 2025 12:41
@jclapisjclapis changed the title [WIP] - Fix misc findings from reauditFix misc findings from reauditSep 22, 2025
Comment threadcrates/signer/src/service.rs Outdated
Comment threadcrates/signer/src/service.rs Outdated
Comment on lines +98 to +100
jwts: Arc::new(RwLock::new(config.mod_signing_configs)),
admin_secret: Arc::new(RwLock::new(config.admin_secret)),
jwt_auth_failures: Arc::new(RwLock::new(HashMap::new())),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i dont think we should use tokio RwLock here, in general I'd much rather take a guard lock twice than hold it for long

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Discussed offline and reverted in a29a366 due to performance reasons.

Comment threadcrates/signer/src/service.rs Outdated
Comment threadcrates/signer/src/service.rs Outdated
@jclapis
jclapis merged commit d566dea into sigp-audit-fixesSep 30, 2025
2 checks passed
@jclapis
jclapis deleted the misc-reaudit-fixes branch September 30, 2025 21:09
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

coreCore part of the repo (signer, modules interface)pbsPbs module / Builder APIsignerSigner module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@jclapis@ManuelBilbao@ltitanb