bug(download): /d/{token} is behind requireSession — Secure Client receives the LOGIN PAGE, never a bundle #5

Closed
opened 2026-08-05 16:27:04 +02:00 by bosun · 1 comment
Owner

The defect

internal/web/server.go:46

mux.HandleFunc("GET /d/{token}", s.requireSession(s.handleDownload))

The one-time download URL requires a browser session. A client that cannot log in gets a 303 to /login, follows it, and receives HTML.

Measured — nginx access log, 2026-08-05

13:35:12  GET /purser/d/<token>  303 → /purser/login   "AnyConnect/5.1.16264 CFNetwork/3860.600.21 Darwin/25.5.0"
13:35:13  GET /purser/login      200  2353 bytes       "AnyConnect/5.1.16264 …"
13:35:25  GET /purser/d/<token>  200  3651 bytes       "Safari"   ← browser, has the cookie

Cisco Secure Client parsed 2,353 bytes of login form as PKCS#12. That is the operator-reported "unable to decode the provided data" — it is decoding an HTML page.

The identical 303 appears against the previous certificate at 13:23:40, so this has been the failure the whole time.

🔴 What it cost

An entire afternoon was spent on the bundle ENCODING — SHA-256 vs SHA-1 MAC, AES-256 vs 3DES — with three separate fixes shipped. Every bundle measured was fine. None ever reached Secure Client. The browser path always worked because Safari carries the session cookie, which is why Keychain import succeeded and masked the real failure.

The design fault

A one-time download token IS a credential. Requiring a session on top of it defeats the point of an unguessable URL handed to a client that cannot authenticate interactively — and that client is precisely the one the feature exists for.

The downloads table already carries the controls that matter: expires_at (15 min) and fetch_count.

Scope

  • GET /d/{token} serves on the token alone — no session required
  • Expiry + fetch-count remain enforced; token stays the only credential
  • Decide and document the fetch-count policy: cert 3 shows fetch_count=4, so it is not one-shot today. State what it should be.
  • Negative control: an expired token and a wrong token must both 404 — not redirect
  • Regression test asserting an UNAUTHENTICATED GET /d/{token} returns application/x-pkcs12, not text/html. The test must fail on the current code.
  • alcatraz-infra#422 — Purser design record
  • The encoding work this masked: PURSER_P12_ENCODING now legacy-des (3DES + SHA-1 MAC), which matches the shape the operator imports successfully. Keep it — it is correct — but it was never the cause.

Anchor

Found by the operator, 2026-08-05, from the one asymmetry that mattered: same file, Keychain accepts it, Secure Client does not. No amount of byte-level probing of the bundle could have produced that.

## The defect `internal/web/server.go:46` ```go mux.HandleFunc("GET /d/{token}", s.requireSession(s.handleDownload)) ``` **The one-time download URL requires a browser session.** A client that cannot log in gets a `303` to `/login`, follows it, and receives HTML. ## Measured — nginx access log, 2026-08-05 ``` 13:35:12 GET /purser/d/<token> 303 → /purser/login "AnyConnect/5.1.16264 CFNetwork/3860.600.21 Darwin/25.5.0" 13:35:13 GET /purser/login 200 2353 bytes "AnyConnect/5.1.16264 …" 13:35:25 GET /purser/d/<token> 200 3651 bytes "Safari" ← browser, has the cookie ``` **Cisco Secure Client parsed 2,353 bytes of login form as PKCS#12.** That is the operator-reported *"unable to decode the provided data"* — it is decoding an HTML page. The identical `303` appears against the previous certificate at `13:23:40`, so this has been the failure the whole time. ## 🔴 What it cost An entire afternoon was spent on the bundle ENCODING — SHA-256 vs SHA-1 MAC, AES-256 vs 3DES — with three separate fixes shipped. **Every bundle measured was fine. None ever reached Secure Client.** The browser path always worked because Safari carries the session cookie, which is why Keychain import succeeded and masked the real failure. ## The design fault **A one-time download token IS a credential.** Requiring a session on top of it defeats the point of an unguessable URL handed to a client that cannot authenticate interactively — and that client is precisely the one the feature exists for. The `downloads` table already carries the controls that matter: `expires_at` (15 min) and `fetch_count`. ## Scope - [x] `GET /d/{token}` serves on the token alone — no session required - [x] Expiry + fetch-count remain enforced; token stays the only credential - [x] Decide and document the fetch-count policy: cert 3 shows `fetch_count=4`, so it is not one-shot today. State what it should be. - [x] Negative control: an expired token and a wrong token must both 404 — not redirect - [x] Regression test asserting an UNAUTHENTICATED `GET /d/{token}` returns `application/x-pkcs12`, not `text/html`. **The test must fail on the current code.** ## Related - `alcatraz-infra#422` — Purser design record - The encoding work this masked: `PURSER_P12_ENCODING` now `legacy-des` (3DES + SHA-1 MAC), which matches the shape the operator imports successfully. Keep it — it is correct — but it was never the cause. ## Anchor Found by the operator, 2026-08-05, from the one asymmetry that mattered: **same file, Keychain accepts it, Secure Client does not.** No amount of byte-level probing of the bundle could have produced that.
bosun closed this issue 2026-08-05 19:30:21 +02:00
Author
Owner

