bitborg/bitborg-infra
Bevaka
2
0
Avgrena
Du har redan avgrenat bitborg-infra
 0
Kod Ärenden 64 Ändringsförfrågningar 3 Projekt 1 Utgåvor Paket Wiki Aktiviteter Actions

forgejo: stale native TOTP enrolments become a 500 lockout after SECRET_KEY rotation #292

Nytt ärende
Öppen
öppnade 2026-07-31 21:44:40 +00:00 av supernaut · 4 kommentarer
supernaut kommenterad 2026-07-31 21:44:40 +00:00
Ägare
Kopiera länk

Summary

A SECRET_KEY rotation silently bricks any Forgejo-native TOTP enrolment that predates it.
The account is not degraded — it is locked out: OIDC sign-in succeeds, Forgejo then demands a
TOTP code, and submitting any code returns a 500. No code can ever succeed.

Hit live on 2026-07-31 against an account created before the Kanidm cutover.

2fa.go:64:TwoFactorPost() [E] UserSignIn: chacha20poly1305: message authentication failed
router: completed POST /user/two_factor … 500 Internal Server Error

Root cause

two_factor.secret is encrypted at rest under SECRET_KEY. That key was rotated in #73/#82
(2026-07-17) and again in #216 (2026-07-24). The AEAD (ChaCha20-Poly1305) tag therefore fails to
verify, the error escapes UserSignIn unhandled, and the request 500s.

flowchart LR
  R["SECRET_KEY rotated<br/>(#73/#82, #216)"] --> C["two_factor.secret<br/>ciphertext orphaned"]
  C --> P{"passkey still<br/>registered?"}
  P -->|yes| M["passkey branch taken<br/>→ latent, invisible"]
  P -->|no| L["TOTP branch taken<br/>→ 500, locked out"]

The passkey branch is why this stayed hidden: while a WebAuthn credential exists Forgejo never
touches the TOTP secret. Removing the passkey is what surfaces the pre-existing breakage.

Why we did not catch it

The rotation runbook did warn that SECRET_KEY rotation destroys what it encrypted, but wrote
off the 2FA half as "(moot while SSO-only)". That holds only for accounts created after the
Kanidm cutover; older accounts can still carry a native enrolment. The Actions-secret half of the
same warning was actioned at the time, the 2FA half was not.

Remediation applied

  • Recovery is to delete the dead enrolment row — the ciphertext is unrecoverable, and native
    2FA is redundant under SSO-only (MFA belongs to Kanidm).
  • Runbook corrected: the false "moot while SSO-only" reassurance is gone, SECRET_KEY rotation
    is now framed as a destructive migration of every ciphertext column, and a symptom-first
    troubleshooting entry with the recovery commands was added.
  • Resolved 2026-07-31: the dead row was deleted and sign-in via Kanidm works again.

Blast radius — closed

The audit below came back clean: no other account holds a pre-rotation TOTP enrolment. The
exposure was exactly one account, and it is fixed. That downgrades everything remaining in this
issue from remediation to prevention — there is no known user currently able to trigger this.

Remaining work

  • Audit two_factor for other accounts holding an enrolment older than 2026-07-17 — each
    one is a latent lockout waiting for its owner to drop a passkey.
    → Done, clean. No other affected accounts.
  • Detect, don't discover. Extend the 5.18 access review (scripts/access-review-2fa.py)
    to flag native 2FA enrolments predating the current SECRET_KEY, so this surfaces in a
    report rather than as a lockout. Still worth doing: it protects the next rotation, which
    is when a fresh crop of enrolments becomes undecryptable.
  • Add a post-rotation step to the rotation procedure itself: enumerate + purge stale
    enrolments as part of rotating, not afterwards.
    → Done in #431. The runbook now carries the DELETE alongside the SELECT, plus a
    pre-rotation guard that at least one admin holds a non-TOTP factor.
  • Open question: should Forgejo-native 2FA enrolment be prevented outright now that
    sign-in is SSO-only?
    → Decided 2026-08-18: no, accept it and rely on the hardened runbook. No supported
    switch exists. The only related setting is [security] GLOBAL_TWO_FACTOR_REQUIREMENT
    (none/all/admin), which can only ever require 2FA, never forbid enrolment. We do not
    set it, so the default none applies, and it must stay unset — all or admin would
    force native enrolments estate-wide and re-arm this trap for every account. The residual risk
    stands as described (a user can enrol at any time) and is caught by the rotation procedure
    rather than prevented.
  • Report upstream: Forgejo returns a 500 rather than degrading to a
    "2FA unavailable, contact your administrator" path.
    → Dropped 2026-08-18. This reverses the "report it" decision taken earlier the same day.
    The local position is unchanged: an unhandled AEAD failure reaching the user as a 500 is
    still an upstream error-handling bug on its own terms, and the recovery stays in the
    runbook. We are simply not taking it upstream. The drafted report is preserved in this
    issue's comments if it is ever wanted.

