Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Secret rotation / secret loss recovery #16832

Open
clarfonthey opened this issue Aug 26, 2021 · 7 comments
Open

Secret rotation / secret loss recovery #16832

clarfonthey opened this issue Aug 26, 2021 · 7 comments
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP type/proposal The new feature has not been accepted yet but needs to be discussed first.

Comments

@clarfonthey
Copy link
Contributor

clarfonthey commented Aug 26, 2021

I've run into issues like #1851 in the past and didn't realise it was due to the SECRET_KEY changing when recreating a config. It would be nice if we could somehow add some mitigations to this.

Recovery from secret loss

If the secret is changed without access to the previous secret, and things like 2FA secrets can't be decrypted, then there should be some easy way to mitigate this. Right now, the only solution is to delete the corrupted rows in the DB manually.

A few potential options:

  • A gitea doctor command to delete 2FA data, potentially notifying users who had 2FA enabled
  • A flag on users in the database to prompt them to reconfigure 2FA on login

Proactive secret rotation

If the user wishes to proactively change the secret, there should be an option to include (at least) two secrets in the config: the new secret for future operations, and the old secret for past operations.

A few options here:

  • Secrets could be configured in optional pairs, where there's a "primary" secret and a "backup" secret. The backup secret is exclusively for decryption, and anything decrypted with it should be re-encrypted with the primary secret.
  • There could also be the option for more than one backup secret.
  • A gitea doctor command to re-encrypt things encrypted with a backup secret.
  • A cron job to re-encrypt things encrypted with a backup secret.
  • The ability to automatically remove the backup secret once all its data is updated.

The actual action upon rotation of the secret depends on the reason for the rotation. If there's some kind of breach of security, things that were protected by the secret like 2FA keys should be regenerated. So, a few options here:

  • Secrets could be able to be marked as compromised, and this could also be used with the user flag mentioned in the first section, prompting users to reconfigure 2FA on login.
  • This could potentially up the priority on the cron job for re-encrypting things, putting it to a high priority.
@wxiaoguang wxiaoguang added issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented priority/medium labels Dec 8, 2021
@NotNite
Copy link

NotNite commented Apr 22, 2023

For any people unfortunate enough to be frantically Googling this at 11 PM like I was: I was moving my infrastructure to Nix, and the NixOS module for Gitea automatically generated a secret key for me, and so I couldn't log in because it wasn't using the default.

In case anyone needs it, the default secret key is here.

lunny pushed a commit that referenced this issue May 7, 2023
Help some users like #16832 #1851

There are many users reporting similar problem: if the SECRET_KEY
mismatches, some operations (like 2FA login) only reports unclear 500
error and unclear "base64 decode error" log (some maintainers ever spent
a lot of time on debugging such problem)

The SECRET_KEY was not well-designed and it is also a kind of technical
debt. Since it couldn't be fixed easily, it's good to add clearer error
messages, then at least users could know what the real problem is.

---------

Co-authored-by: delvh <dev.lh@web.de>
lunny pushed a commit that referenced this issue May 7, 2023
Backport #24573

Help some users like #16832 #1851

There are many users reporting similar problem: if the SECRET_KEY
mismatches, some operations (like 2FA login) only reports unclear 500
error and unclear "base64 decode error" log (some maintainers ever spent
a lot of time on debugging such problem)

The SECRET_KEY was not well-designed and it is also a kind of technical
debt. Since it couldn't be fixed easily, it's good to add clearer error
messages, then at least users could know what the real problem is.
@Renderer6060
Copy link

id also like to be able to change this from the default

@techknowlogick
Copy link
Member

An approach we could take is what minio does where they will take old and new key as config option to rotate

@clarfonthey
Copy link
Contributor Author

clarfonthey commented Dec 14, 2023

Note that this is included in the original description, albeit probably not clearly enough. I'll take another pass through it and see what I can do to make that clearer.

EDIT: I've updated the original post to use bulleted lists wherever possible, so that it's hopefully clearer to skim and see the potential tasks people could work on. IMHO we should probably create separate issues for the individual pieces.

@lunny lunny added type/proposal The new feature has not been accepted yet but needs to be discussed first. issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP labels Jan 4, 2024
@r10r
Copy link

r10r commented Jan 24, 2024

I have a similar issue after upgrading an installation from the gitea helm chart. In the installation the SECRET_KEY was not set and so values in the database are not encrypted. But after an
upgrade the helm chart generated a fresh non-emptySECRET_KEY resulting in the error when trying to access /admin/auths

...s/web/admin/auths.go:53:Authentications() [E] auth.Sources: failed to decrypt by secret, the key (maybe SECRET_KEY?)

Setting theSECRET_KEY to an empty value works - but how can I start using a non-empty SECRET_KEY now ?

@3bbbeau
Copy link

3bbbeau commented Jul 7, 2024

For me I was getting this issue on every runner following a restored PostgreSQL dump for Gitea:

level=error msg="failed to fetch task" error="unknown: rpc error: code = Internal desc = pick task: GetSecretsOfTask: failed to decrypt by secret, the key (maybe SECRET_KEY?) might be incorrect: AesDecrypt invalid decrypted base64 string: illegal base64 data at input byte 1"

I removed the secrets for the org/repo (simply updating them did not work), re-added them, and then the error went away and CI jobs ran as expected.

@Brawl345
Copy link

Brawl345 commented Sep 17, 2024

For any people unfortunate enough to be frantically Googling this at 11 PM like I was: I was moving my infrastructure to Nix, and the NixOS module for Gitea automatically generated a secret key for me, and so I couldn't log in because it wasn't using the default.

In case anyone needs it, the default secret key is here.

And what needs to be set? Copied my old gitea data where there is no SECRET_KEY inside the app.ini. I tried setting the SECRET_KEY to the default value but it still doesn't work, still getting this AesDecrypt error. Can't login anymore, this is very frustrating and undocumented...

EDIT: "Fixed" it by deleting the relevant entries from the two_factor database table and regenerating the 2FA token...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented issue/critical This issue should be fixed ASAP. If it is a PR, the PR should be merged ASAP type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

No branches or pull requests

10 participants