From 63d2250494de58c19eecaa7fe5337e54cb25ab4e Mon Sep 17 00:00:00 2001 From: Alan Parra Date: Wed, 6 Oct 2021 13:18:21 -0300 Subject: [PATCH] Remove pre-v7 device migration logic (#8448) Follows up on a few `// DELETE IN: 7` TODOs. * Remove pre-v7 device migration logic * Fix godoc typos --- lib/auth/init.go | 79 ++--------------------- lib/auth/init_test.go | 111 +-------------------------------- lib/services/local/resource.go | 53 +--------------- lib/services/local/users.go | 14 ++--- 4 files changed, 10 insertions(+), 247 deletions(-) diff --git a/lib/auth/init.go b/lib/auth/init.go index 753bf010ae0..e43621ef0af 100644 --- a/lib/auth/init.go +++ b/lib/auth/init.go @@ -18,7 +18,6 @@ package auth import ( "context" - "crypto/ecdsa" "crypto/tls" "crypto/x509" "fmt" @@ -34,7 +33,6 @@ import ( apisshutils "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/teleport/lib" "github.com/gravitational/teleport/lib/auth/keystore" - "github.com/gravitational/teleport/lib/auth/u2f" "github.com/gravitational/teleport/lib/backend" "github.com/gravitational/teleport/lib/events" "github.com/gravitational/teleport/lib/modules" @@ -390,7 +388,7 @@ func Init(cfg InitConfig, opts ...ServerOption) (*Server, error) { } // Migrate any legacy resources to new format. - err = migrateLegacyResources(ctx, cfg, asrv) + err = migrateLegacyResources(ctx, asrv) if err != nil { return nil, trace.Wrap(err) } @@ -496,7 +494,7 @@ func shouldInitReplaceResourceWithOrigin(stored, candidate types.ResourceWithOri return false, nil } -func migrateLegacyResources(ctx context.Context, cfg InitConfig, asrv *Server) error { +func migrateLegacyResources(ctx context.Context, asrv *Server) error { err := migrateOSS(ctx, asrv) if err != nil { return trace.Wrap(err) @@ -512,14 +510,9 @@ func migrateLegacyResources(ctx context.Context, cfg InitConfig, asrv *Server) e return trace.Wrap(err) } - if err := migrateMFADevices(ctx, asrv); err != nil { - return trace.Wrap(err) - } - if err := migrateCertAuthorities(ctx, asrv); err != nil { return trace.Wrap(err, "fail to migrate certificate authorities to the v7 storage format: %v; please report this at https://github.com/gravitational/teleport/issues/new?assignees=&labels=bug&template=bug_report.md including the *redacted* output of 'tctl get cert_authority'", err) } - return nil } @@ -883,7 +876,8 @@ func CertAuthorityInfo(ca types.CertAuthority) string { return fmt.Sprintf("cert authority(state: %v, phase: %v, roots: %v)", ca.GetRotation().State, ca.GetRotation().Phase, strings.Join(out, ", ")) } -// HasTSLConfig returns true if this identity has TLS certificate and private key +// HasTLSConfig returns true if this identity has TLS certificate and private +// key. func (i *Identity) HasTLSConfig() bool { return len(i.TLSCACertsBytes) != 0 && len(i.TLSCertBytes) != 0 } @@ -1226,71 +1220,6 @@ func migrateRoleOptions(ctx context.Context, asrv *Server) error { return nil } -// DELETE IN: 7.0.0 -// migrateMFADevices migrates registered MFA devices to the new storage format. -func migrateMFADevices(ctx context.Context, asrv *Server) error { - users, err := asrv.GetUsers(true) - if err != nil { - return trace.Wrap(err) - } - - for _, user := range users { - la := user.GetLocalAuth() - if la == nil { - continue - } - if len(la.MFA) > 0 { - // User already migrated. - continue - } - - if len(la.TOTPKey) > 0 { - d, err := services.NewTOTPDevice("totp", la.TOTPKey, asrv.clock.Now()) - if err != nil { - return trace.Wrap(err) - } - la.MFA = append(la.MFA, d) - - la.TOTPKey = "" - } - if la.U2FRegistration != nil { - pubKeyI, err := x509.ParsePKIXPublicKey(la.U2FRegistration.PubKey) - if err != nil { - return trace.Wrap(err) - } - pubKey, ok := pubKeyI.(*ecdsa.PublicKey) - if !ok { - return trace.BadParameter("expected *ecdsa.PublicKey, got %T", pubKeyI) - } - d, err := u2f.NewDevice("u2f", &u2f.Registration{ - KeyHandle: la.U2FRegistration.KeyHandle, - PubKey: *pubKey, - }, asrv.clock.Now()) - if err != nil { - return trace.Wrap(err) - } - d.GetU2F().Counter = la.U2FCounter - la.MFA = append(la.MFA, d) - - la.U2FRegistration = nil - la.U2FCounter = 0 - } - - if len(la.MFA) == 0 { - // No MFA devices to migrate. - continue - } - - log.Debugf("Migrating MFA devices in LocalAuth for user %q", user.GetName()) - user.SetLocalAuth(la) - if err := asrv.UpsertUser(user); err != nil { - return trace.Wrap(err) - } - } - - return nil -} - // DELETE IN: 8.0.0 // migrateCertAuthorities migrates the keypair storage format in cert // authorities to the new format. diff --git a/lib/auth/init_test.go b/lib/auth/init_test.go index bb77802d1d4..93558b3d655 100644 --- a/lib/auth/init_test.go +++ b/lib/auth/init_test.go @@ -18,11 +18,6 @@ package auth import ( "context" - "crypto/ecdsa" - "crypto/elliptic" - "crypto/rand" - "crypto/x509" - "encoding/base64" "fmt" "strings" "testing" @@ -33,7 +28,6 @@ import ( "github.com/gravitational/teleport/api/types" apisshutils "github.com/gravitational/teleport/api/utils/sshutils" "github.com/gravitational/teleport/lib/auth/testauthority" - "github.com/gravitational/teleport/lib/auth/u2f" "github.com/gravitational/teleport/lib/backend" "github.com/gravitational/teleport/lib/backend/lite" "github.com/gravitational/teleport/lib/defaults" @@ -153,7 +147,7 @@ func TestBadIdentity(t *testing.T) { HostID: "example.com", NodeName: "", ClusterName: "id1", - Role: types.SystemRole("bad role"), + Role: "bad role", TTL: 0, }) require.NoError(t, err) @@ -496,102 +490,6 @@ func TestCASigningAlg(t *testing.T) { verifyCAs(auth, ssh.SigAlgoRSA) } -func TestMigrateMFADevices(t *testing.T) { - ctx := context.Background() - as := newTestAuthServer(ctx, t) - clock := clockwork.NewFakeClock() - as.SetClock(clock) - - // Fake credentials and MFA secrets for migration. - fakePasswordHash := []byte(`$2a$10$Yy.e6BmS2SrGbBDsyDLVkOANZmvjjMR890nUGSXFJHBXWzxe7T44m`) - totpKey := "totp-key" - u2fPrivKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) - require.NoError(t, err) - u2fPubKey := u2fPrivKey.PublicKey - u2fPubKeyBin, err := x509.MarshalPKIXPublicKey(&u2fPubKey) - require.NoError(t, err) - u2fKeyHandle := []byte("dummy handle") - - // Create un-migrated users. - for name, localAuth := range map[string]*backend.Item{ - "no-mfa-user": nil, - // Insert MFA data in the legacy format by manually writing to the - // backend. All the code for writing these in lib/services/local was - // removed. - "totp-user": { - Key: []byte("/web/users/totp-user/totp"), - Value: []byte(totpKey), - }, - "u2f-user": { - Key: []byte("/web/users/u2f-user/u2fregistration"), - Value: []byte(fmt.Sprintf(`{"keyhandle":%q,"marshalled_pubkey":%q}`, - base64.StdEncoding.EncodeToString(u2fKeyHandle), - base64.StdEncoding.EncodeToString(u2fPubKeyBin), - )), - }, - } { - u, err := types.NewUser(name) - require.NoError(t, err) - // Set a fake but valid bcrypt password hash. - u.SetLocalAuth(&types.LocalAuthSecrets{PasswordHash: fakePasswordHash}) - err = as.CreateUser(ctx, u) - require.NoError(t, err) - - if localAuth != nil { - _, err = as.bk.Put(ctx, *localAuth) - require.NoError(t, err) - } - } - - // Run the migration. - err = migrateMFADevices(ctx, as) - require.NoError(t, err) - - // Generate expected users with migrated MFA. - requireNewDevice := func(d *types.MFADevice, err error) []*types.MFADevice { - require.NoError(t, err) - return []*types.MFADevice{d} - } - wantUsers := []types.User{ - newUserWithAuth(t, "no-mfa-user", &types.LocalAuthSecrets{PasswordHash: fakePasswordHash}), - newUserWithAuth(t, "totp-user", &types.LocalAuthSecrets{ - PasswordHash: fakePasswordHash, - TOTPKey: totpKey, - MFA: requireNewDevice(services.NewTOTPDevice("totp", totpKey, clock.Now())), - }), - newUserWithAuth(t, "u2f-user", &types.LocalAuthSecrets{ - PasswordHash: fakePasswordHash, - U2FRegistration: &types.U2FRegistrationData{ - KeyHandle: u2fKeyHandle, - PubKey: u2fPubKeyBin, - }, - MFA: requireNewDevice(u2f.NewDevice("u2f", &u2f.Registration{ - KeyHandle: u2fKeyHandle, - PubKey: u2fPubKey, - }, clock.Now())), - }), - } - cmpOpts := []cmp.Option{ - cmpopts.IgnoreFields(types.UserSpecV2{}, "CreatedBy"), - cmpopts.IgnoreFields(types.MFADevice{}, "Id"), - cmpopts.IgnoreFields(types.Metadata{}, "ID"), - cmpopts.SortSlices(func(a, b types.User) bool { return a.GetName() < b.GetName() }), - } - - // Check the actual users from the backend. - users, err := as.GetUsers(true) - require.NoError(t, err) - require.Empty(t, cmp.Diff(users, wantUsers, cmpOpts...)) - - // A second migration should be a noop. - err = migrateMFADevices(ctx, as) - require.NoError(t, err) - - users, err = as.GetUsers(true) - require.NoError(t, err) - require.Empty(t, cmp.Diff(users, wantUsers, cmpOpts...)) -} - // TestPresets tests behavior of presets func TestPresets(t *testing.T) { ctx := context.Background() @@ -858,13 +756,6 @@ func setupConfig(t *testing.T) InitConfig { } } -func newUserWithAuth(t *testing.T, name string, auth *types.LocalAuthSecrets) types.User { - u, err := types.NewUser(name) - require.NoError(t, err) - u.SetLocalAuth(auth) - return u -} - func newU2FAuthPreferenceFromConfigFile(t *testing.T) types.AuthPreference { ap, err := types.NewAuthPreferenceFromConfigFile(types.AuthPreferenceSpecV2{ Type: constants.Local, diff --git a/lib/services/local/resource.go b/lib/services/local/resource.go index 47ba32071a1..fd5815aca85 100644 --- a/lib/services/local/resource.go +++ b/lib/services/local/resource.go @@ -454,39 +454,6 @@ func itemToLocalAuthSecrets(items userItems) (*types.LocalAuthSecrets, error) { } auth.Webauthn = wal } - - // DELETE IN 7.0: these items are migrated to items.mfa on 6.0 first - // startup. - // - // Delete starts here... - if items.totp != nil { - auth.TOTPKey = string(items.totp.Value) - } - if items.u2fRegistration != nil { - var raw struct { - Raw []byte `json:"raw"` - KeyHandle []byte `json:"keyhandle"` - MarshalledPubKey []byte `json:"marshalled_pubkey"` - } - if err := json.Unmarshal(items.u2fRegistration.Value, &raw); err != nil { - return nil, trace.Wrap(err) - } - auth.U2FRegistration = &types.U2FRegistrationData{ - Raw: raw.Raw, - KeyHandle: raw.KeyHandle, - PubKey: raw.MarshalledPubKey, - } - } - if items.u2fCounter != nil { - var raw struct { - Counter uint32 `json:"counter"` - } - if err := json.Unmarshal(items.u2fCounter.Value, &raw); err != nil { - return nil, trace.Wrap(err) - } - auth.U2FCounter = raw.Counter - } - // ... delete ends here. if err := services.ValidateLocalAuthSecrets(&auth); err != nil { return nil, trace.Wrap(err) } @@ -522,7 +489,7 @@ func itemsFromLocalAuthSecrets(user string, auth types.LocalAuthSecrets) ([]back // has order N cost. // fullUsersPrefix is the entire string preceding the name of a user in a key -var fullUsersPrefix string = string(backend.Key(webPrefix, usersPrefix)) + "/" +var fullUsersPrefix = string(backend.Key(webPrefix, usersPrefix)) + "/" // splitUsernameAndSuffix is a helper for extracting usernames and suffixes from // backend key values. @@ -580,11 +547,6 @@ type userItems struct { pwd *backend.Item mfa []*backend.Item webauthnLocalAuth *backend.Item - - // Deprecated fields, only used for migration on auth server startup. - totp *backend.Item - u2fRegistration *backend.Item - u2fCounter *backend.Item } // Set attempts to set a field by suffix. @@ -596,19 +558,6 @@ func (u *userItems) Set(suffix string, item backend.Item) (ok bool) { u.pwd = &item case webauthnLocalAuthPrefix: u.webauthnLocalAuth = &item - - // DELETE IN 7.0: these items are migrated to mfaDevicePrefix on 6.0 first - // startup. - // - // Delete starts here... - case totpPrefix: - u.totp = &item - case u2fRegistrationPrefix: - u.u2fRegistration = &item - case u2fRegistrationCounterPrefix: - u.u2fCounter = &item - // ... delete ends here. - default: if strings.HasPrefix(suffix, mfaDevicePrefix) { u.mfa = append(u.mfa, &item) diff --git a/lib/services/local/users.go b/lib/services/local/users.go index 51524386fbc..07d95731374 100644 --- a/lib/services/local/users.go +++ b/lib/services/local/users.go @@ -286,8 +286,8 @@ func (s *IdentityService) GetUserByOIDCIdentity(id types.ExternalIdentity) (type return nil, trace.NotFound("user with identity %q not found", &id) } -// GetUserBySAMLCIdentity returns a user by it's specified OIDC Identity, returns first -// user specified with this identity +// GetUserBySAMLIdentity returns a user by it's specified OIDC Identity, returns +// first user specified with this identity. func (s *IdentityService) GetUserBySAMLIdentity(id types.ExternalIdentity) (types.User, error) { users, err := s.GetUsers(false) if err != nil { @@ -1124,7 +1124,7 @@ func (s *IdentityService) GetGithubConnectors(ctx context.Context, withSecrets b return connectors, nil } -// GetGithubConnectot returns a particular Github connector +// GetGithubConnector returns a particular Github connector. func (s *IdentityService) GetGithubConnector(ctx context.Context, name string, withSecrets bool) (types.GithubConnector, error) { if name == "" { return nil, trace.BadParameter("missing parameter name") @@ -1266,7 +1266,7 @@ func (s *IdentityService) CreateUserRecoveryAttempt(ctx context.Context, user st return trace.Wrap(err) } -// GetUserRecoveryAttempt returns users recovery attempts. +// GetUserRecoveryAttempts returns users recovery attempts. func (s *IdentityService) GetUserRecoveryAttempts(ctx context.Context, user string) ([]*types.RecoveryAttempt, error) { if user == "" { return nil, trace.BadParameter("missing parameter user") @@ -1339,10 +1339,4 @@ const ( webauthnSessionData = "webauthnsessiondata" recoveryCodesPrefix = "recoverycodes" recoveryAttemptsPrefix = "recoveryattempts" - - // DELETE IN 7.0: these prefixes are migrated to mfaDevicePrefix in 6.0 on - // first startup. - totpPrefix = "totp" - u2fRegistrationPrefix = "u2fregistration" - u2fRegistrationCounterPrefix = "u2fregistrationcounter" )