References

  • Rotation that caused it: #73 / #82 (2026-07-17), #216 (2026-07-24)
  • Runbook fix: #293
  • docs/runbook.md → "Rotate secrets" and the OIDC/login troubleshooting list
## Summary A `SECRET_KEY` rotation silently bricks any **Forgejo-native TOTP enrolment** that predates it. The account is not degraded — it is **locked out**: OIDC sign-in succeeds, Forgejo then demands a TOTP code, and submitting any code returns a **500**. No code can ever succeed. Hit live on 2026-07-31 against an account created before the Kanidm cutover. ``` 2fa.go:64:TwoFactorPost() [E] UserSignIn: chacha20poly1305: message authentication failed router: completed POST /user/two_factor … 500 Internal Server Error ``` ## Root cause `two_factor.secret` is encrypted at rest under `SECRET_KEY`. That key was rotated in #73/#82 (2026-07-17) and again in #216 (2026-07-24). The AEAD (ChaCha20-Poly1305) tag therefore fails to verify, the error escapes `UserSignIn` unhandled, and the request 500s. ```mermaid flowchart LR R["SECRET_KEY rotated<br/>(#73/#82, #216)"] --> C["two_factor.secret<br/>ciphertext orphaned"] C --> P{"passkey still<br/>registered?"} P -->|yes| M["passkey branch taken<br/>→ latent, invisible"] P -->|no| L["TOTP branch taken<br/>→ 500, locked out"] ``` The passkey branch is why this stayed hidden: while a WebAuthn credential exists Forgejo never touches the TOTP secret. Removing the passkey is what surfaces the pre-existing breakage. ## Why we did not catch it The rotation runbook **did** warn that `SECRET_KEY` rotation destroys what it encrypted, but wrote off the 2FA half as _"(moot while SSO-only)"_. That holds only for accounts created after the Kanidm cutover; older accounts can still carry a native enrolment. The Actions-secret half of the same warning was actioned at the time, the 2FA half was not. ## Remediation applied - Recovery is to **delete the dead enrolment row** — the ciphertext is unrecoverable, and native 2FA is redundant under SSO-only (MFA belongs to Kanidm). - Runbook corrected: the false _"moot while SSO-only"_ reassurance is gone, `SECRET_KEY` rotation is now framed as a destructive migration of every ciphertext column, and a symptom-first troubleshooting entry with the recovery commands was added. - **Resolved 2026-07-31:** the dead row was deleted and sign-in via Kanidm works again. ## Blast radius — closed The audit below came back **clean: no other account holds a pre-rotation TOTP enrolment.** The exposure was exactly one account, and it is fixed. That downgrades everything remaining in this issue from remediation to **prevention** — there is no known user currently able to trigger this. ## Remaining work - [x] **Audit** `two_factor` for other accounts holding an enrolment older than 2026-07-17 — each one is a latent lockout waiting for its owner to drop a passkey. → **Done, clean.** No other affected accounts. - [ ] **Detect, don't discover.** Extend the 5.18 access review (`scripts/access-review-2fa.py`) to flag native 2FA enrolments predating the current `SECRET_KEY`, so this surfaces in a report rather than as a lockout. Still worth doing: it protects the **next** rotation, which is when a fresh crop of enrolments becomes undecryptable. - [x] Add a post-rotation step to the rotation procedure itself: enumerate + purge stale enrolments as part of rotating, not afterwards. → **Done in #431.** The runbook now carries the `DELETE` alongside the `SELECT`, plus a pre-rotation guard that at least one admin holds a non-TOTP factor. - [x] **Open question:** should Forgejo-native 2FA enrolment be prevented outright now that sign-in is SSO-only? → **Decided 2026-08-18: no, accept it and rely on the hardened runbook.** No supported switch exists. The only related setting is `[security] GLOBAL_TWO_FACTOR_REQUIREMENT` (`none`/`all`/`admin`), which can only ever *require* 2FA, never forbid enrolment. We do not set it, so the default `none` applies, and **it must stay unset** — `all` or `admin` would force native enrolments estate-wide and re-arm this trap for every account. The residual risk stands as described (a user can enrol at any time) and is caught by the rotation procedure rather than prevented. - [x] Report upstream: Forgejo returns a 500 rather than degrading to a "2FA unavailable, contact your administrator" path. → **Dropped 2026-08-18.** This reverses the "report it" decision taken earlier the same day. The local position is unchanged: an unhandled AEAD failure reaching the user as a 500 is still an upstream error-handling bug on its own terms, and the recovery stays in the runbook. We are simply not taking it upstream. The drafted report is preserved in this issue's comments if it is ever wanted. ## References - Rotation that caused it: #73 / #82 (2026-07-17), #216 (2026-07-24) - Runbook fix: #293 - `docs/runbook.md` → "Rotate secrets" and the OIDC/login troubleshooting list
supernaut kommenterad 2026-08-16 12:30:57 +00:00
Upphovsperson
Ägare
Kopiera länk