AC sweep @ mainall five verified against the substrate, then ticked. Closed 2026-08-05 with every box empty.

AC1  route      internal/web/server.go:60  mux.HandleFunc("GET /d/{token}", s.handleDownload)
     live       GET /purser/d/<bogus> → 404   (was 303 → login)
AC2  store      ExpiresAt + FetchCount present; ErrExpired enforced at store.go:79-84
AC3  policy     docs/operations.md:26 — "The fetch count restricts nothing. It records how many
                times a bundle was fetched." ⇒ decided AND documented, and framed honestly
AC4  controls   TestDownload_ExpiredWindowIsGone · TestDownload_UnknownToken ·
                TestDownload_UnauthenticatedFailuresDoNotRedirect
AC5  regression TestDownload_ServesWithoutASession asserts
                Content-Type == "application/x-pkcs12", with the comment
                "serving HTML here IS the defect"

🔑 AC5 is the one that could have been faked and was not. It asserts the CONTENT TYPE, not merely a 200 — and TestDownload_UnauthenticatedFailuresDoNotRedirect carries the negative half: "a refusal became a redirect, which is #5 again." ⚠️ A test asserting only status == 200 would have passed on a login page.

📌 The ticks were applied after reading each one against main, not from memory of the merge. Per § Acceptance-criteria tick discipline, a state-asserting tick is a claim about the world and gets re-derived from the substrate.

## AC sweep @ `main` — **all five verified against the substrate, then ticked. Closed 2026-08-05 with every box empty.** ``` AC1 route internal/web/server.go:60 mux.HandleFunc("GET /d/{token}", s.handleDownload) live GET /purser/d/<bogus> → 404 (was 303 → login) AC2 store ExpiresAt + FetchCount present; ErrExpired enforced at store.go:79-84 AC3 policy docs/operations.md:26 — "The fetch count restricts nothing. It records how many times a bundle was fetched." ⇒ decided AND documented, and framed honestly AC4 controls TestDownload_ExpiredWindowIsGone · TestDownload_UnknownToken · TestDownload_UnauthenticatedFailuresDoNotRedirect AC5 regression TestDownload_ServesWithoutASession asserts Content-Type == "application/x-pkcs12", with the comment "serving HTML here IS the defect" ``` 🔑 **AC5 is the one that could have been faked and was not.** ✅ **It asserts the CONTENT TYPE, not merely a 200 — and `TestDownload_UnauthenticatedFailuresDoNotRedirect` carries the negative half: *"a refusal became a redirect, which is #5 again."*** ⚠️ **A test asserting only `status == 200` would have passed on a login page.** 📌 **The ticks were applied after reading each one against `main`, not from memory of the merge. Per § Acceptance-criteria tick discipline, a state-asserting tick is a claim about the world and gets re-derived from the substrate.**
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
frankenbit/purser#5
No description provided.