- Notifications
You must be signed in to change notification settings - Fork913
fix(coderd): ensure that clearing invalid oauth refresh tokens works with dbcrypt#15721
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| -- Required for sqlc to generate a parameter for the oauth_refresh_token_key_id | ||
| @oauth_refresh_token_key_id :: text = @oauth_refresh_token_key_id :: text; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
self-review: this is yuck. We don't actually need this parameter in the query but we need in the params for dbcrypt. This is the 'best' way I could find to set it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I feel pretty numb now to these kinda sqlc hacks now it doesn't phase me 🚶
| // If you're looking here, you're probably in trouble. | ||
| // Here's what you need to do: | ||
| // 1. Get the current CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS environment variable. | ||
| // 2. Run the following command: | ||
| // ENCRYPT_ME="<value to encrypt>" CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS="<secret keys here>" go test -v -count=1 ./enterprise/dbcrypt -test.run='^TestHelpMeEncryptSomeValue$' | ||
| // 3. Copy the value from the test output and do what you need with it. | ||
| func TestHelpMeEncryptSomeValue(t *testing.T) { | ||
| t.Parallel() | ||
| t.Skip("this only exists if you need to encrypt a value with dbcrypt, it does not actually test anything") | ||
| valueToEncrypt := os.Getenv("ENCRYPT_ME") | ||
| t.Logf("valueToEncrypt: %q", valueToEncrypt) | ||
| keys := os.Getenv("CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS") | ||
| require.NotEmpty(t, keys, "Set the CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS environment variable to use this") | ||
| base64Keys := strings.Split(keys, ",") | ||
| activeKey := base64Keys[0] | ||
| decodedKey, err := base64.StdEncoding.DecodeString(activeKey) | ||
| require.NoError(t, err, "the active key should be valid base64") | ||
| cipher, err := cipherAES256(decodedKey) | ||
| require.NoError(t, err) | ||
| t.Logf("cipher digest: %+v", cipher.HexDigest()) | ||
| encryptedEmptyString, err := cipher.Encrypt([]byte(valueToEncrypt)) | ||
| require.NoError(t, err) | ||
| t.Logf("encrypted and base64-encoded: %q", base64.StdEncoding.EncodeToString(encryptedEmptyString)) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
self-review: we could potentially make this a proper CLI function for use in a pinch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Seems pretty reasonable to leave as a test for now
| // If you're looking here, you're probably in trouble. | ||
| // Here's what you need to do: | ||
| // 1. Get the current CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS environment variable. | ||
| // 2. Run the following command: | ||
| // ENCRYPT_ME="<value to encrypt>" CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS="<secret keys here>" go test -v -count=1 ./enterprise/dbcrypt -test.run='^TestHelpMeEncryptSomeValue$' | ||
| // 3. Copy the value from the test output and do what you need with it. | ||
| func TestHelpMeEncryptSomeValue(t *testing.T) { | ||
| t.Parallel() | ||
| t.Skip("this only exists if you need to encrypt a value with dbcrypt, it does not actually test anything") | ||
| valueToEncrypt := os.Getenv("ENCRYPT_ME") | ||
| t.Logf("valueToEncrypt: %q", valueToEncrypt) | ||
| keys := os.Getenv("CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS") | ||
| require.NotEmpty(t, keys, "Set the CODER_EXTERNAL_TOKEN_ENCRYPTION_KEYS environment variable to use this") | ||
| base64Keys := strings.Split(keys, ",") | ||
| activeKey := base64Keys[0] | ||
| decodedKey, err := base64.StdEncoding.DecodeString(activeKey) | ||
| require.NoError(t, err, "the active key should be valid base64") | ||
| cipher, err := cipherAES256(decodedKey) | ||
| require.NoError(t, err) | ||
| t.Logf("cipher digest: %+v", cipher.HexDigest()) | ||
| encryptedEmptyString, err := cipher.Encrypt([]byte(valueToEncrypt)) | ||
| require.NoError(t, err) | ||
| t.Logf("encrypted and base64-encoded: %q", base64.StdEncoding.EncodeToString(encryptedEmptyString)) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Seems pretty reasonable to leave as a test for now
| -- Required for sqlc to generate a parameter for the oauth_refresh_token_key_id | ||
| @oauth_refresh_token_key_id :: text = @oauth_refresh_token_key_id :: text; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I feel pretty numb now to these kinda sqlc hacks now it doesn't phase me 🚶
e744cde intomainUh oh!
There was an error while loading.Please reload this page.
…with dbcrypt (#15721)#15608 introduced a buggy behaviourwith dbcrypt enabled.When clearing an oauth refresh token, we had been setting the value tothe empty string.The database encryption package considers decrypting an empty string tobe an error, as an empty encrypted string value will still have a nonceassociated with it and thus not actually be empty when stored at rest.Instead of 'deleting' the refresh token, 'update' it to be the emptystring.This plays nicely with dbcrypt.It also adds a 'utility test' in the dbcrypt package to help encrypt avalue. This was useful when manually fixing users affected by this bugon our dogfood instance.(cherry picked from commite744cde)
Uh oh!
There was an error while loading.Please reload this page.
#15608 introduced a buggy behaviour with dbcrypt enabled.
When clearing an oauth refresh token, we had been setting the value to the empty string.
The database encryption package considers decrypting an empty string to be an error, as an empty encrypted string value will still have a nonce associated with it and thus not actually be empty when stored at rest.
Instead of 'deleting' the refresh token, 'update' it to be the empty string.
This plays nicely with dbcrypt.
It also adds a 'utility test' in the dbcrypt package to help encrypt a value. This was useful when manually fixing users affected by this bug on our dogfood instance.
cc@bpmct@stirby