Correction: the blast radius is wider than this issue records, and the lockout is narrower

Verified against the Forgejo v16.0.2 source while researching identity architecture. Three
corrections, one of which is good news.

1. Scratch codes and WebAuthn credentials SURVIVE a rotation

This issue treats the account as locked out. It is not fully locked out.

keying is referenced on exactly three lines of models/auth/twofactor.go: the import, SetSecret
and ValidateTOTP. VerifyScratchToken is plain PBKDF2 with no keying call, and
models/auth/webauthn.go has none at all.

So after a SECRET_KEY rotation, all of these still work:

  • the scratch code at /user/two_factor/scratch
  • the scratch code during a password reset
  • a registered security key
  • personal access tokens and SSH keys

Only the TOTP secret is undecryptable. Re-enrolment also works, because ReenrollTwoFactorPost
never calls ValidateTOTP.

Caveat that matters: Forgejo stores one scratch code per user, not a set of ten
(GenerateScratchToken). It is shown once via a flash message, and using it regenerates a
replacement whose plaintext is discarded.

2. The blast radius is not just 2FA

keying.Init seeds six subkeys from SECRET_KEY. A rotation orphans all of them:

  • TOTP secrets
  • Actions secrets
  • webhook Authorization headers
  • push-mirror SSH keys
  • pull-mirror remote addresses, including embedded credentials
  • migration task credentials

The runbook fix in #293 reframed rotation as a destructive migration, which was right. This is the
list it destroys.

3. Rotation is not supported upstream, and is not being worked on

Worth recording so nobody waits for a fix:

  • gitea#16832 (secret rotation) has been open since 2021-08-26.
  • forgejo#12389 ("key rotation for encrypted database values"), opened 2026-05-03, has zero
    comments    .
  • forgejo#9860 was closed as user error, with a maintainer stating plainly that Forgejo does not
    support changing the SECRET_KEY once it is set.
  • There is no OLD_SECRET_KEY. Zero hits tree-wide.

Forgejo's own code carries a FIXME at modules/setting/security.go refusing to move users off the
default key because rotation is not supported.

4. Two operational notes for the remaining prevention work

There is no API to reset 2FA. Only forgejo admin user reset-mfa -u <user> or the admin web
UI checkbox. The admin UI sits behind the very 2FA that broke. At least one admin must hold a
non-TOTP factor or documented shell access, or a rotation can lock out the only account able to
fix it.

