diff --git a/hack/test.env b/hack/test.env index f4f3d0e6e..409940314 100644 --- a/hack/test.env +++ b/hack/test.env @@ -114,3 +114,7 @@ GOTRUE_SAML_ENABLED="true" GOTRUE_SAML_PRIVATE_KEY="MIIEowIBAAKCAQEAszrVveMQcSsa0Y+zN1ZFb19cRS0jn4UgIHTprW2tVBmO2PABzjY3XFCfx6vPirMAPWBYpsKmXrvm1tr0A6DZYmA8YmJd937VUQ67fa6DMyppBYTjNgGEkEhmKuszvF3MARsIKCGtZqUrmS7UG4404wYxVppnr2EYm3RGtHlkYsXu20MBqSDXP47bQP+PkJqC3BuNGk3xt5UHl2FSFpTHelkI6lBynw16B+lUT1F96SERNDaMqi/TRsZdGe5mB/29ngC/QBMpEbRBLNRir5iUevKS7Pn4aph9Qjaxx/97siktK210FJT23KjHpgcUfjoQ6BgPBTLtEeQdRyDuc/CgfwIDAQABAoIBAGYDWOEpupQPSsZ4mjMnAYJwrp4ZISuMpEqVAORbhspVeb70bLKonT4IDcmiexCg7cQBcLQKGpPVM4CbQ0RFazXZPMVq470ZDeWDEyhoCfk3bGtdxc1Zc9CDxNMs6FeQs6r1beEZug6weG5J/yRn/qYxQife3qEuDMl+lzfl2EN3HYVOSnBmdt50dxRuX26iW3nqqbMRqYn9OHuJ1LvRRfYeyVKqgC5vgt/6Tf7DAJwGe0dD7q08byHV8DBZ0pnMVU0bYpf1GTgMibgjnLjK//EVWafFHtN+RXcjzGmyJrk3+7ZyPUpzpDjO21kpzUQLrpEkkBRnmg6bwHnSrBr8avECgYEA3pq1PTCAOuLQoIm1CWR9/dhkbJQiKTJevlWV8slXQLR50P0WvI2RdFuSxlWmA4xZej8s4e7iD3MYye6SBsQHygOVGc4efvvEZV8/XTlDdyj7iLVGhnEmu2r7AFKzy8cOvXx0QcLg+zNd7vxZv/8D3Qj9Jje2LjLHKM5n/dZ3RzUCgYEAzh5Lo2anc4WN8faLGt7rPkGQF+7/18ImQE11joHWa3LzAEy7FbeOGpE/vhOv5umq5M/KlWFIRahMEQv4RusieHWI19ZLIP+JwQFxWxS+cPp3xOiGcquSAZnlyVSxZ//dlVgaZq2o2MfrxECcovRlaknl2csyf+HjFFwKlNxHm2MCgYAr//R3BdEy0oZeVRndo2lr9YvUEmu2LOihQpWDCd0fQw0ZDA2kc28eysL2RROte95r1XTvq6IvX5a0w11FzRWlDpQ4J4/LlcQ6LVt+98SoFwew+/PWuyLmxLycUbyMOOpm9eSc4wJJZNvaUzMCSkvfMtmm5jgyZYMMQ9A2Ul/9SQKBgB9mfh9mhBwVPIqgBJETZMMXOdxrjI5SBYHGSyJqpT+5Q0vIZLfqPrvNZOiQFzwWXPJ+tV4Mc/YorW3rZOdo6tdvEGnRO6DLTTEaByrY/io3/gcBZXoSqSuVRmxleqFdWWRnB56c1hwwWLqNHU+1671FhL6pNghFYVK4suP6qu4BAoGBAMk+VipXcIlD67mfGrET/xDqiWWBZtgTzTMjTpODhDY1GZck1eb4CQMP5j5V3gFJ4cSgWDJvnWg8rcz0unz/q4aeMGl1rah5WNDWj1QKWMS6vJhMHM/rqN1WHWR0ZnV83svYgtg0zDnQKlLujqW4JmGXLMU7ur6a+e6lpa1fvLsP" GOTRUE_MAX_VERIFIED_FACTORS=10 GOTRUE_SMS_TEST_OTP_VALID_UNTIL="" +GOTRUE_SECURITY_DB_ENCRYPTION_ENCRYPT=true +GOTRUE_SECURITY_DB_ENCRYPTION_ENCRYPTION_KEY_ID=abc +GOTRUE_SECURITY_DB_ENCRYPTION_ENCRYPTION_KEY=pwFoiPyybQMqNmYVN0gUnpbfpGQV2sDv9vp0ZAxi_Y4 +GOTRUE_SECURITY_DB_ENCRYPTION_DECRYPTION_KEYS=abc:pwFoiPyybQMqNmYVN0gUnpbfpGQV2sDv9vp0ZAxi_Y4 diff --git a/internal/api/admin.go b/internal/api/admin.go index dfb5bf8fa..053a75d35 100644 --- a/internal/api/admin.go +++ b/internal/api/admin.go @@ -134,6 +134,7 @@ func (a *API) adminUserGet(w http.ResponseWriter, r *http.Request) error { func (a *API) adminUserUpdate(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() db := a.db.WithContext(ctx) + config := a.config user := getUser(ctx) adminUser := getAdminUser(ctx) params, err := a.getAdminParams(r) @@ -175,7 +176,7 @@ func (a *API) adminUserUpdate(w http.ResponseWriter, r *http.Request) error { return err } - if err := user.SetPassword(ctx, password); err != nil { + if err := user.SetPassword(ctx, password, config.Security.DBEncryption.Encrypt, config.Security.DBEncryption.EncryptionKeyID, config.Security.DBEncryption.EncryptionKey); err != nil { return err } } diff --git a/internal/api/admin_test.go b/internal/api/admin_test.go index c57659414..fa046045e 100644 --- a/internal/api/admin_test.go +++ b/internal/api/admin_test.go @@ -350,7 +350,10 @@ func (ts *AdminTestSuite) TestAdminUserCreate() { expectedPassword = fmt.Sprintf("%v", c.params["password"]) } - assert.Equal(ts.T(), c.expected["isAuthenticated"], u.Authenticate(context.Background(), expectedPassword)) + isAuthenticated, _, err := u.Authenticate(context.Background(), expectedPassword, ts.API.config.Security.DBEncryption.DecryptionKeys, ts.API.config.Security.DBEncryption.Encrypt, ts.API.config.Security.DBEncryption.EncryptionKeyID) + require.NoError(ts.T(), err) + + assert.Equal(ts.T(), c.expected["isAuthenticated"], isAuthenticated) // remove created user after each case require.NoError(ts.T(), ts.API.db.Destroy(u)) @@ -726,7 +729,8 @@ func (ts *AdminTestSuite) TestAdminUserDeleteFactor() { require.NoError(ts.T(), err, "Error making new user") require.NoError(ts.T(), ts.API.db.Create(u), "Error creating user") - f := models.NewFactor(u, "testSimpleName", models.TOTP, models.FactorStateVerified, "secretkey") + f := models.NewFactor(u, "testSimpleName", models.TOTP, models.FactorStateVerified) + require.NoError(ts.T(), f.SetSecret("secretkey", ts.Config.Security.DBEncryption.Encrypt, ts.Config.Security.DBEncryption.EncryptionKeyID, ts.Config.Security.DBEncryption.EncryptionKey)) require.NoError(ts.T(), ts.API.db.Create(f), "Error saving new test factor") // Setup request @@ -749,7 +753,8 @@ func (ts *AdminTestSuite) TestAdminUserGetFactors() { require.NoError(ts.T(), err, "Error making new user") require.NoError(ts.T(), ts.API.db.Create(u), "Error creating user") - f := models.NewFactor(u, "testSimpleName", models.TOTP, models.FactorStateUnverified, "secretkey") + f := models.NewFactor(u, "testSimpleName", models.TOTP, models.FactorStateUnverified) + require.NoError(ts.T(), f.SetSecret("secretkey", ts.Config.Security.DBEncryption.Encrypt, ts.Config.Security.DBEncryption.EncryptionKeyID, ts.Config.Security.DBEncryption.EncryptionKey)) require.NoError(ts.T(), ts.API.db.Create(f), "Error saving new test factor") // Setup request @@ -770,7 +775,8 @@ func (ts *AdminTestSuite) TestAdminUserUpdateFactor() { require.NoError(ts.T(), err, "Error making new user") require.NoError(ts.T(), ts.API.db.Create(u), "Error creating user") - f := models.NewFactor(u, "testSimpleName", models.TOTP, models.FactorStateUnverified, "secretkey") + f := models.NewFactor(u, "testSimpleName", models.TOTP, models.FactorStateUnverified) + require.NoError(ts.T(), f.SetSecret("secretkey", ts.Config.Security.DBEncryption.Encrypt, ts.Config.Security.DBEncryption.EncryptionKeyID, ts.Config.Security.DBEncryption.EncryptionKey)) require.NoError(ts.T(), ts.API.db.Create(f), "Error saving new test factor") var cases = []struct { diff --git a/internal/api/mfa.go b/internal/api/mfa.go index 07221fc2e..d2e8295f7 100644 --- a/internal/api/mfa.go +++ b/internal/api/mfa.go @@ -11,6 +11,7 @@ import ( "github.com/boombuler/barcode/qr" "github.com/gofrs/uuid" "github.com/pquerna/otp/totp" + "github.com/supabase/auth/internal/crypto" "github.com/supabase/auth/internal/hooks" "github.com/supabase/auth/internal/metering" "github.com/supabase/auth/internal/models" @@ -63,6 +64,7 @@ func (a *API) EnrollFactor(w http.ResponseWriter, r *http.Request) error { user := getUser(ctx) session := getSession(ctx) config := a.config + db := a.db.WithContext(ctx) if session == nil || user == nil { return internalServerError("A valid session and a registered user are required to enroll a factor") @@ -92,7 +94,7 @@ func (a *API) EnrollFactor(w http.ResponseWriter, r *http.Request) error { factorCount := len(factors) numVerifiedFactors := 0 - if err := models.DeleteExpiredFactors(a.db, config.MFA.FactorExpiryDuration); err != nil { + if err := models.DeleteExpiredFactors(db, config.MFA.FactorExpiryDuration); err != nil { return err } @@ -132,9 +134,12 @@ func (a *API) EnrollFactor(w http.ResponseWriter, r *http.Request) error { } svgData.End() - factor := models.NewFactor(user, params.FriendlyName, params.FactorType, models.FactorStateUnverified, key.Secret()) + factor := models.NewFactor(user, params.FriendlyName, params.FactorType, models.FactorStateUnverified) + if err := factor.SetSecret(key.Secret(), config.Security.DBEncryption.Encrypt, config.Security.DBEncryption.EncryptionKeyID, config.Security.DBEncryption.EncryptionKey); err != nil { + return err + } - err = a.db.Transaction(func(tx *storage.Connection) error { + err = db.Transaction(func(tx *storage.Connection) error { if terr := tx.Create(factor); terr != nil { pgErr := utilities.NewPostgresError(terr) if pgErr.IsUniqueConstraintViolated() { @@ -161,7 +166,7 @@ func (a *API) EnrollFactor(w http.ResponseWriter, r *http.Request) error { TOTP: TOTPObject{ // See: https://css-tricks.com/probably-dont-base64-svg/ QRCode: buf.String(), - Secret: factor.Secret, + Secret: key.Secret(), URI: key.URL(), }, }) @@ -170,13 +175,14 @@ func (a *API) EnrollFactor(w http.ResponseWriter, r *http.Request) error { func (a *API) ChallengeFactor(w http.ResponseWriter, r *http.Request) error { ctx := r.Context() config := a.config + db := a.db.WithContext(ctx) user := getUser(ctx) factor := getFactor(ctx) ipAddress := utilities.GetIPAddress(r) challenge := models.NewChallenge(factor, ipAddress) - if err := a.db.Transaction(func(tx *storage.Connection) error { + if err := db.Transaction(func(tx *storage.Connection) error { if terr := tx.Create(challenge); terr != nil { return terr } @@ -203,6 +209,7 @@ func (a *API) VerifyFactor(w http.ResponseWriter, r *http.Request) error { user := getUser(ctx) factor := getFactor(ctx) config := a.config + db := a.db.WithContext(ctx) params := &VerifyFactorParams{} if err := retrieveRequestParams(r, params); err != nil { @@ -214,7 +221,7 @@ func (a *API) VerifyFactor(w http.ResponseWriter, r *http.Request) error { return internalServerError(InvalidFactorOwnerErrorMessage) } - challenge, err := models.FindChallengeByID(a.db, params.ChallengeID) + challenge, err := models.FindChallengeByID(db, params.ChallengeID) if err != nil && models.IsNotFoundError(err) { return notFoundError(ErrorCodeMFAFactorNotFound, "MFA factor with the provided challenge ID not found") } else if err != nil { @@ -226,13 +233,18 @@ func (a *API) VerifyFactor(w http.ResponseWriter, r *http.Request) error { } if challenge.HasExpired(config.MFA.ChallengeExpiryDuration) { - if err := a.db.Destroy(challenge); err != nil { + if err := db.Destroy(challenge); err != nil { return internalServerError("Database error deleting challenge").WithInternalError(err) } return unprocessableEntityError(ErrorCodeMFAChallengeExpired, "MFA challenge %v has expired, verify against another challenge or create a new challenge.", challenge.ID) } - valid := totp.Validate(params.Code, factor.Secret) + secret, shouldReEncrypt, err := factor.GetSecret(config.Security.DBEncryption.DecryptionKeys, config.Security.DBEncryption.Encrypt, config.Security.DBEncryption.EncryptionKeyID) + if err != nil { + return internalServerError("Database error verifying MFA TOTP secret").WithInternalError(err) + } + + valid := totp.Validate(params.Code, secret) if config.Hook.MFAVerificationAttempt.Enabled { input := hooks.MFAVerificationAttemptInput{ @@ -248,7 +260,7 @@ func (a *API) VerifyFactor(w http.ResponseWriter, r *http.Request) error { } if output.Decision == hooks.HookRejection { - if err := models.Logout(a.db, user.ID); err != nil { + if err := models.Logout(db, user.ID); err != nil { return err } @@ -259,12 +271,22 @@ func (a *API) VerifyFactor(w http.ResponseWriter, r *http.Request) error { return forbiddenError(ErrorCodeMFAVerificationRejected, output.Message) } } + if !valid { + if shouldReEncrypt && config.Security.DBEncryption.Encrypt { + if err := factor.SetSecret(secret, true, config.Security.DBEncryption.EncryptionKeyID, config.Security.DBEncryption.EncryptionKey); err != nil { + return err + } + + if err := db.UpdateOnly(factor, "secret"); err != nil { + return err + } + } return unprocessableEntityError(ErrorCodeMFAVerificationFailed, "Invalid TOTP code entered") } var token *AccessTokenResponse - err = a.db.Transaction(func(tx *storage.Connection) error { + err = db.Transaction(func(tx *storage.Connection) error { var terr error if terr = models.NewAuditLogEntry(r, tx, user, models.VerifyFactorAction, r.RemoteAddr, map[string]interface{}{ "factor_id": factor.ID, @@ -280,6 +302,17 @@ func (a *API) VerifyFactor(w http.ResponseWriter, r *http.Request) error { return terr } } + if shouldReEncrypt && config.Security.DBEncryption.Encrypt { + es, terr := crypto.NewEncryptedString(factor.ID.String(), []byte(secret), config.Security.DBEncryption.EncryptionKeyID, config.Security.DBEncryption.EncryptionKey) + if terr != nil { + return terr + } + + factor.Secret = es.String() + if terr := tx.UpdateOnly(factor, "secret"); terr != nil { + return terr + } + } user, terr = models.FindUserByID(tx, user.ID) if terr != nil { return terr @@ -316,6 +349,8 @@ func (a *API) UnenrollFactor(w http.ResponseWriter, r *http.Request) error { user := getUser(ctx) factor := getFactor(ctx) session := getSession(ctx) + db := a.db.WithContext(ctx) + if factor == nil || session == nil || user == nil { return internalServerError("A valid session and factor are required to unenroll a factor") } @@ -327,7 +362,7 @@ func (a *API) UnenrollFactor(w http.ResponseWriter, r *http.Request) error { return internalServerError(InvalidFactorOwnerErrorMessage) } - err = a.db.Transaction(func(tx *storage.Connection) error { + err = db.Transaction(func(tx *storage.Connection) error { var terr error if terr := tx.Destroy(factor); terr != nil { return terr diff --git a/internal/api/mfa_test.go b/internal/api/mfa_test.go index 991cc52f9..63f813249 100644 --- a/internal/api/mfa_test.go +++ b/internal/api/mfa_test.go @@ -2,7 +2,6 @@ package api import ( "bytes" - "context" "encoding/json" "fmt" "net/http" @@ -14,15 +13,15 @@ import ( "github.com/gofrs/uuid" "database/sql" + "github.com/pkg/errors" "github.com/pquerna/otp" "github.com/supabase/auth/internal/conf" + "github.com/supabase/auth/internal/crypto" "github.com/supabase/auth/internal/models" "github.com/supabase/auth/internal/storage" "github.com/supabase/auth/internal/utilities" - "github.com/jackc/pgx/v4" - "github.com/pquerna/otp/totp" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" @@ -62,7 +61,8 @@ func (ts *MFATestSuite) SetupTest() { require.NoError(ts.T(), err, "Error creating test user model") require.NoError(ts.T(), ts.API.db.Create(u), "Error saving new test user") // Create Factor - f := models.NewFactor(u, "test_factor", models.TOTP, models.FactorStateUnverified, "secretkey") + f := models.NewFactor(u, "test_factor", models.TOTP, models.FactorStateUnverified) + require.NoError(ts.T(), f.SetSecret("secretkey", ts.Config.Security.DBEncryption.Encrypt, ts.Config.Security.DBEncryption.EncryptionKeyID, ts.Config.Security.DBEncryption.EncryptionKey)) require.NoError(ts.T(), ts.API.db.Create(f), "Error saving new test factor") // Create corresponding session s, err := models.NewSession(u.ID, &f.ID) @@ -482,14 +482,19 @@ func ServeAuthenticatedRequest(ts *MFATestSuite, method, path, token string, buf func performVerifyFlow(ts *MFATestSuite, challengeID, factorID uuid.UUID, token string, requireStatusOK bool) *httptest.ResponseRecorder { var buffer bytes.Buffer - conn, err := pgx.Connect(context.Background(), ts.API.db.URL()) + factor, err := models.FindFactorByFactorID(ts.API.db, factorID) require.NoError(ts.T(), err) + require.NotNil(ts.T(), factor) - defer conn.Close(context.Background()) + totpSecret := factor.Secret - var totpSecret string - err = conn.QueryRow(context.Background(), "select secret from mfa_factors where id=$1", factorID).Scan(&totpSecret) - require.NoError(ts.T(), err) + if es := crypto.ParseEncryptedString(factor.Secret); es != nil { + secret, err := es.Decrypt(factor.ID.String(), ts.API.config.Security.DBEncryption.DecryptionKeys) + require.NoError(ts.T(), err) + require.NotNil(ts.T(), secret) + + totpSecret = string(secret) + } code, err := totp.GenerateCode(totpSecret, time.Now().UTC()) require.NoError(ts.T(), err) diff --git a/internal/api/token.go b/internal/api/token.go index 542c68edf..11af2883f 100644 --- a/internal/api/token.go +++ b/internal/api/token.go @@ -145,7 +145,10 @@ func (a *API) ResourceOwnerPasswordGrant(ctx context.Context, w http.ResponseWri return oauthError("invalid_grant", InvalidLoginMessage) } - isValidPassword := user.Authenticate(ctx, params.Password) + isValidPassword, shouldReEncrypt, err := user.Authenticate(ctx, params.Password, config.Security.DBEncryption.DecryptionKeys, config.Security.DBEncryption.Encrypt, config.Security.DBEncryption.EncryptionKeyID) + if err != nil { + return err + } var weakPasswordError *WeakPasswordError if isValidPassword { @@ -156,6 +159,20 @@ func (a *API) ResourceOwnerPasswordGrant(ctx context.Context, w http.ResponseWri observability.GetLogEntry(r).Entry.WithError(err).Warn("Password strength check on sign-in failed") } } + + if shouldReEncrypt { + if err := user.SetPassword(ctx, params.Password, true, config.Security.DBEncryption.EncryptionKeyID, config.Security.DBEncryption.EncryptionKey); err != nil { + return err + } + + // directly change this in the database without + // calling user.UpdatePassword() because this + // is not a password change, just encryption + // change in the database + if err := db.UpdateOnly(user, "encrypted_password"); err != nil { + return err + } + } } if config.Hook.PasswordVerificationAttempt.Enabled { diff --git a/internal/api/user.go b/internal/api/user.go index 10fbc93d2..33c35aa57 100644 --- a/internal/api/user.go +++ b/internal/api/user.go @@ -153,12 +153,23 @@ func (a *API) UserUpdate(w http.ResponseWriter, r *http.Request) error { password := *params.Password if password != "" { - if user.EncryptedPassword != "" && user.Authenticate(ctx, password) { + isSamePassword := false + + if user.EncryptedPassword != "" { + auth, _, err := user.Authenticate(ctx, password, config.Security.DBEncryption.DecryptionKeys, false, "") + if err != nil { + return err + } + + isSamePassword = auth + } + + if isSamePassword { return unprocessableEntityError(ErrorCodeSamePassword, "New password should be different from the old password.") } } - if err := user.SetPassword(ctx, password); err != nil { + if err := user.SetPassword(ctx, password, config.Security.DBEncryption.Encrypt, config.Security.DBEncryption.EncryptionKeyID, config.Security.DBEncryption.EncryptionKey); err != nil { return err } } diff --git a/internal/api/user_test.go b/internal/api/user_test.go index ac97d9c24..8272bb87e 100644 --- a/internal/api/user_test.go +++ b/internal/api/user_test.go @@ -310,7 +310,10 @@ func (ts *UserTestSuite) TestUserUpdatePassword() { u, err = models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) - require.Equal(ts.T(), c.expected.isAuthenticated, u.Authenticate(context.Background(), c.newPassword)) + isAuthenticated, _, err := u.Authenticate(context.Background(), c.newPassword, ts.API.config.Security.DBEncryption.DecryptionKeys, ts.API.config.Security.DBEncryption.Encrypt, ts.API.config.Security.DBEncryption.EncryptionKeyID) + require.NoError(ts.T(), err) + + require.Equal(ts.T(), c.expected.isAuthenticated, isAuthenticated) }) } } @@ -369,7 +372,10 @@ func (ts *UserTestSuite) TestUserUpdatePasswordNoReauthenticationRequired() { u, err = models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) - require.Equal(ts.T(), c.expected.isAuthenticated, u.Authenticate(context.Background(), c.newPassword)) + isAuthenticated, _, err := u.Authenticate(context.Background(), c.newPassword, ts.API.config.Security.DBEncryption.DecryptionKeys, ts.API.config.Security.DBEncryption.Encrypt, ts.API.config.Security.DBEncryption.EncryptionKeyID) + require.NoError(ts.T(), err) + + require.Equal(ts.T(), c.expected.isAuthenticated, isAuthenticated) }) } } @@ -424,7 +430,10 @@ func (ts *UserTestSuite) TestUserUpdatePasswordReauthentication() { u, err = models.FindUserByEmailAndAudience(ts.API.db, "test@example.com", ts.Config.JWT.Aud) require.NoError(ts.T(), err) - require.True(ts.T(), u.Authenticate(context.Background(), "newpass")) + isAuthenticated, _, err := u.Authenticate(context.Background(), "newpass", ts.Config.Security.DBEncryption.DecryptionKeys, ts.Config.Security.DBEncryption.Encrypt, ts.Config.Security.DBEncryption.EncryptionKeyID) + require.NoError(ts.T(), err) + + require.True(ts.T(), isAuthenticated) require.Empty(ts.T(), u.ReauthenticationToken) require.Nil(ts.T(), u.ReauthenticationSentAt) } diff --git a/internal/api/verify.go b/internal/api/verify.go index 5badfc77e..91df8bc21 100644 --- a/internal/api/verify.go +++ b/internal/api/verify.go @@ -304,6 +304,8 @@ func (a *API) verifyPost(w http.ResponseWriter, r *http.Request, params *VerifyP } func (a *API) signupVerify(r *http.Request, ctx context.Context, conn *storage.Connection, user *models.User) (*models.User, error) { + config := a.config + if user.EncryptedPassword == "" && user.InvitedAt != nil { // sign them up with temporary password, and require application // to present the user with a password set form @@ -313,7 +315,7 @@ func (a *API) signupVerify(r *http.Request, ctx context.Context, conn *storage.C panic(err) } - if err := user.SetPassword(ctx, password); err != nil { + if err := user.SetPassword(ctx, password, config.Security.DBEncryption.Encrypt, config.Security.DBEncryption.EncryptionKeyID, config.Security.DBEncryption.EncryptionKey); err != nil { return nil, err } } diff --git a/internal/conf/configuration.go b/internal/conf/configuration.go index 2c8212abd..99f0f1879 100644 --- a/internal/conf/configuration.go +++ b/internal/conf/configuration.go @@ -2,6 +2,7 @@ package conf import ( "bytes" + "encoding/base64" "errors" "fmt" "net/url" @@ -421,16 +422,74 @@ func (c *CaptchaConfiguration) Validate() error { return nil } +// DatabaseEncryptionConfiguration configures Auth to encrypt certain columns. +// Once Encrypt is set to true, data will start getting encrypted with the +// provided encryption key. Setting it to false just stops encryption from +// going on further, but DecryptionKeys would have to contain the same key so +// the encrypted data remains accessible. +type DatabaseEncryptionConfiguration struct { + Encrypt bool `json:"encrypt"` + + EncryptionKeyID string `json:"encryption_key_id" split_words:"true"` + EncryptionKey string `json:"-" split_words:"true"` + + DecryptionKeys map[string]string `json:"-" split_words:"true"` +} + +func (c *DatabaseEncryptionConfiguration) Validate() error { + if c.Encrypt { + if c.EncryptionKeyID == "" { + return errors.New("conf: encryption key ID must be specified") + } + + decodedKey, err := base64.RawURLEncoding.DecodeString(c.EncryptionKey) + if err != nil { + return err + } + + if len(decodedKey) != 256/8 { + return errors.New("conf: encryption key is not 256 bits") + } + + if c.DecryptionKeys == nil || c.DecryptionKeys[c.EncryptionKeyID] == "" { + return errors.New("conf: encryption key must also be present in decryption keys") + } + } + + for id, key := range c.DecryptionKeys { + decodedKey, err := base64.RawURLEncoding.DecodeString(key) + if err != nil { + return err + } + + if len(decodedKey) != 256/8 { + return fmt.Errorf("conf: decryption key with ID %q must be 256 bits", id) + } + } + + return nil +} + type SecurityConfiguration struct { Captcha CaptchaConfiguration `json:"captcha"` RefreshTokenRotationEnabled bool `json:"refresh_token_rotation_enabled" split_words:"true" default:"true"` RefreshTokenReuseInterval int `json:"refresh_token_reuse_interval" split_words:"true"` UpdatePasswordRequireReauthentication bool `json:"update_password_require_reauthentication" split_words:"true"` ManualLinkingEnabled bool `json:"manual_linking_enabled" split_words:"true" default:"false"` + + DBEncryption DatabaseEncryptionConfiguration `json:"database_encryption" split_words:"true"` } func (c *SecurityConfiguration) Validate() error { - return c.Captcha.Validate() + if err := c.Captcha.Validate(); err != nil { + return err + } + + if err := c.DBEncryption.Validate(); err != nil { + return err + } + + return nil } func loadEnvironment(filename string) error { diff --git a/internal/crypto/crypto.go b/internal/crypto/crypto.go index 590d1ba4d..be6a2b5df 100644 --- a/internal/crypto/crypto.go +++ b/internal/crypto/crypto.go @@ -1,9 +1,12 @@ package crypto import ( + "crypto/aes" + "crypto/cipher" "crypto/rand" "crypto/sha256" "encoding/base64" + "encoding/json" "fmt" "io" "math" @@ -14,6 +17,7 @@ import ( "github.com/gofrs/uuid" standardwebhooks "github.com/standard-webhooks/standard-webhooks/libraries/go" + "golang.org/x/crypto/hkdf" "github.com/pkg/errors" ) @@ -69,3 +73,137 @@ func GenerateSignatures(secrets []string, msgID uuid.UUID, currentTime time.Time } return signatureList, nil } + +type EncryptedString struct { + KeyID string `json:"key_id"` + Algorithm string `json:"alg"` + Data []byte `json:"data"` + Nonce []byte `json:"nonce,omitempty"` +} + +func (es *EncryptedString) IsValid() bool { + return es.KeyID != "" && len(es.Data) > 0 && len(es.Nonce) > 0 && es.Algorithm == "aes-gcm-hkdf" +} + +// ShouldReEncrypt tells you if the value encrypted needs to be encrypted again with a newer key. +func (es *EncryptedString) ShouldReEncrypt(encryptionKeyID string) bool { + return es.KeyID != encryptionKeyID +} + +func (es *EncryptedString) Decrypt(id string, decryptionKeys map[string]string) ([]byte, error) { + decryptionKey := decryptionKeys[es.KeyID] + + if decryptionKey == "" { + return nil, fmt.Errorf("crypto: decryption key with name %q does not exist", es.KeyID) + } + + key, err := deriveSymmetricKey(id, es.KeyID, decryptionKey) + if err != nil { + return nil, err + } + + aes, err := aes.NewCipher(key) + if err != nil { + return nil, err + } + + cipher, err := cipher.NewGCM(aes) + if err != nil { + return nil, err + } + + decrypted, err := cipher.Open(nil, es.Nonce, es.Data, nil) + if err != nil { + return nil, err + } + + return decrypted, nil +} + +func ParseEncryptedString(str string) *EncryptedString { + if !strings.HasPrefix(str, "{") { + return nil + } + + var es EncryptedString + + if err := json.Unmarshal([]byte(str), &es); err != nil { + return nil + } + + if !es.IsValid() { + return nil + } + + return &es +} + +func (es *EncryptedString) String() string { + out, err := json.Marshal(es) + if err != nil { + panic(err) + } + + return string(out) +} + +func deriveSymmetricKey(id, keyID, keyBase64URL string) ([]byte, error) { + hkdfKey, err := base64.RawURLEncoding.DecodeString(keyBase64URL) + if err != nil { + return nil, err + } + + if len(hkdfKey) != 256/8 { + return nil, fmt.Errorf("crypto: key with ID %q is not 256 bits", keyID) + } + + // Since we use AES-GCM here, the same symmetric key *must not be used + // more than* 2^32 times. But, that's not that much. Suppose a system + // with 100 million users, then a user can only change their password + // 42 times. To prevent this, the actual symmetric key is derived by + // using HKDF using the encryption key and the "ID" of the object + // containing the encryption string. Ideally this ID is a UUID. This + // has the added benefit that the encrypted string is bound to that + // specific object, and can't accidentally be "moved" to other objects + // without changing their ID to the original one. + + keyReader := hkdf.New(sha256.New, hkdfKey, nil, []byte(id)) + key := make([]byte, 256/8) + + if _, err := io.ReadFull(keyReader, key); err != nil { + panic(err) + } + + return key, nil +} + +func NewEncryptedString(id string, data []byte, keyID string, keyBase64URL string) (*EncryptedString, error) { + key, err := deriveSymmetricKey(id, keyID, keyBase64URL) + if err != nil { + return nil, err + } + + aes, err := aes.NewCipher(key) + if err != nil { + return nil, err + } + + cipher, err := cipher.NewGCM(aes) + if err != nil { + panic(err) + } + + es := EncryptedString{ + KeyID: keyID, + Algorithm: "aes-gcm-hkdf", + Nonce: make([]byte, 12), + } + + if _, err := io.ReadFull(rand.Reader, es.Nonce); err != nil { + panic(err) + } + + es.Data = cipher.Seal(nil, es.Nonce, data, nil) + + return &es, nil +} diff --git a/internal/crypto/crypto_test.go b/internal/crypto/crypto_test.go new file mode 100644 index 000000000..b677b918d --- /dev/null +++ b/internal/crypto/crypto_test.go @@ -0,0 +1,34 @@ +package crypto + +import ( + "testing" + + "github.com/gofrs/uuid" + "github.com/stretchr/testify/assert" +) + +func TestEncryptedString(t *testing.T) { + id := uuid.Must(uuid.NewV4()).String() + + es, err := NewEncryptedString(id, []byte("data"), "key-id", "pwFoiPyybQMqNmYVN0gUnpbfpGQV2sDv9vp0ZAxi_Y4") + assert.NoError(t, err) + + assert.Equal(t, es.KeyID, "key-id") + assert.Equal(t, es.Algorithm, "aes-gcm-hkdf") + assert.Len(t, es.Data, 20) + assert.Len(t, es.Nonce, 12) + + dec := ParseEncryptedString(es.String()) + + assert.NotNil(t, dec) + assert.Equal(t, dec.Algorithm, "aes-gcm-hkdf") + assert.Len(t, dec.Data, 20) + assert.Len(t, dec.Nonce, 12) + + decrypted, err := dec.Decrypt(id, map[string]string{ + "key-id": "pwFoiPyybQMqNmYVN0gUnpbfpGQV2sDv9vp0ZAxi_Y4", + }) + + assert.NoError(t, err) + assert.Equal(t, []byte("data"), decrypted) +} diff --git a/internal/models/factor.go b/internal/models/factor.go index 265733564..53fddc260 100644 --- a/internal/models/factor.go +++ b/internal/models/factor.go @@ -9,6 +9,7 @@ import ( "github.com/gobuffalo/pop/v6" "github.com/gofrs/uuid" "github.com/pkg/errors" + "github.com/supabase/auth/internal/crypto" "github.com/supabase/auth/internal/storage" ) @@ -127,20 +128,46 @@ func (Factor) TableName() string { return tableName } -func NewFactor(user *User, friendlyName string, factorType string, state FactorState, secret string) *Factor { +func NewFactor(user *User, friendlyName string, factorType string, state FactorState) *Factor { id := uuid.Must(uuid.NewV4()) factor := &Factor{ - UserID: user.ID, ID: id, + UserID: user.ID, Status: state.String(), FriendlyName: friendlyName, - Secret: secret, FactorType: factorType, } return factor } +func (f *Factor) SetSecret(secret string, encrypt bool, encryptionKeyID, encryptionKey string) error { + f.Secret = secret + if encrypt { + es, err := crypto.NewEncryptedString(f.ID.String(), []byte(secret), encryptionKeyID, encryptionKey) + if err != nil { + return err + } + + f.Secret = es.String() + } + + return nil +} + +func (f *Factor) GetSecret(decryptionKeys map[string]string, encrypt bool, encryptionKeyID string) (string, bool, error) { + if es := crypto.ParseEncryptedString(f.Secret); es != nil { + bytes, err := es.Decrypt(f.ID.String(), decryptionKeys) + if err != nil { + return "", false, err + } + + return string(bytes), encrypt && es.ShouldReEncrypt(encryptionKeyID), nil + } + + return f.Secret, encrypt, nil +} + func FindFactorByFactorID(conn *storage.Connection, factorID uuid.UUID) (*Factor, error) { var factor Factor err := conn.Find(&factor, factorID) diff --git a/internal/models/factor_test.go b/internal/models/factor_test.go index c6d1f4a70..1ca782ce6 100644 --- a/internal/models/factor_test.go +++ b/internal/models/factor_test.go @@ -37,7 +37,8 @@ func (ts *FactorTestSuite) SetupTest() { require.NoError(ts.T(), err) require.NoError(ts.T(), ts.db.Create(user)) - factor := NewFactor(user, "asimplename", TOTP, FactorStateUnverified, "topsecret") + factor := NewFactor(user, "asimplename", TOTP, FactorStateUnverified) + require.NoError(ts.T(), factor.SetSecret("topsecret", false, "", "")) require.NoError(ts.T(), ts.db.Create(factor)) ts.TestFactor = factor } diff --git a/internal/models/user.go b/internal/models/user.go index 270484e08..721819eab 100644 --- a/internal/models/user.go +++ b/internal/models/user.go @@ -283,7 +283,7 @@ func (u *User) SetPhone(tx *storage.Connection, phone string) error { return tx.UpdateOnly(u, "phone") } -func (u *User) SetPassword(ctx context.Context, password string) error { +func (u *User) SetPassword(ctx context.Context, password string, encrypt bool, encryptionKeyID, encryptionKey string) error { if password == "" { u.EncryptedPassword = "" return nil @@ -295,6 +295,14 @@ func (u *User) SetPassword(ctx context.Context, password string) error { } u.EncryptedPassword = pw + if encrypt { + es, err := crypto.NewEncryptedString(u.ID.String(), []byte(pw), encryptionKeyID, encryptionKey) + if err != nil { + return err + } + + u.EncryptedPassword = es.String() + } return nil } @@ -332,9 +340,22 @@ func (u *User) UpdatePassword(tx *storage.Connection, sessionID *uuid.UUID) erro } // Authenticate a user from a password -func (u *User) Authenticate(ctx context.Context, password string) bool { - err := crypto.CompareHashAndPassword(ctx, u.EncryptedPassword, password) - return err == nil +func (u *User) Authenticate(ctx context.Context, password string, decryptionKeys map[string]string, encrypt bool, encryptionKeyID string) (bool, bool, error) { + hash := u.EncryptedPassword + + es := crypto.ParseEncryptedString(u.EncryptedPassword) + if es != nil { + h, err := es.Decrypt(u.ID.String(), decryptionKeys) + if err != nil { + return false, false, err + } + + hash = string(h) + } + + compareErr := crypto.CompareHashAndPassword(ctx, hash, password) + + return compareErr == nil, encrypt && (es == nil || es.ShouldReEncrypt(encryptionKeyID)), nil } // ConfirmReauthentication resets the reauthentication token diff --git a/internal/models/user_test.go b/internal/models/user_test.go index 6c915f6af..011cf28f0 100644 --- a/internal/models/user_test.go +++ b/internal/models/user_test.go @@ -372,9 +372,9 @@ func (ts *UserTestSuite) TestSetPasswordTooLong() { require.NoError(ts.T(), err) require.NoError(ts.T(), ts.db.Create(user)) - err = user.SetPassword(ts.db.Context(), strings.Repeat("a", crypto.MaxPasswordLength+1)) + err = user.SetPassword(ts.db.Context(), strings.Repeat("a", crypto.MaxPasswordLength+1), false, "", "") require.Error(ts.T(), err) - err = user.SetPassword(ts.db.Context(), strings.Repeat("a", crypto.MaxPasswordLength)) + err = user.SetPassword(ts.db.Context(), strings.Repeat("a", crypto.MaxPasswordLength), false, "", "") require.NoError(ts.T(), err) }