[PR #6385] [auth] Add clock-skew leeway to JWT validation #28147

Open
opened 2026-08-05 08:05:51 -04:00 by saavagebueno · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6385
Author: @bdolgov
Created: 6/9/2026
Status: 🔄 Open

Base: mainHead: fix/jwt-clock-skew-leeway


📝 Commits (2)

  • b236d29 [auth] Add clock-skew leeway to JWT validation
  • 4cc3f17 [auth] test: cover unknown-kid and wrong-signature paths separately

📊 Changes

2 files changed (+160 additions, -0 deletions)

View changed files

📝 shared/auth/jwt/validator.go (+21 -0)
shared/auth/jwt/validator_test.go (+139 -0)

📄 Description

Describe your changes

Validator.ValidateAndParse (shared/auth/jwt/validator.go) parses tokens with golang-jwt/v5, which defaults to zero leeway, and explicitly enables iat validation via jwt.WithIssuedAt():

parsedToken, err := jwt.Parse(
    token,
    v.getKeyFunc(ctx),
    jwt.WithAudience(v.audienceList...),
    jwt.WithIssuer(v.issuer),
    jwt.WithIssuedAt(),
)

With zero tolerance, the validator compares exp/nbf/iat against its own wall clock with no slack. If the IdP issuing the token is even slightly ahead of the NetBird service validating it,
iat/nbf land in the future from the validator's perspective and the token is rejected — surfacing as token used before issued (v5 / iat) or the older Token is not valid yet (v4 / nbf). Same
root cause: zero clock-skew tolerance.

This happens routinely without any misconfiguration:

  • right after boot, before NTP has converged;
  • small steady drift between two separately-synced hosts.

NTP doesn't eliminate it — it's exactly the gap RFC 7519 calls out leeway to cover, rather than relying on perfect clocks.

Fix

Apply a conservative, non-configurable 60s leeway via jwt.WithLeeway(...) (the library's documented mechanism for "exp, nbf, iat ... to account for clock skew between systems"). Expiry and
signature verification stay strict — this only widens the time-based comparison window by under a minute.

jwt.WithLeeway(defaultClockSkewLeeway), // 60s

The change lives in the shared validator, so it covers both the management auth manager and the client SSH server, which both use it.

Why 60s — RFC and prior art

RFC 7519 §4.1.4 (exp) / §4.1.5 (nbf) — the normative source:

Implementers MAY provide for some small leeway, usually no more than a few minutes, to account for clock skew.

https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.5

Comparable validators ship non-zero defaults, so defaulting to 0 is the outlier:

Implementation Default clock skew
HashiCorp cap/jwt 150s (nbf/exp/iat)
MIT Kerberos clockskew 300s

60s sits well under the RFC's "few minutes" ceiling while covering the unsynced-NTP startup window.

Tests

Added shared/auth/jwt/validator_test.go (the file had no tests). Table covers: a future-issued token within leeway (accepted), one beyond leeway (rejected), an expired token (rejected), and a token
signed with an unknown key (rejected) — confirming the leeway widens only the time window and does not weaken expiry or signature checks.

https://github.com/netbirdio/netbird/issues/4500

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)
  • This change does not modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — OR I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See CONTRIBUTING.md.

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why) -- subtle details of jwt validation not worth documenting

Docs PR URL (required if "docs added" is checked)

n/a