Over HTTP Basic the failure surfaces as a 401, not a 500 (services/auth/method/basic.go).
The symptom-first troubleshooting entry added in #293 should mention that, or a git-over-HTTPS
report will be misdiagnosed as bad credentials.

Also relevant to the unticked "prevent native 2FA outright" item: enrolling a security key
permanently disables HTTP Basic auth for that user, so git-over-HTTPS with a password stops
working. That is a real cost to pushing users toward WebAuthn as the native second factor, and it
argues further for keeping MFA in Kanidm rather than Forgejo.

Suggested edit to the remaining work

The item "extend the access review to flag native 2FA enrolments predating the current
SECRET_KEY" is still the right call. Consider widening it: the same review should confirm at
least one admin holds a non-TOTP factor, since that is the control that keeps the next rotation
recoverable.

## Correction: the blast radius is wider than this issue records, and the lockout is narrower Verified against the Forgejo v16.0.2 source while researching identity architecture. Three corrections, one of which is good news. ### 1. Scratch codes and WebAuthn credentials SURVIVE a rotation This issue treats the account as locked out. It is not fully locked out. `keying` is referenced on exactly three lines of `models/auth/twofactor.go`: the import, `SetSecret` and `ValidateTOTP`. `VerifyScratchToken` is plain PBKDF2 with no keying call, and `models/auth/webauthn.go` has none at all. So after a `SECRET_KEY` rotation, all of these still work: - the scratch code at `/user/two_factor/scratch` - the scratch code during a password reset - a registered security key - personal access tokens and SSH keys Only the TOTP secret is undecryptable. Re-enrolment also works, because `ReenrollTwoFactorPost` never calls `ValidateTOTP`. Caveat that matters: Forgejo stores **one** scratch code per user, not a set of ten (`GenerateScratchToken`). It is shown once via a flash message, and using it regenerates a replacement whose plaintext is discarded. ### 2. The blast radius is not just 2FA `keying.Init` seeds six subkeys from `SECRET_KEY`. A rotation orphans all of them: - TOTP secrets - **Actions secrets** - **webhook `Authorization` headers** - **push-mirror SSH keys** - **pull-mirror remote addresses, including embedded credentials** - **migration task credentials** The runbook fix in #293 reframed rotation as a destructive migration, which was right. This is the list it destroys. ### 3. Rotation is not supported upstream, and is not being worked on Worth recording so nobody waits for a fix: - gitea#16832 (secret rotation) has been open since 2021-08-26. - forgejo#12389 ("key rotation for encrypted database values"), opened 2026-05-03, has **zero comments**. - forgejo#9860 was closed as user error, with a maintainer stating plainly that Forgejo does not support changing the `SECRET_KEY` once it is set. - There is no `OLD_SECRET_KEY`. Zero hits tree-wide. Forgejo's own code carries a FIXME at `modules/setting/security.go` refusing to move users off the default key **because rotation is not supported**. ### 4. Two operational notes for the remaining prevention work **There is no API to reset 2FA.** Only `forgejo admin user reset-mfa -u <user>` or the admin web UI checkbox. The admin UI sits behind the very 2FA that broke. **At least one admin must hold a non-TOTP factor or documented shell access**, or a rotation can lock out the only account able to fix it. **Over HTTP Basic the failure surfaces as a 401, not a 500** (`services/auth/method/basic.go`). The symptom-first troubleshooting entry added in #293 should mention that, or a git-over-HTTPS report will be misdiagnosed as bad credentials. Also relevant to the unticked "prevent native 2FA outright" item: enrolling a security key **permanently disables HTTP Basic auth** for that user, so git-over-HTTPS with a password stops working. That is a real cost to pushing users toward WebAuthn as the native second factor, and it argues further for keeping MFA in Kanidm rather than Forgejo. ### Suggested edit to the remaining work The item "extend the access review to flag native 2FA enrolments predating the current `SECRET_KEY`" is still the right call. Consider widening it: the same review should confirm at least one admin holds a non-TOTP factor, since that is the control that keeps the next rotation recoverable.
supernaut kommenterad 2026-08-16 18:02:23 +00:00
Upphovsperson
Ägare
Kopiera länk

