From 359bede628216503acc7616fdf90778d099d401f Mon Sep 17 00:00:00 2001 From: Seth Vargo Date: Tue, 1 Sep 2020 20:08:24 -0400 Subject: [PATCH] Allow rotating most secrets --- docs/production.md | 179 +++++++++++++++++++++++++++++++++ pkg/config/server_config.go | 5 +- pkg/database/authorized_app.go | 84 ++++++++++++---- pkg/database/config.go | 6 +- pkg/database/database.go | 4 +- pkg/database/database_util.go | 30 +++--- pkg/database/migrations.go | 6 +- pkg/database/token.go | 18 ++-- pkg/database/token_test.go | 21 ++-- pkg/database/vercode.go | 67 ++++++++---- 10 files changed, 340 insertions(+), 80 deletions(-) diff --git a/docs/production.md b/docs/production.md index e415f470f..e204f57d0 100644 --- a/docs/production.md +++ b/docs/production.md @@ -2,6 +2,7 @@ This page includes helpful tips for configuring things in production: + ## Observability (tracing and metrics) The observability component is responsible for metrics. The following @@ -11,3 +12,181 @@ configurations are available: | ----------------------- | ------------------------------- | ----------- | OpenCensus Agent | `OCAGENT` | Use OpenCensus. | Stackdriver\* | `STACKDRIVER` | Use Stackdriver. + + +## Rotating secrets + +This section describes how to rotate secrets in the system. + +### Cookie keys + +**Recommended frequency:** 30 days, on breach + +The cookie keys are an array. The items at odd indicies are HMAC keys and the +items at even indicies are encryption keys. The HMAC key should be 64 bytes and +the encryption key should be 32. Both keys are supplied to this system in +base64, for example: + +```sh +export COOKIE_KEYS="ARLaFwAqBGIkm5pLjAveJuahtCnX2NLoAUz2kCZKrScUaUkEaxHSvJLVYb5yAPCc441Cho5n5yp8jdEmy6hyig==,RLjcRZeqc07s6dh3OK4CM1POjHDZHC+usNU1w/XNTjM=" +``` + +Even though the array is flat, each even/odd pairing is actually a tuple: + +```text +[, , , ] +``` + +To rotate the cookie keys, generate two new keys of the correct lengths as +specificed above and insert them **into the front** of the array. **Do not +remove the existing values from the array**, as doing so will invalidate all +existing sessions. + +```text +[, , , , , ] +``` + +Just as before, the new values should be base64-encoded: + +```sh +export COOKIE_KEYS="c8+OD0vpvT/FrtGAtHc1nYhtkYMhjEEHCLgzuIiKJbskAbMI7bJxSnlBMKmc2AQmo8QVAViJuKoopuSuXE7tYw==,KRN9OK/lcs/uBWKQ2/1I0g9KR/iL3/MHuCn6esI02fs=,ARLaFwAqBGIkm5pLjAveJuahtCnX2NLoAUz2kCZKrScUaUkEaxHSvJLVYb5yAPCc441Cho5n5yp8jdEmy6hyig==,RLjcRZeqc07s6dh3OK4CM1POjHDZHC+usNU1w/XNTjM=" +``` + +Upon deploying, all _new_ sessions will use these new keys. Old sessions will be +automatically upgraded on the next visit. After a period of time that you deem +acceptable (e.g. 30d), you can remove the older keys from the end of the array. + +You can use `openssl` or similar tooling to generate the secret. Note that this +is **not** preferred since it requires a human to see the plaintext secret. + +```sh +openssl rand -base64 64 | tr -d "\n" # or 32 +``` + +If you are using a secret manager, put this value in the secret manager and +provie its _reference_ in the environment. If you are not using a secret +manager, +provide this value directly in the environment. + + +### Cross-site request forgery (CSRF) key + +**Recommended frequency:** 90 days, on breach + +To rotate the key, generate a new 32-byte key. You can use `openssl` or similar: + +```sh +openssl rand -base64 32 | tr -d "\n" +``` + +Update the `CSRF_AUTH_KEY` environment variable and re-deploy. The system [only +supports a single key for CSRF](https://github.com/gorilla/csrf/issues/65). When +you deploy the new key, any existing open HTML forms will fail to submit as an +invalid request. + + +### Database encryption keys + +**Recommend frequency:** 30 days, on breach + +These keys control application-layer encryption of secrets before they are +stored in the database. For example, this key encrypts Twilio credentials so +they are not in plaintext in the database. Note that we only use the encryption +key where encryption is appropriate. For API keys and tokens, we HMAC the values +as their plaintext values are not required. + +To rotate the encryption keys, rotate them in the underlying key manager. Note +that old entries will still be encrypted with the old key. You do not need to +upgrade them so long as the older key version is still available in your key +manager. + +While unlikely, this may require you to update the `DB_ENCRYPTION_KEY` +environment variable. + + +### API Key Signature HMAC keys + +**Recommended frequency:** 90 days + +This key is used as the HMAC secret when signing API keys. API keys are signed +and verified using this value. Like cookies, it accepts an array of values. The +first item in the array is used to sign all new API keys, but all remaining +values are still accepted as valid. These keys should be at least 64 bytes, but 128 is recommended. + +To generate a new key: + +```sh +openssl rand -base64 128 | tr -d "\n" +``` + +Insert this new value **into the front** of the `DB_APIKEY_SIGNATURE_KEY` +environment variable: + +```sh +DB_APIKEY_SIGNATURE_KEY="gSEGlr482MSTm0eGRm2VvS86iQin3+/+80ALBkKKBYgu2EJyhGkvi8BULeFQDW/qZp2y3IbKY0MUTqioG7InBZdCkisYjr8UNuA+PONxMSaw/x8m+CXF28qb2WF0OGYQIPgbOdQ7cChG3Ox5AQgWFmNwyr486lTxSM8TE7dfCfU=,oXrnYzt6MXKBB/+zZWTvkUABW8SSCAFv5Mc475sSVPGBqCf1hWvv/VmByFj/5457Ho0AVbDUiCpKnjW2Q8ZlxqRo5dJyRifwvmW2JYcpxe+Ff/d+tb2x+TwlzqEzrKVatEWX4cmMG7ZP6B1oTCw/uZPTDhgB3lerXVIBTxdAaQc=" +``` + +Note: Removing any of the keys from this list will invalidate API keys signed by +that version. + + +### API Key Database HMAC keys + +**Recommended frequency:** 90 days + +This key is used as the HMAC secret when saving hashed API keys in the database. +Like cookies, it accepts an array of values. The first item in the array is used +to HMAC all new API keys, but all remaining values are still accepted as valid. +These keys should be at least 64 bytes, but 128 is recommended. + +To generate a new key: + +```sh +openssl rand -base64 128 | tr -d "\n" +``` + +Insert this new value **into the front** of the `DB_APIKEY_DATABASE_KEY` +environment variable: + +```sh +DB_APIKEY_SIGNATURE_KEY="1do5HM96Bk9WD15BQC3qbW9e3T2V6T0DHn2i1xGJRKX8tZubxuaezivMhO3uJFEye8SITH3UFB+mo9oE0VGPiP/4TOXejfsd1g2L518itJbrK4/qNh6QMk0I04mqNtR55uvyt7G/ObADn2hQDYMVOGg/C6nLiqO+nqQ/UoUcGkA=,tJiUPEi0xS5QbykypVlquWsxQ0DAgxY41w+PkNqpoqzWQyDnEUAWFwIFUUFllqT+m0f2Kqh8EB1zjYgFcGP16O52rHer5sr4x6nsnQ/AiOHDrztJnEqGvutetHhZHLGKY0HxlxkZxcFLCmbgs6pa0vNUodrzOsCYysD7MLCSL5M=" +``` + +Note: Removing any of the keys from this list will invalidate API keys HMACed by +that version. + + +### Verification Code Database HMAC Key + +**Recommended frequency:** 30 days + +This key is used as the HMAC secret when saving verification codes in the +database. Like cookies, it accepts an array of values. The first item in the +array is used to HMAC all new verification codes, but all remaining values are +still accepted as valid. These keys should be at least 64 bytes, but 128 is +recommended. + +To generate a new key: + +```sh +openssl rand -base64 128 | tr -d "\n" +``` + +Insert this new value **into the front** of the `DB_VERIFICATION_CODE_DATABASE_KEY` +environment variable: + +```sh +DB_APIKEY_SIGNATURE_KEY="g7GdsjuN+eydQIUCena2gleSHsmu46Gs+62ENViXsaV123AoVEwZ94ywpCQ2hxJ6CSBc4wXOwrxhg+psiwfp9eyBcpOFC7i98mOTLu1gxznZe943PVKl9vKJx9SgFrSnI1prWoQj85xGJKMBlM/pj608LWpZ3aIxyk0t7Sk/iWE=,G1VCqQVqe+4GD60YsqOHVgYEXN6WMh8tuF9bAfRJyt9sVk9kBWbPdhFQVUdCqoE3cckSsxz7LMApiN1/2jbwG3qkTicx4YuwQMUDVg2Stdv0L2kvek/+MYcA0lVYaNZWBJCSgmaMzjzOGW/BsbR/ssX1WhGI9aVoGpFQMiEJaVE=" +``` + +Note: Removing any of the keys from this list will invalidate verification codes +HMACed by that version. However, given verification a verification code's +lifetime is short, it is probably safe to remove the key beyond 30 days. + + +### Certificate and token signing keys + +**Recommended frequency:** on demand + +If you are using system keys, the system administrator will handle rotation. If +you are using realm keys, you can generate new keys in the UI. diff --git a/pkg/config/server_config.go b/pkg/config/server_config.go index 76bc3e073..14660382b 100644 --- a/pkg/config/server_config.go +++ b/pkg/config/server_config.go @@ -44,9 +44,8 @@ type ServerConfig struct { SessionDuration time.Duration `env:"SESSION_DURATION,default=24h"` RevokeCheckPeriod time.Duration `env:"REVOKE_CHECK_DURATION,default=5m"` - // CookieKeys is a slice of bytes. The odd values are hash keys to HMAC the - // cookies. The even values are block keys to encrypt the cookie. Both keys - // should be 64 bytes. The value's should be specified as base64 encoded. + // CookieKeys is a slice of bytes. Tthe first is 64 bytes, the second is 32. + // They should be base64-encoded. CookieKeys Base64ByteSlice `env:"COOKIE_KEYS,required"` // CookieDomain is the domain for which cookie should be valid. diff --git a/pkg/database/authorized_app.go b/pkg/database/authorized_app.go index 16e231f0d..8b30c8c82 100644 --- a/pkg/database/authorized_app.go +++ b/pkg/database/authorized_app.go @@ -121,7 +121,7 @@ func (r *Realm) CreateAuthorizedApp(db *Database, app *AuthorizedApp) (string, e } apiKey := parts[0] - hmacedKey, err := db.hmacAPIKey(apiKey) + hmacedKey, err := db.GenerateAPIKeyHMAC(apiKey) if err != nil { return "", fmt.Errorf("failed to create hmac: %w", err) } @@ -146,7 +146,7 @@ func (db *Database) FindAuthorizedAppByAPIKey(apiKey string) (*AuthorizedApp, er return nil, err } - hmacedKey, err := db.hmacAPIKey(apiKey) + hmacedKeys, err := db.generateAPIKeyHMACs(apiKey) if err != nil { return nil, fmt.Errorf("failed to create hmac: %w", err) } @@ -154,7 +154,7 @@ func (db *Database) FindAuthorizedAppByAPIKey(apiKey string) (*AuthorizedApp, er // Find the API key that matches the constraints. var app AuthorizedApp if err := db.db. - Where("api_key = ?", hmacedKey). + Where("api_key IN (?)", hmacedKeys). Where("realm_id = ?", realmID). First(&app). Error; err != nil { @@ -163,17 +163,15 @@ func (db *Database) FindAuthorizedAppByAPIKey(apiKey string) (*AuthorizedApp, er return &app, nil } - // The API key is either invalid or a v1 API key. We need to check both the - // HMACed value and the plaintext value since earlier versions of the API keys - // were not HMACed. - hmacedKey, err := db.hmacAPIKey(apiKey) + // The API key is either invalid or a v1 API key. + hmacedKeys, err := db.generateAPIKeyHMACs(apiKey) if err != nil { return nil, fmt.Errorf("failed to create hmac: %w", err) } var app AuthorizedApp if err := db.db. - Or("api_key = ?", hmacedKey). + Or("api_key IN (?)", hmacedKeys). First(&app). Error; err != nil { return nil, err @@ -229,16 +227,35 @@ func (db *Database) SaveAuthorizedApp(r *AuthorizedApp) error { return db.db.Save(r).Error } -// hmacAPIKey is a helper for generating the HMAC of an API key. It returns the -// hex-encoded HMACed value, suitable for insertion into the database. -func (db *Database) hmacAPIKey(v string) (string, error) { - sig := hmac.New(sha512.New, db.config.APIKeyDatabaseHMAC) - if _, err := sig.Write([]byte(v)); err != nil { - return "", nil +// GenerateAPIKeyHMAC generates the HMAC of the provided API key using the +// latest HMAC key. +func (db *Database) GenerateAPIKeyHMAC(apiKey string) (string, error) { + keys := db.config.APIKeyDatabaseHMAC + if len(keys) < 1 { + return "", fmt.Errorf("expected at least 1 hmac key") + } + + sig := hmac.New(sha512.New, keys[0]) + if _, err := sig.Write([]byte(apiKey)); err != nil { + return "", err } return base64.RawURLEncoding.EncodeToString(sig.Sum(nil)), nil } +// generateAPIKeyHMACs creates a permutation of all possible API keys based on +// the provided HMACs. It's primarily used to find an API key in the database. +func (db *Database) generateAPIKeyHMACs(apiKey string) ([]string, error) { + sigs := make([]string, 0, len(db.config.APIKeyDatabaseHMAC)) + for _, key := range db.config.APIKeyDatabaseHMAC { + sig := hmac.New(sha512.New, key) + if _, err := sig.Write([]byte(apiKey)); err != nil { + return nil, err + } + sigs = append(sigs, base64.RawURLEncoding.EncodeToString(sig.Sum(nil))) + } + return sigs, nil +} + // GenerateAPIKey generates a new API key that is bound to the given realm. This // API key is NOT stored in the database. API keys are of the format: // @@ -267,15 +284,32 @@ func (db *Database) GenerateAPIKey(realmID uint) (string, error) { return key, nil } -// GenerateAPIKeySignature signs the given API key using an HMAC shared secret. -func (db *Database) GenerateAPIKeySignature(key string) ([]byte, error) { - sig := hmac.New(sha512.New, db.config.APIKeySignatureHMAC) - if _, err := sig.Write([]byte(key)); err != nil { +// GenerateAPIKeySignature returns all possible signatures of the given key. +func (db *Database) GenerateAPIKeySignature(apiKey string) ([]byte, error) { + keys := db.config.APIKeySignatureHMAC + if len(keys) < 1 { + return nil, fmt.Errorf("expected at least 1 hmac key") + } + sig := hmac.New(sha512.New, keys[0]) + if _, err := sig.Write([]byte(apiKey)); err != nil { return nil, err } return sig.Sum(nil), nil } +// generateAPIKeySignatures returns all possible signatures of the given key. +func (db *Database) generateAPIKeySignatures(apiKey string) ([][]byte, error) { + sigs := make([][]byte, 0, len(db.config.APIKeySignatureHMAC)) + for _, key := range db.config.APIKeySignatureHMAC { + sig := hmac.New(sha512.New, key) + if _, err := sig.Write([]byte(apiKey)); err != nil { + return nil, err + } + sigs = append(sigs, sig.Sum(nil)) + } + return sigs, nil +} + // VerifyAPIKeySignature verifies the signature matches the expected value for // the key. It does this by computing the expected signature and then doing a // constant-time comparison against the provided signature. @@ -292,13 +326,21 @@ func (db *Database) VerifyAPIKeySignature(key string) (string, uint, error) { } // Generate the expected signature. - expSig, err := db.GenerateAPIKeySignature(parts[0] + "." + parts[1]) + expSigs, err := db.generateAPIKeySignatures(parts[0] + "." + parts[1]) if err != nil { return "", 0, fmt.Errorf("invalid API key format: signature invalid") } - // Compare (this is an equal-time algorithm). - if !hmac.Equal(gotSig, expSig) { + found := false + for _, expSig := range expSigs { + // Compare (this is an equal-time algorithm). + if hmac.Equal(gotSig, expSig) { + found = true + // break // No! Don't break - we want constant time! + } + } + + if !found { return "", 0, fmt.Errorf("invalid API key format: signature invalid") } diff --git a/pkg/database/config.go b/pkg/database/config.go index eae71da9f..f630a0734 100644 --- a/pkg/database/config.go +++ b/pkg/database/config.go @@ -62,15 +62,15 @@ type Config struct { // APIKeyDatabaseHMAC is the HMAC key to use for API keys before storing them // in the database. - APIKeyDatabaseHMAC envconfig.Base64Bytes `env:"DB_APIKEY_DATABASE_KEY,required" json:"-"` + APIKeyDatabaseHMAC []envconfig.Base64Bytes `env:"DB_APIKEY_DATABASE_KEY,required" json:"-"` // APIKeySignatureHMAC is the HMAC key to sign API keys before returning them // to the requestor. - APIKeySignatureHMAC envconfig.Base64Bytes `env:"DB_APIKEY_SIGNATURE_KEY,required" json:"-"` + APIKeySignatureHMAC []envconfig.Base64Bytes `env:"DB_APIKEY_SIGNATURE_KEY,required" json:"-"` // VerificationCodeDatabaseHMAC is the HMAC key to hash codes before storing // them in the database. - VerificationCodeDatabaseHMAC envconfig.Base64Bytes `env:"DB_VERIFICATION_CODE_DATABASE_KEY,required"` + VerificationCodeDatabaseHMAC []envconfig.Base64Bytes `env:"DB_VERIFICATION_CODE_DATABASE_KEY,required"` // Secrets is the secret configuration. This is used to resolve values that // are actually pointers to secrets before returning them to the caller. The diff --git a/pkg/database/database.go b/pkg/database/database.go index 537dfede9..cb693d831 100644 --- a/pkg/database/database.go +++ b/pkg/database/database.go @@ -174,8 +174,8 @@ func (db *Database) OpenWithCacher(ctx context.Context, cacher cache.Cacher) err callbacks.Query().After("gorm:after_query").Register("sms_configs:decrypt", callbackKMSDecrypt(ctx, db.keyManager, c.EncryptionKey, "sms_configs", "TwilioAuthToken")) // Verification codes - callbacks.Create().Before("gorm:create").Register("verification_codes:hmac_code", callbackHMAC(ctx, db.hmacVerificationCode, "verification_codes", "code")) - callbacks.Create().Before("gorm:create").Register("verification_codes:hmac_long_code", callbackHMAC(ctx, db.hmacVerificationCode, "verification_codes", "long_code")) + callbacks.Create().Before("gorm:create").Register("verification_codes:hmac_code", callbackHMAC(ctx, db.GenerateVerificationCodeHMAC, "verification_codes", "code")) + callbacks.Create().Before("gorm:create").Register("verification_codes:hmac_long_code", callbackHMAC(ctx, db.GenerateVerificationCodeHMAC, "verification_codes", "long_code")) // Cache clearing if cacher != nil { diff --git a/pkg/database/database_util.go b/pkg/database/database_util.go index 65a982348..07c6f7250 100644 --- a/pkg/database/database_util.go +++ b/pkg/database/database_util.go @@ -28,6 +28,7 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/ory/dockertest" + "github.com/sethvargo/go-envconfig" ) var ( @@ -90,8 +91,9 @@ func NewTestDatabaseWithConfig(tb testing.TB) (*Database, *Config) { // build database config. config := Config{ - APIKeyDatabaseHMAC: generateKey(tb, 128), - APIKeySignatureHMAC: generateKey(tb, 128), + APIKeyDatabaseHMAC: generateKeys(tb, 3, 128), + APIKeySignatureHMAC: generateKeys(tb, 3, 128), + VerificationCodeDatabaseHMAC: generateKeys(tb, 3, 128), User: username, Port: port, @@ -106,7 +108,7 @@ func NewTestDatabaseWithConfig(tb testing.TB) (*Database, *Config) { Keys: keys.Config{ KeyManagerType: keys.KeyManagerTypeInMemory, }, - EncryptionKey: base64.RawStdEncoding.EncodeToString(generateKey(tb, 32)), + EncryptionKey: base64.RawStdEncoding.EncodeToString(generateKeys(tb, 1, 32)[0]), } // Wait for the container to start - we'll retry connections in a loop below, @@ -143,17 +145,21 @@ func NewTestDatabase(tb testing.TB) *Database { return db } -func generateKey(tb testing.TB, length int) []byte { +func generateKeys(tb testing.TB, qty, length int) []envconfig.Base64Bytes { tb.Helper() - buf := make([]byte, length) - n, err := rand.Read(buf) - if err != nil { - tb.Fatal(err) - } - if n < length { - tb.Fatalf("insufficient bytes read: %v, expected %v", n, length) + keys := make([]envconfig.Base64Bytes, 0, qty) + for i := 0; i < qty; i++ { + buf := make([]byte, length) + n, err := rand.Read(buf) + if err != nil { + tb.Fatal(err) + } + if n < length { + tb.Fatalf("insufficient bytes read: %v, expected %v", n, length) + } + keys = append(keys, buf) } - return buf + return keys } diff --git a/pkg/database/migrations.go b/pkg/database/migrations.go index eaa647032..b2c689e3b 100644 --- a/pkg/database/migrations.go +++ b/pkg/database/migrations.go @@ -474,7 +474,7 @@ func (db *Database) getMigrations(ctx context.Context) *gormigrate.Gormigrate { } apiKeyPreview := app.APIKey[:6] - newAPIKey, err := db.hmacAPIKey(app.APIKey) + newAPIKey, err := db.GenerateAPIKeyHMAC(app.APIKey) if err != nil { return fmt.Errorf("failed to hmac %v: %w", app.Name, err) } @@ -730,7 +730,7 @@ func (db *Database) getMigrations(ctx context.Context) *gormigrate.Gormigrate { // Sanity if len(code.Code) < 20 { - h, err := db.hmacVerificationCode(code.Code) + h, err := db.GenerateVerificationCodeHMAC(code.Code) if err != nil { return err } @@ -740,7 +740,7 @@ func (db *Database) getMigrations(ctx context.Context) *gormigrate.Gormigrate { // Sanity if len(code.LongCode) < 20 { - h, err := db.hmacVerificationCode(code.LongCode) + h, err := db.GenerateVerificationCodeHMAC(code.LongCode) if err != nil { return err } diff --git a/pkg/database/token.go b/pkg/database/token.go index 9936554db..20add27ba 100644 --- a/pkg/database/token.go +++ b/pkg/database/token.go @@ -153,14 +153,7 @@ func (db *Database) ClaimToken(realmID uint, tokenID string, subject *Subject) e // // The long term token can be used later to sign keys when they are submitted. func (db *Database) VerifyCodeAndIssueToken(realmID uint, verCode string, acceptTypes api.AcceptTypes, expireAfter time.Duration) (*Token, error) { - buffer := make([]byte, tokenBytes) - _, err := rand.Read(buffer) - if err != nil { - return nil, fmt.Errorf("rand.Read: %v", err) - } - tokenID := base64.RawStdEncoding.EncodeToString(buffer) - - hmacedCode, err := db.hmacVerificationCode(verCode) + hmacedCodes, err := db.generateVerificationCodeHMACs(verCode) if err != nil { return nil, fmt.Errorf("failed to create hmac: %w", err) } @@ -173,8 +166,7 @@ func (db *Database) VerifyCodeAndIssueToken(realmID uint, verCode string, accept if err := tx. Set("gorm:query_option", "FOR UPDATE"). Where("realm_id = ?", realmID). - // TODO(sethvargo): remove plaintext token check after migrations. - Where("(code = ? OR code = ? OR long_code = ? OR long_code = ?)", hmacedCode, verCode, hmacedCode, verCode). + Where("(code IN (?) OR long_code IN (?))", hmacedCodes, hmacedCodes). First(&vc). Error; err != nil { if gorm.IsRecordNotFoundError(err) { @@ -204,6 +196,12 @@ func (db *Database) VerifyCodeAndIssueToken(realmID uint, verCode string, accept return err } + buffer := make([]byte, tokenBytes) + if _, err := rand.Read(buffer); err != nil { + return fmt.Errorf("failed to create token: %w", err) + } + tokenID := base64.RawStdEncoding.EncodeToString(buffer) + // Issue the token. Take the generated value and create a new long term token. tok = &Token{ TokenID: tokenID, diff --git a/pkg/database/token_test.go b/pkg/database/token_test.go index 54061d5f4..4ff7bfb57 100644 --- a/pkg/database/token_test.go +++ b/pkg/database/token_test.go @@ -219,11 +219,21 @@ func TestIssueToken(t *testing.T) { } for _, tc := range cases { + tc := tc + t.Run(tc.Name, func(t *testing.T) { + t.Parallel() - realm, err := db.CreateRealm(fmt.Sprintf("test realm - %s", tc.Name)) - if err != nil { - t.Fatalf("unable to create test realm") + realm := NewRealmWithDefaults(fmt.Sprintf("TestIssueToken/%s", tc.Name)) + if err := db.SaveRealm(realm); err != nil { + t.Fatal(err) + } + + // Extract the code before saving. After saving, the code on the struct + // will be the HMAC. + code := tc.Verification.Code + if tc.UseLongCode { + code = tc.Verification.LongCode } tc.Verification.RealmID = realm.ID @@ -235,11 +245,6 @@ func TestIssueToken(t *testing.T) { time.Sleep(tc.Delay) } - code := tc.Verification.Code - if tc.UseLongCode { - code = tc.Verification.LongCode - } - tok, err := db.VerifyCodeAndIssueToken(realm.ID, code, tc.Accept, tc.TokenAge) if err != nil { if tc.Error == "" { diff --git a/pkg/database/vercode.go b/pkg/database/vercode.go index b161d01a3..5892e298c 100644 --- a/pkg/database/vercode.go +++ b/pkg/database/vercode.go @@ -116,19 +116,34 @@ func (v *VerificationCode) FormatSymptomDate() string { return v.SymptomDate.Format("2006-01-02") } -// IsCodeExpired checks to see if the actual code provides is the -// short or long code and determines if it is expired based on that. +// IsCodeExpired checks to see if the actual code provided is the short or long +// code, and determines if it is expired based on that. func (db *Database) IsCodeExpired(v *VerificationCode, code string) (bool, error) { - // it's possible that this could be called with the already HMACd version. - hmacedCode, err := db.hmacVerificationCode(code) + if v == nil { + return false, fmt.Errorf("provided code is nil") + } + + // It's possible that this could be called with the already HMACd version. + possibles, err := db.generateVerificationCodeHMACs(code) if err != nil { return false, fmt.Errorf("failed to create hmac: %w", err) } + possibles = append(possibles, code) + + inList := func(needle string, haystack []string) bool { + for _, hay := range haystack { + if hay == needle { + return true + } + } + return false + } + now := time.Now().UTC() switch { - case v.Code == code || v.Code == hmacedCode: + case inList(v.Code, possibles): return !v.ExpiresAt.After(now), nil - case v.LongCode == code || v.LongCode == hmacedCode: + case inList(v.LongCode, possibles): return !v.LongExpiresAt.After(now), nil default: return true, fmt.Errorf("not found") @@ -170,15 +185,14 @@ func (v *VerificationCode) Validate(maxAge time.Duration) error { // FindVerificationCode find a verification code by the code number (can be short // code or long code). func (db *Database) FindVerificationCode(code string) (*VerificationCode, error) { - hmacedCode, err := db.hmacVerificationCode(code) + hmacedCodes, err := db.generateVerificationCodeHMACs(code) if err != nil { return nil, fmt.Errorf("failed to create hmac: %w", err) } var vc VerificationCode if err := db.db. - // TODO(sethvargo): remove non-hmaced lookup after migrations - Where("code = ? OR code = ? OR long_code = ? OR long_code = ?", hmacedCode, code, hmacedCode, code). + Where("code IN (?) OR long_code IN (?)", hmacedCodes, hmacedCodes). First(&vc). Error; err != nil { return nil, err @@ -236,14 +250,13 @@ func (db *Database) SaveVerificationCode(vc *VerificationCode, maxAge time.Durat // DeleteVerificationCode deletes the code if it exists. This is a hard delete. func (db *Database) DeleteVerificationCode(code string) error { - hmacedCode, err := db.hmacVerificationCode(code) + hmacedCodes, err := db.generateVerificationCodeHMACs(code) if err != nil { return fmt.Errorf("failed to create hmac: %w", err) } return db.db.Unscoped(). - // TODO(sethvargo): remove non-hmaced lookup after migrations - Where("code = ? OR code = ? OR long_code = ? OR long_code = ?", hmacedCode, code, hmacedCode, code). + Where("code IN (?) OR long_code IN (?)", hmacedCodes, hmacedCodes). Delete(&VerificationCode{}). Error } @@ -261,12 +274,30 @@ func (db *Database) PurgeVerificationCodes(maxAge time.Duration) (int64, error) return rtn.RowsAffected, rtn.Error } -// hmacVerificationCode is a helper for generating the HMAC of a token. It returns the -// hex-encoded HMACed value, suitable for insertion into the database. -func (db *Database) hmacVerificationCode(v string) (string, error) { - sig := hmac.New(sha512.New, db.config.VerificationCodeDatabaseHMAC) - if _, err := sig.Write([]byte(v)); err != nil { - return "", nil +// GenerateVerificationCodeHMAC generates the HMAC of the code using the latest +// key. +func (db *Database) GenerateVerificationCodeHMAC(verCode string) (string, error) { + keys := db.config.VerificationCodeDatabaseHMAC + if len(keys) < 1 { + return "", fmt.Errorf("expected at least 1 hmac key") + } + sig := hmac.New(sha512.New, keys[0]) + if _, err := sig.Write([]byte(verCode)); err != nil { + return "", err } return base64.RawURLEncoding.EncodeToString(sig.Sum(nil)), nil } + +// generateVerificationCodeHMACs is a helper for generating all possible HMACs of a +// token. +func (db *Database) generateVerificationCodeHMACs(v string) ([]string, error) { + sigs := make([]string, 0, len(db.config.VerificationCodeDatabaseHMAC)) + for _, key := range db.config.VerificationCodeDatabaseHMAC { + sig := hmac.New(sha512.New, key) + if _, err := sig.Write([]byte(v)); err != nil { + return nil, err + } + sigs = append(sigs, base64.RawURLEncoding.EncodeToString(sig.Sum(nil))) + } + return sigs, nil +}