Fix issue where users would get locked out when using OTP tokens.

lib/auth/apiserver.go

@@ -73,6 +73,7 @@ func NewAPIServer(config *APIConfig) http.Handler {
 	// Generating certificates for user and host authorities
 	srv.POST("/:version/ca/host/certs", srv.withAuth(srv.generateHostCert))
 	srv.POST("/:version/ca/user/certs", srv.withAuth(srv.generateUserCert))
+	srv.POST("/:version/ca/user/certs/bundle", srv.withAuth(srv.generateUserCertBundle))
 
 	// Operations on users
 	srv.GET("/:version/users", srv.withAuth(srv.getUsers))
@@ -746,6 +747,46 @@ func (s *APIServer) generateUserCert(auth ClientI, w http.ResponseWriter, r *htt
 	return string(cert), nil
 }
 
+type sshUserCertBundleResponse struct {
+	Username    string                     `json:"username"`
+	Cert        []byte                     `json:"cert"`
+	HostSigners []services.CertAuthorityV1 `json:"host_signers"`
+}
+
+func (s *APIServer) generateUserCertBundle(auth ClientI, w http.ResponseWriter, r *http.Request, _ httprouter.Params, version string) (interface{}, error) {
+	var req *generateUserCertReq
+
+	if err := httplib.ReadJSON(r, &req); err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	// create the user certificate
+	compatibility, err := utils.CheckCompatibilityFlag(req.Compatibility)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	cert, err := auth.GenerateUserCert(req.Key, req.User, req.TTL, compatibility)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	// get the host ca
+	hostSigners, err := auth.GetCertAuthorities(services.HostCA, false)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	signers, err := services.CertAuthoritiesToV1(hostSigners)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	return &sshUserCertBundleResponse{
+		Username:    req.User,
+		Cert:        cert,
+		HostSigners: signers,
+	}, nil
+}
+
 type generateTokenReq struct {
 	Roles teleport.Roles `json:"roles"`
 	TTL   time.Duration  `json:"ttl"`

lib/auth/auth.go

@@ -251,6 +251,12 @@ func (s *AuthServer) WithUserLock(username string, authenticateFn func() error)
 	}
 	fnErr := authenticateFn()
 	if fnErr == nil {
+		// upon successful login, reset the failed attempt counter
+		err = s.DeleteUserLoginAttempts(username)
+		if !trace.IsNotFound(err) {
+			return trace.Wrap(err)
+		}
+
 		return nil
 	}
 	// do not lock user in case if DB is flaky or down

lib/auth/clt.go

@@ -785,6 +785,30 @@ func (c *Client) GenerateUserCert(key []byte, user string, ttl time.Duration, co
 	return []byte(cert), nil
 }
 
+// GenerateUserCertBundle takes the public key in the OpenSSH `authorized_keys`
+// plain text format, signs it using User Certificate Authority signing key and
+// returns the resulting certificate. It also includes the host certificate that
+// can be added to the known_hosts file.
+func (c *Client) GenerateUserCertBundle(key []byte, user string, ttl time.Duration, compatibility string) ([]byte, []services.CertAuthorityV1, error) {
+	out, err := c.PostJSON(c.Endpoint("ca", "user", "certs", "bundle"),
+		generateUserCertReq{
+			Key:           key,
+			User:          user,
+			TTL:           ttl,
+			Compatibility: compatibility,
+		})
+	if err != nil {
+		return nil, nil, trace.Wrap(err)
+	}
+
+	var br sshUserCertBundleResponse
+	if err := json.Unmarshal(out.Bytes(), &br); err != nil {
+		return nil, nil, trace.Wrap(err)
+	}
+
+	return br.Cert, br.HostSigners, nil
+}
+
 // CreateSignupToken creates one time token for creating account for the user
 // For each token it creates username and otp generator
 func (c *Client) CreateSignupToken(user services.UserV1) (string, error) {

lib/auth/tun.go

@@ -85,9 +85,12 @@ type TunClient struct {
 	discoveredAuthServers []utils.NetAddr
 	authMethods           []ssh.AuthMethod
 	refreshTicker         *time.Ticker
-	closeC                chan struct{}
-	closeOnce             sync.Once
-	addrStorage           utils.AddrStorage
+	// disableRefresh will disable the refresh ticker. Used when we only call a
+	// single function with a TunClient (initial fetch of certs).
+	disableRefresh bool
+	closeC         chan struct{}
+	closeOnce      sync.Once
+	addrStorage    utils.AddrStorage
 	// purpose is used for more informative logging. it explains _why_ this
 	// client was created
 	purpose string
@@ -757,6 +760,17 @@ func TunClientStorage(storage utils.AddrStorage) TunClientOption {
 	}
 }
 
+// TunDisableRefresh will disable refreshing the list of auth servers. This is
+// required when requesting user certificates because we only allow a single
+// HTTP request to be made over the tunnel. This is because each request does
+// keyAuth, and for situations like password+otp where the OTP token is invalid
+// after the first use, that means all other requests would fail.
+func TunDisableRefresh() TunClientOption {
+	return func(t *TunClient) {
+		t.disableRefresh = true
+	}
+}
+
 // NewTunClient returns an instance of new HTTP client to Auth server API
 // exposed over SSH tunnel, so client  uses SSH credentials to dial and authenticate
 //  - purpose is mostly for debuggin, like "web client" or "reverse tunnel client"
@@ -889,9 +903,11 @@ func (c *TunClient) Dial(network, address string) (net.Conn, error) {
 	}
 	// dialed & authenticated? lets start synchronizing the
 	// list of auth servers:
-	if c.refreshTicker == nil {
-		c.refreshTicker = time.NewTicker(defaults.AuthServersRefreshPeriod)
-		go c.authServersSyncLoop()
+	if c.disableRefresh == false {
+		if c.refreshTicker == nil {
+			c.refreshTicker = time.NewTicker(defaults.AuthServersRefreshPeriod)
+			go c.authServersSyncLoop()
+		}
 	}
 	return &tunConn{client: client, Conn: conn}, nil
 }

lib/auth/tun_test.go

@@ -542,6 +542,76 @@ func (s *TunSuite) TestSessionsBadPassword(c *C) {
 	c.Assert(ws, IsNil)
 }
 
+// TestLoginAttempts makes sure the login attempt counter is incremented and
+// reset correctly.
+func (s *TunSuite) TestLoginAttempts(c *C) {
+	c.Assert(s.a.UpsertCertAuthority(
+		suite.NewTestCA(services.UserCA, "localhost")), IsNil)
+
+	user := "system-test"
+	pass := []byte("system-abc123")
+	rawSecret := "def456"
+	otpSecret := base32.StdEncoding.EncodeToString([]byte(rawSecret))
+
+	err := s.a.UpsertPassword(user, pass)
+	c.Assert(err, IsNil)
+
+	err = s.a.UpsertTOTP(user, otpSecret)
+	c.Assert(err, IsNil)
+
+	otpURL, _, err := s.a.GetOTPData(user)
+	c.Assert(err, IsNil)
+
+	// make sure label in url is correct
+	u, err := url.Parse(otpURL)
+	c.Assert(err, IsNil)
+	c.Assert(u.Path, Equals, "/system-test")
+
+	// create a valid otp token
+	validToken, err := totp.GenerateCode(otpSecret, time.Now())
+	c.Assert(err, IsNil)
+
+	// try first to login with an invalid password
+	authMethod, err := NewWebPasswordAuth(user, []byte("invalid-password"), validToken)
+	c.Assert(err, IsNil)
+
+	clt, err := NewTunClient("test",
+		[]utils.NetAddr{{AddrNetwork: "tcp", Addr: s.tsrv.Addr()}}, user, authMethod, TunDisableRefresh())
+	c.Assert(err, IsNil)
+	c.Assert(err, IsNil)
+
+	// we can make any request and don't care about the result. the code keyAuth
+	// code we care about is run during the ssh handshake.
+	clt.GetUsers()
+	err = clt.Close()
+	c.Assert(err, IsNil)
+
+	// should only create a single failed login attempt
+	loginAttempts, err := s.a.GetUserLoginAttempts(user)
+	c.Assert(err, IsNil)
+	c.Assert(loginAttempts, HasLen, 1)
+
+	// try again with the correct password
+	authMethod, err = NewWebPasswordAuth(user, pass, validToken)
+	c.Assert(err, IsNil)
+
+	clt, err = NewTunClient("test",
+		[]utils.NetAddr{{AddrNetwork: "tcp", Addr: s.tsrv.Addr()}}, user, authMethod, TunDisableRefresh())
+	c.Assert(err, IsNil)
+	c.Assert(err, IsNil)
+
+	// once again, we can make any request and don't care about the result. the
+	// code keyAuth code we care about is run during the ssh handshake.
+	clt.GetUsers()
+	err = clt.Close()
+	c.Assert(err, IsNil)
+
+	// login was successful, attempts should be reset back to 0
+	loginAttempts, err = s.a.GetUserLoginAttempts(user)
+	c.Assert(err, IsNil)
+	c.Assert(loginAttempts, HasLen, 0)
+}
+
 func (s *TunSuite) TestFailover(c *C) {
 	node := newServer(
 		services.KindNode,

lib/defaults/defaults.go

@@ -144,7 +144,7 @@ const (
 	Namespace = "default"
 
 	// AttemptTTL is TTL for login attempt
-	AttemptTTL = time.Hour * 24 * 7
+	AttemptTTL = time.Minute * 30
 )
 
 var (

lib/services/identity.go

@@ -46,6 +46,10 @@ type Identity interface {
 	// GetUserLoginAttempts returns user login attempts
 	GetUserLoginAttempts(user string) ([]LoginAttempt, error)
 
+	// DeleteUserLoginAttempts removes all login attempts of a user. Should be
+	// called after successful login.
+	DeleteUserLoginAttempts(user string) error
+
 	// CreateUser creates user if it does not exist
 	CreateUser(user User) error
 

lib/services/local/users.go

@@ -351,6 +351,18 @@ func (s *IdentityService) GetUserLoginAttempts(user string) ([]services.LoginAtt
 	return out, nil
 }
 
+// DeleteUserLoginAttempts removes all login attempts of a user. Should be
+// called after successful login.
+func (s *IdentityService) DeleteUserLoginAttempts(user string) error {
+	err := s.DeleteBucket([]string{"web", "users", user}, "attempts")
+	if err != nil {
+		if trace.IsNotFound(err) {
+			return trace.NotFound(fmt.Sprintf("user '%v' is not found", user))
+		}
+	}
+	return trace.Wrap(err)
+}
+
 // GetWebSession returns a web session state for a given user and session id
 func (s *IdentityService) GetWebSession(user, sid string) (services.WebSession, error) {
 	val, err := s.GetVal([]string{"web", "users", user, "sessions"}, sid)

lib/web/sessions.go

@@ -371,12 +371,13 @@ func (s *sessionCache) AuthWithOTP(user, pass string, otpToken string) (services
 	// only using the tunnel for authentication, we close it right afterwards
 	// because we will not be using this connection (initiated using password
 	// based credentials) later.
-	clt, err := auth.NewTunClient("web.client.password-and-otp", s.authServers, user, method)
+	clt, err := auth.NewTunClient("web.client.password-and-otp", s.authServers, user, method, auth.TunDisableRefresh())
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
 	defer clt.Close()
 
+	// create a web session
 	session, err := clt.SignIn(user, []byte(pass))
 	if err != nil {
 		defer clt.Close()
@@ -391,12 +392,13 @@ func (s *sessionCache) AuthWithoutOTP(user, pass string) (services.WebSession, e
 		return nil, trace.Wrap(err)
 	}
 
-	clt, err := auth.NewTunClient("web.client.password-only", s.authServers, user, method)
+	clt, err := auth.NewTunClient("web.client.password-only", s.authServers, user, method, auth.TunDisableRefresh())
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
 	defer clt.Close()
 
+	// create a web session
 	session, err := clt.SignIn(user, []byte(pass))
 	if err != nil {
 		defer clt.Close()
@@ -431,15 +433,18 @@ func (s *sessionCache) AuthWithU2FSignResponse(user string, response *u2f.SignRe
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
-	clt, err := auth.NewTunClient("web.client-u2f", s.authServers, user, method)
+	clt, err := auth.NewTunClient("web.client-u2f", s.authServers, user, method, auth.TunDisableRefresh())
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
 	defer clt.Close()
+
+	// create a web session
 	session, err := clt.PreAuthenticatedSignIn(user)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+
 	return session, nil
 }
 
@@ -449,7 +454,7 @@ func (s *sessionCache) GetCertificateWithoutOTP(c client.CreateSSHCertReq) (*cli
 		return nil, trace.Wrap(err)
 	}
 
-	clt, err := auth.NewTunClient("web.session.password-only", s.authServers, c.User, method)
+	clt, err := auth.NewTunClient("web.session.password-only", s.authServers, c.User, method, auth.TunDisableRefresh())
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
@@ -464,7 +469,7 @@ func (s *sessionCache) GetCertificateWithOTP(c client.CreateSSHCertReq) (*client
 		return nil, trace.Wrap(err)
 	}
 
-	clt, err := auth.NewTunClient("web.session.password+otp", s.authServers, c.User, method)
+	clt, err := auth.NewTunClient("web.session.password+otp", s.authServers, c.User, method, auth.TunDisableRefresh())
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
@@ -474,22 +479,14 @@ func (s *sessionCache) GetCertificateWithOTP(c client.CreateSSHCertReq) (*client
 }
 
 func createCertificate(user string, pubkey []byte, ttl time.Duration, compatibility string, clt *auth.TunClient) (*client.SSHLoginResponse, error) {
-	cert, err := clt.GenerateUserCert(pubkey, user, ttl, compatibility)
-	if err != nil {
-		return nil, trace.Wrap(err)
-	}
-	hostSigners, err := clt.GetCertAuthorities(services.HostCA, false)
-	if err != nil {
-		return nil, trace.Wrap(err)
-	}
-	signers, err := services.CertAuthoritiesToV1(hostSigners)
+	userCert, hostCA, err := clt.GenerateUserCertBundle(pubkey, user, ttl, compatibility)
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
 
 	return &client.SSHLoginResponse{
-		Cert:        cert,
-		HostSigners: signers,
+		Cert:        userCert,
+		HostSigners: hostCA,
 	}, nil
 }
 
@@ -499,7 +496,7 @@ func (s *sessionCache) GetCertificateWithU2F(c client.CreateSSHCertWithU2FReq) (
 		return nil, trace.Wrap(err)
 	}
 
-	clt, err := auth.NewTunClient("web.session-u2f", s.authServers, c.User, method)
+	clt, err := auth.NewTunClient("web.session-u2f", s.authServers, c.User, method, auth.TunDisableRefresh())
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}