Prevention work: two items addressed, one is not implementable as written

PR #431.

Done

Post-rotation purge is now part of the rotation procedure, not a later exercise. The runbook
enumerated stale enrolments but stopped there, so a rotation could complete leaving a live lockout
for whoever signed in next. It now carries the DELETE alongside the SELECT.

It also gains a pre-rotation guard that was missing entirely: there is no API to reset 2FA, only
forgejo admin user reset-mfa on the host or the admin web UI, and that UI sits behind the very 2FA
the rotation breaks. Without a non-TOTP factor on at least one admin, a rotation can lock out the
only account able to repair it. That is the single most useful line in this whole issue and it was
nowhere.

The access review no longer trusts its own filter. Unrelated to the enrolment question, but
found while looking: access-review-2fa.py derives its entire 2FA column from a query parameter,
and Forgejo ignores unrecognised query parameters rather than rejecting them. If is_2fa_enabled
were ever not honoured, the report would mark the whole estate as 2FA-enabled, invisibly, and that
report is ISO 27001 evidence. It now verifies the two filtered halves partition the full list and
refuses to render otherwise.

Not implementable as written

Extend the access review (scripts/access-review-2fa.py) to flag native 2FA enrolments predating
the current SECRET_KEY

Not possible through the admin API. Checked against the API surface:

  • the User model exposes no 2FA field at all;
  • there are no 2FA or WebAuthn API paths;
  • nothing exposes an enrolment timestamp.

The timestamp exists only in the database (two_factor.created_unix), which is exactly why the
runbook's procedure is a psql query rather than a script. Building a weaker API-based
approximation and ticking the box would be worse than saying so, so the checkbox stays unticked and
the runbook covers the same ground operationally.

If a scripted version is still wanted it needs database access, which would make it a different
tool with a different trust boundary from the read-only PAT this one uses.

A correction to this issue's framing

The issue describes the account as locked out. It is not fully locked out, verified against the
v16.0.2 source: keying appears on three lines of models/auth/twofactor.go,
VerifyScratchToken is plain PBKDF2, and models/auth/webauthn.go has no keying call. Scratch
codes, security keys, PATs, SSH keys and re-enrolment all survive a rotation. Only the TOTP secret
is lost.

Also worth knowing for triage: over HTTP Basic the same breakage surfaces as a 401, not a 500,
so a git-over-HTTPS report will read as bad credentials.

Still open, both decisions rather than code

  • Whether to prevent Forgejo-native 2FA enrolment outright under SSO-only. Note one real cost:
    enrolling a security key permanently disables HTTP Basic auth for that user, so
    git-over-HTTPS with a password stops working. That argues for keeping MFA in Kanidm rather than
    pushing users toward WebAuthn in Forgejo.
  • Whether to report the 500 upstream. Worth knowing first that rotation itself is unsupported and
    unscheduled: gitea#16832 open since 2021, forgejo#12389 opened 2026-05-03 with zero comments,
    and forgejo#9860 closed as user error with a maintainer stating plainly that Forgejo does not
    support changing SECRET_KEY once set.
