[PR #6846] [proxy] Preserve JSON/OIDC cookies when stripping nb_session #29953

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

📋 Pull Request Information

Original PR: https://github.com/netbirdio/netbird/pull/6846
Author: @NomisCZ
Created: 7/21/2026
Status: 🔄 Open

Base: mainHead: fix/proxy-preserve-json-cookies


📝 Commits (5)

  • 878f7f6 [proxy] Preserve JSON/OIDC cookies when stripping nb_session
  • a2b8154 [proxy] Handle multiple Cookie headers when stripping nb_session
  • 49e8b4a Merge branch 'main' into fix/proxy-preserve-json-cookies
  • 2c01a60 Merge branch 'main' into fix/proxy-preserve-json-cookies
  • 17aa90e Merge branch 'main' into fix/proxy-preserve-json-cookies

📊 Changes

2 files changed (+67 additions, -5 deletions)

View changed files

📝 proxy/internal/proxy/reverseproxy.go (+34 -5)
📝 proxy/internal/proxy/reverseproxy_test.go (+33 -0)

📄 Description

Summary

stripSessionCookie currently removes the NetBird session cookie by parsing the inbound Cookie header with Request.Cookies() and rewriting the outbound header with AddCookie().

Go's net/http cookie parser silently drops cookie values that contain octets outside RFC 6265 cookie-octet (notably ", ,, {, }). That means any upstream app cookie whose value is JSON is lost on the way to the backend, while alphanumeric cookies (including _interaction.sig) survive.

Impact (example: Logto)

Logto / node-oidc-provider stores OIDC interaction state in cookies like:

Set-Cookie: _interaction={"admin-console":"<uid>","_legacy":"<uid>"}; Path=/; HttpOnly; Secure; SameSite=Lax
Set-Cookie: _logto={"appId":"admin-console"}; Path=/; Secure; SameSite=Lax

With NetBird Reverse Proxy in front of Logto Admin Console:

  1. GET /oidc/auth sets _interaction + _interaction.sig
  2. Browser redirects to /sign-in and sends both cookies
  3. Proxy re-serializes Cookie_interaction / _logto are dropped
  4. Logto cannot load the interaction → redirect to /unknown-session

The same Logto instance works when reached through a proxy that forwards the raw Cookie header (e.g. Cloudflare / nginx). Reproduced against NetBird Reverse Proxy 0.74.7.

Fix

Strip only nb_session from the raw Cookie header string and forward the remainder unchanged. Do not round-trip through Cookies() / AddCookie().

Test plan

  • Unit test: TestRewriteFunc_SessionCookieStripping / preserves JSON cookie values with RFC-invalid octets
  • Existing cases still pass: strips nb_session, preserves normal cookies, empty cookie header
  • Manual: Logto Admin Console behind NetBird RP (0.74.7 + this patch) — sign-in no longer ends on /unknown-session

Notes

  • Traefik TLS passthrough in front of the proxy is unaffected; the bug is entirely in stripSessionCookie.
  • Enabling/disabling "Pass Host Header" / "Rewrite Redirects" does not mitigate this.

View with Codesmith Autofix with Codesmith
Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Bug Fixes
    • Improved proxy cookie forwarding to better preserve cookie values that include special or nonstandard characters.
    • Session cookie stripping is now more robust, correctly removing only the session cookie while leaving other cookie data intact (including cases with multiple Cookie headers).

🔄 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/6846 **Author:** [@NomisCZ](https://github.com/NomisCZ) **Created:** 7/21/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `fix/proxy-preserve-json-cookies` --- ### 📝 Commits (5) - [`878f7f6`](https://github.com/netbirdio/netbird/commit/878f7f6a129801fdb3490ac46aec2876a74b974f) [proxy] Preserve JSON/OIDC cookies when stripping nb_session - [`a2b8154`](https://github.com/netbirdio/netbird/commit/a2b81544a685354d94c64e307af0cd813cce9057) [proxy] Handle multiple Cookie headers when stripping nb_session - [`49e8b4a`](https://github.com/netbirdio/netbird/commit/49e8b4a14088e3226d40851666a40a97d7bc42f1) Merge branch 'main' into fix/proxy-preserve-json-cookies - [`2c01a60`](https://github.com/netbirdio/netbird/commit/2c01a6003fe7e0de6350f19fd8570bd982173051) Merge branch 'main' into fix/proxy-preserve-json-cookies - [`17aa90e`](https://github.com/netbirdio/netbird/commit/17aa90e6c2d5bcb290b9b8102025ca3da77a6234) Merge branch 'main' into fix/proxy-preserve-json-cookies ### 📊 Changes **2 files changed** (+67 additions, -5 deletions) <details> <summary>View changed files</summary> 📝 `proxy/internal/proxy/reverseproxy.go` (+34 -5) 📝 `proxy/internal/proxy/reverseproxy_test.go` (+33 -0) </details> ### 📄 Description ## Summary `stripSessionCookie` currently removes the NetBird session cookie by parsing the inbound `Cookie` header with `Request.Cookies()` and rewriting the outbound header with `AddCookie()`. Go's `net/http` cookie parser silently drops cookie values that contain octets outside RFC 6265 `cookie-octet` (notably `"`, `,`, `{`, `}`). That means any upstream app cookie whose value is JSON is lost on the way to the backend, while alphanumeric cookies (including `_interaction.sig`) survive. ### Impact (example: Logto) Logto / `node-oidc-provider` stores OIDC interaction state in cookies like: ```http Set-Cookie: _interaction={"admin-console":"<uid>","_legacy":"<uid>"}; Path=/; HttpOnly; Secure; SameSite=Lax Set-Cookie: _logto={"appId":"admin-console"}; Path=/; Secure; SameSite=Lax ``` With NetBird Reverse Proxy in front of Logto Admin Console: 1. `GET /oidc/auth` sets `_interaction` + `_interaction.sig` 2. Browser redirects to `/sign-in` and sends both cookies 3. Proxy re-serializes `Cookie` → `_interaction` / `_logto` are dropped 4. Logto cannot load the interaction → redirect to `/unknown-session` The same Logto instance works when reached through a proxy that forwards the raw `Cookie` header (e.g. Cloudflare / nginx). Reproduced against NetBird Reverse Proxy `0.74.7`. ### Fix Strip only `nb_session` from the **raw** `Cookie` header string and forward the remainder unchanged. Do not round-trip through `Cookies()` / `AddCookie()`. ## Test plan - [x] Unit test: `TestRewriteFunc_SessionCookieStripping` / `preserves JSON cookie values with RFC-invalid octets` - [x] Existing cases still pass: strips `nb_session`, preserves normal cookies, empty cookie header - [x] Manual: Logto Admin Console behind NetBird RP (`0.74.7` + this patch) — sign-in no longer ends on `/unknown-session` ## Notes - Traefik TLS passthrough in front of the proxy is unaffected; the bug is entirely in `stripSessionCookie`. - Enabling/disabling "Pass Host Header" / "Rewrite Redirects" does not mitigate this. <!-- codesmith:footer --> --- <a href="https://app.blacksmith.sh/netbirdio/codesmith/netbird/pr/6846"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-light-v2.svg"><img alt="View with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/view-with-codesmith-dark-v2.svg"></picture></a> <a href="https://backend.blacksmith.sh/track/enable-autofix?expires=1787219419&installation_model_id=427504&pr_number=6846&repository=netbirdio%2Fnetbird&return_to=https%3A%2F%2Fgithub.com%2Fnetbirdio%2Fnetbird%2Fpull%2F6846&signature=76617e363845a46ed71147897eeffbb86585ca15bdac772180f36a9f35b0783e"><picture><source media="(prefers-color-scheme: dark)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-light.svg"><img alt="Autofix with Codesmith" src="https://pr-comments-assets.blacksmith.sh/codesmith/autofix-with-codesmith-dark.svg"></picture></a> <sup>Need help on this PR? Tag <code>/codesmith</code> with what you need. Autofix is disabled.</sup> <!-- codesmith:autofix:disabled --> <!-- /codesmith:footer --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved proxy cookie forwarding to better preserve cookie values that include special or nonstandard characters. * Session cookie stripping is now more robust, correctly removing only the session cookie while leaving other cookie data intact (including cases with multiple `Cookie` headers). <!-- 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:09:30 -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#29953