Decouple DB encryption key from 'cookie signing secret' and tell users to keep it in a safe place
Dev: https://dev.gitlab.org/gitlab/gitlabhq/issues/2407
GitLab currently has one '.secret' file which serves multiple purposes.
- Gitlab::Application.config.secret_token
- Gitlab::Application.config.secret_key_base
- devise :two_factor_authenticatable, otp_secret_encryption_key (since 7.11)
The impact of rotating the first two secrets is much lower than that of rotating the third. If the first two get rotated, some users get signed out and confirmation links stop working. If the third is rotated, an admin needs to go into the rails console to disable 2FA for all users, and each of those users needs to remove GitLab from their authenticator app, add it again, and print / store a new set of recovery keys.
I was surprised to learn about this today when we accidentally rotated the secret.
What can we do to inform users and avoid such accidental rotation? Do we need to reconsider how we store the OTP (2FA) secret encryption key?
Jacob
I propose we create a separate file for the DB encryption secret and we start telling users to keep that secret in a safe place.
Marin
Is it wise to re-use the session/CSRF secret
Revoking one secret now does 2 things which is bad
Do we need to reconsider how we store the OTP (2FA) secret encryption key?
I think this will influence everyone who had 2fa enabled and it can be quite a pain to enforce. What would be interesting to consider is separating the current secret for 2fa into a separate file and create something(rake task?) that will duplicate the current one into this new file if 2fa is enabled. This way we get to keep all users with working 2fa and give admins an option of choosing to revoke secrets at any time.
Jacob
It occurred to me that technically, we could have a script that re-encrypts OTP secrets using a new otp_secret_encryption_key, because the user's OTP secrets are stored in the DB with symmetric encryption. Decrypt each user secret with the old encryption key and re-encrypt with the new key. For example, if we want to rotate the 'wonky secret token' on gitlab.com we could do so with minor inconvenience to our users if we re-encrypt their OTP secrets (barcodes) with the new secret token. That way their existing barcodes keep working. I am not sure what this adds to the discussion but I wanted to put it out there that this is possible.
Dmitriy
sounds great
Jacob
Assigning 7.13 because the current situation is dangerous for GitLab sysadmins: it is not obvious how important it is to keep the DB encryption secret safe, or that that secret even exists.
Dmitriy
I dont see how one
.secret
is more important than another.secret
but I am ok with decouple if. Wondering how we are going to do this without breaking 2FA for existing accounts. And what will be new name/location for another N+1 secret value? Maybe its time forconfig/secrets.yml
file with all secrets here?
Jacob
I dont see how one .secret is more important than another .secret
Lose the cookie signing secret: no big deal. Lose the DB encryption secret: users with 2FA can no longer sign in. I think the DB encryption secret is a lot more important.
And what will be new name/location for another N+1 secret value?
I don't know, this is a new thing for GitLab. But I think this is important and we need to come up with a design for it. Also, we are getting the same problem in GitLab CI when we store 'runner secrets' for the user in the DB: losing the DB encryption key is bad.
Wondering how we are going to do this without breaking 2FA for existing accounts.
Create a migration that reads the current .secret, decrypts data in the DB, and re-encrypts with the new key?
I think it would be great to have config/secret.yml where we place all our secret keys so we dont have
.gitlab_shell_secret
,.secret
,.very_secret
,.much_secret
files
gitlab_shell: "eff07d30...."
two_factor_auth: "eff07d30...."
session: "eff07d30...."
OK we can use a secrets.yml. What about the GitLab installation instructions? "Just download this package, install it, and store the DB encryption key in a safe place"?? cc sytse
Sytse
Instructions should be in 2FA docs for admins. I totally agree with storing everything in config/secrets.yml (Rails standard). Can we remove them from gitlab.rb?
Marin
We have
gitlab-secrets.json
that is read by omnibus-gitlab which contains some secrets. Removing all secrets from gitlab.rb will be a breaking change and possibly a large undertaking. This will also cause another split in the documentation ( "if you use version prior to X.X.X store here, otherwise here"). I do not see why this would be needed however considering thatgitlab.rb
is already owned by root and accesible to small subset of users. Only possible benefit I see is that you could dump all secrets using JSON but you can already do that when specifying attribute overrides
Jacob
Instructions should be in 2FA docs for admins.
Fair enough, but only if 2FA is disabled by default. Otherwise users start using it without admins knowing about it.
Can we remove them from gitlab.rb?
I do not understand this question. Do you mean moving them to a separate file in /etc/gitlab?
Sytse
- I'm OK with disabling 2FA by default and having a link to the docs in the UI where you enable it (and maybe a text warning)
- Move them from gitlab.rb to secret.yml, but I realize that maybe gitlab.rb will write secret.yml?
Marin
Move them from gitlab.rb to secret.yml, but I realize that maybe gitlab.rb will write secret.yml?
Why would we need to move secrets from gitlab.rb to secrets.yml? Omnibus-gitlab will have to write to secrets.yml anyway because application won't work without it. Moving anything from gitlab.rb to another file will complicate things for users further with little benefit; we've been radiating for over a year that
gitlab.rb
is the only place you need to change things in.
Marin
2 Move them from gitlab.rb to secret.yml, but I realize that maybe gitlab.rb will write secret.yml?
secrets.yml is a Rails 4.1 feature and it lives in the Rails folder (/opt/gitlab/embedded/service/gitlab-rails). gitlab.rb is an omnibus configuration file and it lives in /etc/gitlab. You could indeed say that 'gitlab.rb writes secrets.yml', I hope that what I just wrote explains why.
Jacob
OK so are these the next steps now?
- use a separate DB encryption key, store it in
config/secrets.yml
. Migrate the current 'cookie secret' to that value for existing installations - turn off 2FA by default
- next to the option to turn it on, link to documentation that explains to admins that/why they need to keep the DB encryption key in a safe place
Dmitriy
what do you mean by
turn off 2FA by default
?
Jacob
I think we should avoid the following:
- sysadmin installs GitLab without knowing about DB encryption key
- user enables 2FA without sysadmin knowing
- something happens, DB encryption key is lost (because the admin did not know it exists)
- user looses 2FA access If the sysadmin needs to explicitly enable 2FA, we can warn them at that time that they need to store the DB encryption key in a safe place.
Dmitriy
so you propose application setting where you enable 2FA feature?
Jacob
Yes
Marin
How will Omnibus interact with secrets in config/secrets.yml if at all?
sytse Until the decision is clearer on how it will be handled in GitLab (and CI) I don't think I have a clear vision on what needs to change in omnibus.
Dmitriy
jacob I am ok with that
Sytse
Same issue for GitLab CI is in https://dev.gitlab.org/gitlab/gitlab-ci/issues/277#note_52972
Jacob
Users keep losing the DB encryption key 'in the wild' https://gitlab.com/gitlab-org/gitlab-ce/issues/1960 , this issue needs a milestone.
2FA is out there, our poor documentation and tooling for admins is hurting people.
Job
jacobvosmaer I don't understand, I don't have proper context for this. This can be in 7.14 if it's important, of course.
Sytse
job It is causing data (token) loss at customers today