## Prevention work: two items addressed, one is not implementable as written PR #431. ### Done **Post-rotation purge is now part of the rotation procedure**, not a later exercise. The runbook enumerated stale enrolments but stopped there, so a rotation could complete leaving a live lockout for whoever signed in next. It now carries the `DELETE` alongside the `SELECT`. It also gains a **pre-rotation guard that was missing entirely**: there is no API to reset 2FA, only `forgejo admin user reset-mfa` on the host or the admin web UI, and that UI sits behind the very 2FA the rotation breaks. Without a non-TOTP factor on at least one admin, a rotation can lock out the only account able to repair it. That is the single most useful line in this whole issue and it was nowhere. **The access review no longer trusts its own filter.** Unrelated to the enrolment question, but found while looking: `access-review-2fa.py` derives its entire 2FA column from a query parameter, and Forgejo ignores unrecognised query parameters rather than rejecting them. If `is_2fa_enabled` were ever not honoured, the report would mark the whole estate as 2FA-enabled, invisibly, and that report is ISO 27001 evidence. It now verifies the two filtered halves partition the full list and refuses to render otherwise. ### Not implementable as written > Extend the access review (`scripts/access-review-2fa.py`) to flag native 2FA enrolments predating > the current `SECRET_KEY` Not possible through the admin API. Checked against the API surface: - the `User` model exposes **no 2FA field at all**; - there are **no** 2FA or WebAuthn API paths; - nothing exposes an enrolment timestamp. The timestamp exists only in the database (`two_factor.created_unix`), which is exactly why the runbook's procedure is a `psql` query rather than a script. Building a weaker API-based approximation and ticking the box would be worse than saying so, so the checkbox stays unticked and the runbook covers the same ground operationally. If a scripted version is still wanted it needs database access, which would make it a different tool with a different trust boundary from the read-only PAT this one uses. ### A correction to this issue's framing The issue describes the account as locked out. **It is not fully locked out**, verified against the v16.0.2 source: `keying` appears on three lines of `models/auth/twofactor.go`, `VerifyScratchToken` is plain PBKDF2, and `models/auth/webauthn.go` has no keying call. Scratch codes, security keys, PATs, SSH keys and re-enrolment all survive a rotation. Only the TOTP secret is lost. Also worth knowing for triage: over HTTP Basic the same breakage surfaces as a **401, not a 500**, so a git-over-HTTPS report will read as bad credentials. ### Still open, both decisions rather than code - Whether to prevent Forgejo-native 2FA enrolment outright under SSO-only. Note one real cost: enrolling a security key **permanently disables HTTP Basic auth** for that user, so git-over-HTTPS with a password stops working. That argues for keeping MFA in Kanidm rather than pushing users toward WebAuthn in Forgejo. - Whether to report the 500 upstream. Worth knowing first that rotation itself is unsupported and unscheduled: `gitea#16832` open since 2021, `forgejo#12389` opened 2026-05-03 with zero comments, and `forgejo#9860` closed as user error with a maintainer stating plainly that Forgejo does not support changing `SECRET_KEY` once set.
supernaut kommenterad 2026-08-18 10:29:59 +00:00
Upphovsperson
Ägare
Kopiera länk

Both open decisions settled, 2026-08-18

1. Native 2FA under SSO-only: accept it, do not try to prevent it

The item asked whether a supported switch exists in v16. It does not. The only
2FA-related setting is [security] GLOBAL_TWO_FACTOR_REQUIREMENT (none / all / admin), which
controls whether 2FA is required. There is nothing that forbids enrolment.

Two consequences worth writing down:

  • We do not set it, so Forgejo's default none applies. It must stay unset. Setting it to all
    or admin would force native enrolments estate-wide and re-arm this exact trap for every account,
    turning a latent single-account hazard into a guaranteed estate-wide one at the next rotation.
  • An edge-level block on the enrolment paths was considered and rejected. Those paths are UI routes,
    not API, so an upgrade can rename them and the block would silently stop working. That is the
    vacuously-passing guard this estate keeps getting caught by, and it would need its own
    proven-able-to-fail test to be trustworthy. Not worth it for a hazard the rotation procedure
    already catches.

So the residual risk stands as the issue describes: any user can enrol native TOTP at any time and
re-arm the trap. It is caught at rotation time by #431's additions (the DELETE beside the
SELECT, and the pre-rotation guard that an admin holds a non-TOTP factor) rather than prevented.
Given native 2FA can only ever duplicate Kanidm MFA, and that enrolling a security key permanently
disables HTTP Basic auth for that user, the incentive to enrol is low and the argument for keeping
MFA in Kanidm is unchanged.

2. Report the 500 upstream: yes, scoped narrowly

Report the unhandled AEAD failure, and do not ask for SECRET_KEY rotation support. The
precedent is clear that the second framing gets closed: forgejo#9860 was closed as user error with
a maintainer stating plainly that changing SECRET_KEY is unsupported, gitea#16832 has been open
since 2021-08-26, and forgejo#12389 has had zero comments since 2026-05-03.

