Merge pull request #2006 from gravitational/rjones/ttl-increase

Allow user certificates with TTL larger than 30 hours

lib/auth/auth.go

@@ -276,6 +276,10 @@ type certRequest struct {
 	publicKey []byte
 	// compatibility is compatibility mode
 	compatibility string
+	// overrideRoleTTL is used for requests when the requested TTL should not be
+	// adjusted based off the role of the user. This is used by tctl to allow
+	// creating long lived user certs.
+	overrideRoleTTL bool
 }
 
 // GenerateUserCerts is used to generate user certificate, used internally for tests
@@ -319,14 +323,33 @@ func (s *AuthServer) generateUserCert(req certRequest) (*certs, error) {
 		certificateFormat = req.roles.CertificateFormat()
 	}
 
-	// adjust session ttl to the smaller of two values: the session
-	// ttl requested in tsh or the session ttl for the role.
-	sessionTTL := req.roles.AdjustSessionTTL(req.ttl)
+	var sessionTTL time.Duration
+	var allowedLogins []string
 
-	// check signing TTL and return a list of allowed logins
-	allowedLogins, err := req.roles.CheckLoginDuration(sessionTTL)
-	if err != nil {
-		return nil, trace.Wrap(err)
+	// If the role TTL is ignored, do not restrict session TTL and allowed logins.
+	// The only caller setting this parameter should be "tctl auth sign".
+	// Otherwise set the session TTL to the smallest of all roles and
+	// then only grant access to allowed logins based on that.
+	if req.overrideRoleTTL {
+		// Take whatever was passed in. Pass in 0 to CheckLoginDuration so all
+		// logins are returned for the role set.
+		sessionTTL = req.ttl
+		allowedLogins, err = req.roles.CheckLoginDuration(0)
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
+	} else {
+		// Adjust session TTL to the smaller of two values: the session TTL
+		// requested in tsh or the session TTL for the role.
+		sessionTTL = req.roles.AdjustSessionTTL(req.ttl)
+
+		// Return a list of logins that meet the session TTL limit. This means if
+		// the requested session TTL is larger than the max session TTL for a login,
+		// that login will not be included in the list of allowed logins.
+		allowedLogins, err = req.roles.CheckLoginDuration(sessionTTL)
+		if err != nil {
+			return nil, trace.Wrap(err)
+		}
 	}
 
 	clusterName, err := s.GetDomainName()

lib/auth/auth_with_roles.go

@@ -72,13 +72,26 @@ func (a *AuthWithRoles) authConnectorAction(namespace string, resource string, v
 	return nil
 }
 
+// hasBuiltinRole checks the type of the role set returned and the name.
+// Returns true if role set is builtin and the name matches.
+func (a *AuthWithRoles) hasBuiltinRole(name string) bool {
+	if _, ok := a.checker.(BuiltinRoleSet); !ok {
+		return false
+	}
+	if !a.checker.HasRole(name) {
+		return false
+	}
+
+	return true
+}
+
 // AuthenticateWebUser authenticates web user, creates and  returns web session
 // in case if authentication is successfull
 func (a *AuthWithRoles) AuthenticateWebUser(req AuthenticateUserRequest) (services.WebSession, error) {
 	// authentication request has it's own authentication, however this limits the requests
 	// types to proxies to make it harder to break
-	if !a.checker.HasRole(string(teleport.RoleProxy)) {
-		return nil, trace.AccessDenied("this request can be only executed by proxy")
+	if !a.hasBuiltinRole(string(teleport.RoleProxy)) {
+		return nil, trace.AccessDenied("this request can be only executed by a proxy")
 	}
 	return a.authServer.AuthenticateWebUser(req)
 }
@@ -88,8 +101,8 @@ func (a *AuthWithRoles) AuthenticateWebUser(req AuthenticateUserRequest) (servic
 func (a *AuthWithRoles) AuthenticateSSHUser(req AuthenticateSSHRequest) (*SSHLoginResponse, error) {
 	// authentication request has it's own authentication, however this limits the requests
 	// types to proxies to make it harder to break
-	if !a.checker.HasRole(string(teleport.RoleProxy)) {
-		return nil, trace.AccessDenied("this request can be only executed by proxy")
+	if !a.hasBuiltinRole(string(teleport.RoleProxy)) {
+		return nil, trace.AccessDenied("this request can be only executed by a proxy")
 	}
 	return a.authServer.AuthenticateSSHUser(req)
 }
@@ -530,37 +543,35 @@ func (a *AuthWithRoles) GenerateHostCert(
 }
 
 func (a *AuthWithRoles) GenerateUserCert(key []byte, username string, ttl time.Duration, compatibility string) ([]byte, error) {
-	if err := a.currentUserAction(username); err != nil {
-		return nil, trace.AccessDenied("%v cannot request a certificate for %v", a.user.GetName(), username)
-	}
-	// notice that user requesting the certificate and the user currently
-	// authenticated may differ (e.g. admin generates certificate for the user scenario)
-	// so we fetch user's permissions
-	checker := a.checker
-	var user services.User
-	var err error
-	if a.user.GetName() != username {
-		user, err = a.GetUser(username)
-		if err != nil {
-			return nil, trace.Wrap(err)
-		}
-		checker, err = services.FetchRoles(user.GetRoles(), a.authServer, user.GetTraits())
-		if err != nil {
-			return nil, trace.Wrap(err)
-		}
-	} else {
-		user = a.user
+	// This endpoint is only accessible to tctl.
+	if !a.hasBuiltinRole(string(teleport.RoleAdmin)) {
+		return nil, trace.AccessDenied("this request can be only executed by an admin")
 	}
+
+	// Extract the user and role set for whom the certificate will be generated.
+	user, err := a.GetUser(username)
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+	checker, err := services.FetchRoles(user.GetRoles(), a.authServer, user.GetTraits())
+	if err != nil {
+		return nil, trace.Wrap(err)
+	}
+
+	// Generate certificate, note that the roles TTL will be ignored because
+	// the request is coming from "tctl auth sign" itself.
 	certs, err := a.authServer.generateUserCert(certRequest{
-		user:          user,
-		roles:         checker,
-		ttl:           ttl,
-		compatibility: compatibility,
-		publicKey:     key,
+		user:            user,
+		roles:           checker,
+		ttl:             ttl,
+		compatibility:   compatibility,
+		publicKey:       key,
+		overrideRoleTTL: true,
 	})
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+
 	return certs.ssh, nil
 }
 
@@ -1193,8 +1204,8 @@ func (a *AuthWithRoles) DeleteAllRemoteClusters() error {
 // signed certificate if sucessfull.
 func (a *AuthWithRoles) ProcessKubeCSR(req KubeCSR) (*KubeCSRResponse, error) {
 	// limits the requests types to proxies to make it harder to break
-	if !a.checker.HasRole(string(teleport.RoleProxy)) {
-		return nil, trace.AccessDenied("this request can be only executed by proxy")
+	if !a.hasBuiltinRole(string(teleport.RoleProxy)) {
+		return nil, trace.AccessDenied("this request can be only executed by a proxy")
 	}
 	return a.authServer.ProcessKubeCSR(req)
 }

lib/auth/permissions.go

@@ -143,12 +143,13 @@ func (a *authorizer) authorizeRemoteUser(u RemoteUser) (*AuthContext, error) {
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
+	// The user is prefixed with "remote-" and suffixed with cluster name with
+	// the hope that it does not match a real local user.
 	user, err := services.NewUser(fmt.Sprintf("remote-%v-%v", u.Username, u.ClusterName))
 	if err != nil {
 		return nil, trace.Wrap(err)
 	}
 	return &AuthContext{
-		// this is done on purpose to make sure user does not match some real local user
 		User:    user,
 		Checker: checker,
 	}, nil
@@ -211,7 +212,7 @@ func (a *authorizer) authorizeRemoteBuiltinRole(r RemoteBuiltinRole) (*AuthConte
 }
 
 // GetCheckerForBuiltinRole returns checkers for embedded builtin role
-func GetCheckerForBuiltinRole(clusterName string, clusterConfig services.ClusterConfig, role teleport.Role) (services.AccessChecker, error) {
+func GetCheckerForBuiltinRole(clusterName string, clusterConfig services.ClusterConfig, role teleport.Role) (services.RoleSet, error) {
 	switch role {
 	case teleport.RoleAuth:
 		return services.FromSpec(
@@ -423,7 +424,7 @@ func contextForBuiltinRole(clusterName string, clusterConfig services.ClusterCon
 	user.SetRoles([]string{string(r)})
 	return &AuthContext{
 		User:    user,
-		Checker: checker,
+		Checker: BuiltinRoleSet{checker},
 	}, nil
 }
 
@@ -466,6 +467,12 @@ type BuiltinRole struct {
 	ClusterName string
 }
 
+// BuiltinRoleSet wraps a services.RoleSet. The type is used to determine if
+// the role is builtin or not.
+type BuiltinRoleSet struct {
+	services.RoleSet
+}
+
 // RemoteBuiltinRole is the role of the remote (service connecting via trusted cluster link)
 // Teleport service.
 type RemoteBuiltinRole struct {

lib/auth/tls_test.go

@@ -1029,7 +1029,7 @@ func (s *TLSSuite) TestGenerateCerts(c *check.C) {
 	_, err = nopClient.GenerateUserCert(pub, user1.GetName(), time.Hour, teleport.CertificateFormatStandard)
 	c.Assert(err, check.NotNil)
 	fixtures.ExpectAccessDenied(c, err)
-	c.Assert(err, check.ErrorMatches, ".*cannot request a certificate for user1")
+	c.Assert(err, check.ErrorMatches, "this request can be only executed by an admin")
 
 	// Users don't match
 	userClient2, err := s.server.NewClient(TestUser(user2.GetName()))
@@ -1038,21 +1038,21 @@ func (s *TLSSuite) TestGenerateCerts(c *check.C) {
 	_, err = userClient2.GenerateUserCert(pub, user1.GetName(), time.Hour, teleport.CertificateFormatStandard)
 	c.Assert(err, check.NotNil)
 	fixtures.ExpectAccessDenied(c, err)
-	c.Assert(err, check.ErrorMatches, ".*cannot request a certificate for user1")
+	c.Assert(err, check.ErrorMatches, "this request can be only executed by an admin")
 
-	// should not be able to generate cert for longer than duration
-	userClient1, err := s.server.NewClient(TestUser(user1.GetName()))
+	// Admin should be allowed to generate certs with TTL longer than max.
+	adminClient, err := s.server.NewClient(TestAdmin())
 	c.Assert(err, check.IsNil)
 
-	cert, err := userClient1.GenerateUserCert(pub, user1.GetName(), 40*time.Hour, teleport.CertificateFormatStandard)
+	cert, err := adminClient.GenerateUserCert(pub, user1.GetName(), 40*time.Hour, teleport.CertificateFormatStandard)
 	c.Assert(err, check.IsNil)
 
 	parsedKey, _, _, _, err := ssh.ParseAuthorizedKey(cert)
 	c.Assert(err, check.IsNil)
 	parsedCert, _ := parsedKey.(*ssh.Certificate)
 	validBefore := time.Unix(int64(parsedCert.ValidBefore), 0)
 	diff := validBefore.Sub(time.Now())
-	c.Assert(diff < defaults.MaxCertDuration, check.Equals, true, check.Commentf("expected %v < %v", diff, defaults.CertDuration))
+	c.Assert(diff > defaults.MaxCertDuration, check.Equals, true, check.Commentf("expected %v > %v", diff, defaults.CertDuration))
 
 	// user should have agent forwarding (default setting)
 	_, exists := parsedCert.Extensions[teleport.CertExtensionPermitAgentForwarding]
@@ -1065,7 +1065,7 @@ func (s *TLSSuite) TestGenerateCerts(c *check.C) {
 	err = s.server.Auth().UpsertRole(userRole, backend.Forever)
 	c.Assert(err, check.IsNil)
 
-	cert, err = userClient1.GenerateUserCert(pub, user1.GetName(), 1*time.Hour, teleport.CertificateFormatStandard)
+	cert, err = adminClient.GenerateUserCert(pub, user1.GetName(), 1*time.Hour, teleport.CertificateFormatStandard)
 	c.Assert(err, check.IsNil)
 	parsedKey, _, _, _, err = ssh.ParseAuthorizedKey(cert)
 	c.Assert(err, check.IsNil)
@@ -1076,7 +1076,7 @@ func (s *TLSSuite) TestGenerateCerts(c *check.C) {
 	c.Assert(exists, check.Equals, true)
 
 	// apply HTTP Auth to generate user cert:
-	cert, err = userClient1.GenerateUserCert(pub, user1.GetName(), time.Hour, teleport.CertificateFormatStandard)
+	cert, err = adminClient.GenerateUserCert(pub, user1.GetName(), time.Hour, teleport.CertificateFormatStandard)
 	c.Assert(err, check.IsNil)
 
 	_, _, _, _, err = ssh.ParseAuthorizedKey(cert)

lib/client/api.go

@@ -555,8 +555,6 @@ func NewClient(c *Config) (tc *TeleportClient, err error) {
 	}
 	if c.KeyTTL == 0 {
 		c.KeyTTL = defaults.CertDuration
-	} else if c.KeyTTL > defaults.MaxCertDuration || c.KeyTTL < defaults.MinCertDuration {
-		return nil, trace.BadParameter("invalid requested cert TTL")
 	}
 	c.Namespace = services.ProcessNamespace(c.Namespace)
 

lib/services/role.go

@@ -119,7 +119,7 @@ func NewImplicitRole() Role {
 		},
 		Spec: RoleSpecV3{
 			Options: RoleOptions{
-				MaxSessionTTL: NewDuration(defaults.MaxCertDuration),
+				MaxSessionTTL: MaxDuration(),
 			},
 			Allow: RoleConditions{
 				Namespaces: []string{defaults.Namespace},