Summary by CodeRabbit

  • Bug Fixes

    • JWT validation now tolerates a 60-second clock skew when checking token issuance, not-before, and expiration times, reducing false rejections due to minor server clock differences.
  • Tests

    • Added end-to-end tests covering valid and invalid JWT scenarios (clock skew, expired tokens, unknown or mismatched signing keys) to ensure robust token validation behavior.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/netbirdio/netbird/pull/6385 **Author:** [@bdolgov](https://github.com/bdolgov) **Created:** 6/9/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/jwt-clock-skew-leeway` --- ### 📝 Commits (2) - [`b236d29`](https://github.com/netbirdio/netbird/commit/b236d29a472a15a7d5a4040f5eec875c765a6a26) [auth] Add clock-skew leeway to JWT validation - [`4cc3f17`](https://github.com/netbirdio/netbird/commit/4cc3f1760dd397100524639d78f5e21fc03d7371) [auth] test: cover unknown-kid and wrong-signature paths separately ### 📊 Changes **2 files changed** (+160 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `shared/auth/jwt/validator.go` (+21 -0) ➕ `shared/auth/jwt/validator_test.go` (+139 -0) </details> ### 📄 Description ## Describe your changes `Validator.ValidateAndParse` (`shared/auth/jwt/validator.go`) parses tokens with `golang-jwt/v5`, which defaults to **zero leeway**, and explicitly enables `iat` validation via `jwt.WithIssuedAt()`: ```go parsedToken, err := jwt.Parse( token, v.getKeyFunc(ctx), jwt.WithAudience(v.audienceList...), jwt.WithIssuer(v.issuer), jwt.WithIssuedAt(), ) ``` With zero tolerance, the validator compares `exp`/`nbf`/`iat` against its own wall clock with no slack. If the IdP issuing the token is even slightly ahead of the NetBird service validating it, `iat`/`nbf` land in the future from the validator's perspective and the token is rejected — surfacing as `token used before issued` (v5 / `iat`) or the older `Token is not valid yet` (v4 / `nbf`). Same root cause: zero clock-skew tolerance. This happens routinely without any misconfiguration: - right after boot, before NTP has converged; - small steady drift between two separately-synced hosts. NTP doesn't eliminate it — it's exactly the gap RFC 7519 calls out leeway to cover, rather than relying on perfect clocks. ## Fix Apply a conservative, non-configurable **60s** leeway via `jwt.WithLeeway(...)` (the library's documented mechanism for "exp, nbf, iat ... to account for clock skew between systems"). Expiry and signature verification stay strict — this only widens the time-based comparison window by under a minute. ```go jwt.WithLeeway(defaultClockSkewLeeway), // 60s ``` The change lives in the shared validator, so it covers both the management auth manager and the client SSH server, which both use it. ## Why 60s — RFC and prior art **RFC 7519 §4.1.4 (`exp`) / §4.1.5 (`nbf`)** — the normative source: > Implementers MAY provide for some small leeway, usually no more than a few minutes, to account for clock skew. https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.5 Comparable validators ship non-zero defaults, so defaulting to 0 is the outlier: | Implementation | Default clock skew | | --- | --- | | HashiCorp `cap/jwt` | [**150s** (nbf/exp/iat)](https://pkg.go.dev/github.com/hashicorp/cap/jwt#pkg-constants) | | MIT Kerberos `clockskew` | [**300s**](https://web.mit.edu/kerberos/krb5-latest/doc/admin/conf_files/krb5_conf.html) | 60s sits well under the RFC's "few minutes" ceiling while covering the unsynced-NTP startup window. ## Tests Added `shared/auth/jwt/validator_test.go` (the file had no tests). Table covers: a future-issued token within leeway (accepted), one beyond leeway (rejected), an expired token (rejected), and a token signed with an unknown key (rejected) — confirming the leeway widens only the time window and does not weaken expiry or signature checks. ## Issue ticket number and link https://github.com/netbirdio/netbird/issues/4500 ## Stack ### Checklist - [x] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [x] Created tests that fail without the change (if possible) - [x] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) -- subtle details of jwt validation not worth documenting ### Docs PR URL (required if "docs added" is checked) n/a <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * JWT validation now tolerates a 60-second clock skew when checking token issuance, not-before, and expiration times, reducing false rejections due to minor server clock differences. * **Tests** * Added end-to-end tests covering valid and invalid JWT scenarios (clock skew, expired tokens, unknown or mismatched signing keys) to ensure robust token validation behavior. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
saavagebueno added the pull-request label 2026-08-05 08:05:51 -04:00
Sign in to join this conversation.
No Label pull-request
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: DYNR/netbird#28147