A 500 is a defect on its own terms regardless of whether rotation is ever supported: an
undecryptable column should degrade to a handled error, not escape as an unhandled one. That is
small, self-contained and does not depend on any policy question.

Draft ready to file, needs a Codeberg account:

Title: Undecryptable two_factor.secret returns 500 instead of a handled error

Forgejo version 16.0.2.

When two_factor.secret cannot be decrypted, TwoFactorPost returns a 500 and the user has no path
forward. The AEAD error escapes UserSignIn unhandled:

2fa.go:64:TwoFactorPost() [E] UserSignIn: chacha20poly1305: message authentication failed
router: completed POST /user/two_factor ... 500 Internal Server Error

keying is referenced on three lines of models/auth/twofactor.go: the import, SetSecret and
ValidateTOTP. When ValidateTOTP cannot decrypt, the error propagates out of the sign-in handler
rather than being handled as an authentication failure.

Expected: a handled error telling the user that TOTP is unavailable and to contact an administrator,
in the same shape as other sign-in failures. Actual: HTTP 500.

The scratch-code and WebAuthn paths are unaffected, since VerifyScratchToken is plain PBKDF2 and
models/auth/webauthn.go has no keying call, so a user with either of those can still sign in. The
500 is reached only when TOTP is the factor being used.

Note the same underlying condition surfaces as a 401 over HTTP Basic
(services/auth/method/basic.go), which makes it easy to misdiagnose a git-over-HTTPS report as bad
credentials.

This report is about the error handling only. It is not a request to support rotating SECRET_KEY.

With that, the only work left on this issue is filing the report. The "detect, don't discover" item
stays unticked for the reason already recorded: the enrolment timestamp exists only in
two_factor.created_unix and there is no API path to it, so a scripted version would need database
access and a different trust boundary from the read-only PAT the access review uses.

## Both open decisions settled, 2026-08-18 ### 1. Native 2FA under SSO-only: accept it, do not try to prevent it The item asked whether a supported switch exists in v16. **It does not.** The only 2FA-related setting is `[security] GLOBAL_TWO_FACTOR_REQUIREMENT` (`none` / `all` / `admin`), which controls whether 2FA is *required*. There is nothing that forbids enrolment. Two consequences worth writing down: - We do not set it, so Forgejo's default `none` applies. **It must stay unset.** Setting it to `all` or `admin` would force native enrolments estate-wide and re-arm this exact trap for every account, turning a latent single-account hazard into a guaranteed estate-wide one at the next rotation. - An edge-level block on the enrolment paths was considered and rejected. Those paths are UI routes, not API, so an upgrade can rename them and the block would silently stop working. That is the vacuously-passing guard this estate keeps getting caught by, and it would need its own proven-able-to-fail test to be trustworthy. Not worth it for a hazard the rotation procedure already catches. So the residual risk stands as the issue describes: any user can enrol native TOTP at any time and re-arm the trap. It is **caught at rotation time** by #431's additions (the `DELETE` beside the `SELECT`, and the pre-rotation guard that an admin holds a non-TOTP factor) rather than prevented. Given native 2FA can only ever duplicate Kanidm MFA, and that enrolling a security key permanently disables HTTP Basic auth for that user, the incentive to enrol is low and the argument for keeping MFA in Kanidm is unchanged. ### 2. Report the 500 upstream: yes, scoped narrowly Report the unhandled AEAD failure, and **do not** ask for `SECRET_KEY` rotation support. The precedent is clear that the second framing gets closed: `forgejo#9860` was closed as user error with a maintainer stating plainly that changing `SECRET_KEY` is unsupported, `gitea#16832` has been open since 2021-08-26, and `forgejo#12389` has had zero comments since 2026-05-03. A 500 is a defect on its own terms regardless of whether rotation is ever supported: an undecryptable column should degrade to a handled error, not escape as an unhandled one. That is small, self-contained and does not depend on any policy question. Draft ready to file, needs a Codeberg account: --- **Title:** `Undecryptable two_factor.secret returns 500 instead of a handled error` Forgejo version 16.0.2. When `two_factor.secret` cannot be decrypted, `TwoFactorPost` returns a 500 and the user has no path forward. The AEAD error escapes `UserSignIn` unhandled: ``` 2fa.go:64:TwoFactorPost() [E] UserSignIn: chacha20poly1305: message authentication failed router: completed POST /user/two_factor ... 500 Internal Server Error ``` `keying` is referenced on three lines of `models/auth/twofactor.go`: the import, `SetSecret` and `ValidateTOTP`. When `ValidateTOTP` cannot decrypt, the error propagates out of the sign-in handler rather than being handled as an authentication failure. Expected: a handled error telling the user that TOTP is unavailable and to contact an administrator, in the same shape as other sign-in failures. Actual: HTTP 500. The scratch-code and WebAuthn paths are unaffected, since `VerifyScratchToken` is plain PBKDF2 and `models/auth/webauthn.go` has no keying call, so a user with either of those can still sign in. The 500 is reached only when TOTP is the factor being used. Note the same underlying condition surfaces as a **401** over HTTP Basic (`services/auth/method/basic.go`), which makes it easy to misdiagnose a git-over-HTTPS report as bad credentials. This report is about the error handling only. It is not a request to support rotating `SECRET_KEY`. --- With that, the only work left on this issue is filing the report. The "detect, don't discover" item stays unticked for the reason already recorded: the enrolment timestamp exists only in `two_factor.created_unix` and there is no API path to it, so a scripted version would need database access and a different trust boundary from the read-only PAT the access review uses.
supernaut kommenterad 2026-10-02 22:53:25 +00:00
Upphovsperson
Ägare
Kopiera länk

Board grooming 2026-10-03: moved from In progress to Backlog. No work since 2026-08-18. Every box is done or decided except "Detect, don't discover", which the last comment says needs database access and a different trust boundary. Pick it up from here when that is decided.

Board grooming 2026-10-03: moved from In progress to Backlog. No work since 2026-08-18. Every box is done or decided except "Detect, don't discover", which the last comment says needs database access and a different trust boundary. Pick it up from here when that is decided.
Logga in för att delta i denna konversation.
Ingen gren/etikett specificerad
Grenar Taggar
main
fix/log-retention-and-erasure
feat/billing-reconcile-trigger
chore/renovate-script-pins
Inga resultat hittades.
Etiketter
Rensa etiketter
  
area/backups
Backup & restore

  
area/ci
Actions / runners

  
area/control-panel
Logged-in self-service area

  
area/identity
Kanidm / OIDC / entitlements

  
area/infra
Infrastructure (OpenTofu/Ansible/host)

  
area/observability
Monitoring / logs / analytics

  
area/payments
Billing / payment providers

  
area/security
Security & compliance (incl. ISO 27001)

  
area/storage
Volumes / data / LFS

  
area/web
Public web portal (gitborg-web)

  
blocked
Evaluated and specified, but waiting on an external dependency

  
needs-info
Waiting on the reporter for more information

  
needs-triage
Not yet evaluated

  
ready-for-implementation
Fully specified, ready to be picked up

  
type
bug
 Something is broken

  
type
chore
 Maintenance / housekeeping

  
type
docs
 Documentation

  
type
epic
 High-level roadmap item

  
type
feature
 New capability

  
type
task
 Concrete unit of work

  
wontfix
Will not be actioned

Inga etiketter
area/backups
area/ci
area/control-panel
area/identity
area/infra
area/observability
area/payments
area/security
area/storage
area/web
blocked
needs-info
needs-triage
ready-for-implementation
type
bug
type
chore
type
docs
type
epic
type
feature
type
task
wontfix
Milstolpe
Rensa milstolpe
Inga objekt
Ingen milstolpe
Projekt
Rensa projekt
Inga objekt
Inget projekt
Bitborg Infrastructure
Tilldelade
Rensa tilldelade
Inga tilldelade
1 deltagare
Notiser
Förfallodatum
Förfallodatumet är ogiltigt eller utanför gränserna. Använd formatet "åååå-mm-dd".

Inget förfallodatum satt.

Beroenden

Inga beroenden satta

Referens
bitborg/bitborg-infra#292
Ingen beskrivning angiven.