From 47d1441cf7d7727d4b1d650c811071895fa8d1b5 Mon Sep 17 00:00:00 2001 From: Edoardo Spadolini Date: Wed, 26 Jan 2022 15:23:57 +0100 Subject: [PATCH] [v7] backport #9501 (access requests in TLS certs) (#9923) * Add access requests to tlsca.Identity, and store them in TLS certs This mirrors what we already do for SSH certs. * Keep track of access requests in web sessions * Keep track of access requests in app sessions * Include the current access requests when issuing new user certs This is necessary because we extend the list of current roles instead of starting from the statically assigned ones, so we should also keep track of all the potential ways that those roles were granted to the user. * fix: pass access requests through PreAuthenticatedSignIn * Tests for access requests in TLS certs --- api/types/session.go | 2 ++ lib/auth/auth.go | 41 ++++++++++++++---------- lib/auth/auth_with_roles.go | 4 +++ lib/auth/sessions.go | 11 ++++--- lib/auth/tls_test.go | 62 +++++++++++++++++++++++++++++++++++++ lib/tlsca/ca.go | 19 ++++++++++++ 6 files changed, 117 insertions(+), 22 deletions(-) diff --git a/api/types/session.go b/api/types/session.go index 9e7fd884fdb7f..c3b2074555bd2 100644 --- a/api/types/session.go +++ b/api/types/session.go @@ -510,6 +510,8 @@ type NewWebSessionRequest struct { SessionTTL time.Duration // LoginTime is the time that this user recently logged in. LoginTime time.Time + // AccessRequests contains the UUIDs of the access requests currently in use. + AccessRequests []string } // Check validates the request. diff --git a/lib/auth/auth.go b/lib/auth/auth.go index fdbf86a0cfab5..ae3ccc77240a3 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -967,11 +967,12 @@ func (a *Server) generateUserCert(req certRequest) (*certs, error) { Username: req.dbUser, Database: req.dbName, }, - DatabaseNames: dbNames, - DatabaseUsers: dbUsers, - MFAVerified: req.mfaVerified, - ClientIP: req.clientIP, - AWSRoleARNs: roleARNs, + DatabaseNames: dbNames, + DatabaseUsers: dbUsers, + MFAVerified: req.mfaVerified, + ClientIP: req.clientIP, + AWSRoleARNs: roleARNs, + ActiveRequests: req.activeRequests.AccessRequests, } subject, err := identity.Subject() if err != nil { @@ -1064,9 +1065,10 @@ func (a *Server) PreAuthenticatedSignIn(user string, identity tlsca.Identity) (t return nil, trace.Wrap(err) } sess, err := a.NewWebSession(types.NewWebSessionRequest{ - User: user, - Roles: roles, - Traits: traits, + User: user, + Roles: roles, + Traits: traits, + AccessRequests: identity.ActiveRequests, }) if err != nil { return nil, trace.Wrap(err) @@ -1171,6 +1173,7 @@ func (a *Server) ExtendWebSession(req WebSessionReq, identity tlsca.Identity) (t return nil, trace.Wrap(err) } + accessRequests := identity.ActiveRequests if req.AccessRequestID != "" { newRoles, requestExpiry, err := a.getRolesAndExpiryFromAccessRequest(req.User, req.AccessRequestID) if err != nil { @@ -1179,6 +1182,7 @@ func (a *Server) ExtendWebSession(req WebSessionReq, identity tlsca.Identity) (t roles = append(roles, newRoles...) roles = apiutils.Deduplicate(roles) + accessRequests = apiutils.Deduplicate(append(accessRequests, req.AccessRequestID)) // Let session expire with the shortest expiry time. if expiresAt.After(requestExpiry) { @@ -1208,14 +1212,16 @@ func (a *Server) ExtendWebSession(req WebSessionReq, identity tlsca.Identity) (t // Set default roles and expiration. expiresAt = prevSession.GetLoginTime().UTC().Add(sessionTTL) roles = user.GetRoles() + accessRequests = nil } sessionTTL := utils.ToTTL(a.clock, expiresAt) sess, err := a.NewWebSession(types.NewWebSessionRequest{ - User: req.User, - Roles: roles, - Traits: traits, - SessionTTL: sessionTTL, + User: req.User, + Roles: roles, + Traits: traits, + SessionTTL: sessionTTL, + AccessRequests: accessRequests, }) if err != nil { return nil, trace.Wrap(err) @@ -1848,11 +1854,12 @@ func (a *Server) NewWebSession(req types.NewWebSessionRequest) (types.WebSession sessionTTL = checker.AdjustSessionTTL(apidefaults.CertDuration) } certs, err := a.generateUserCert(certRequest{ - user: user, - ttl: sessionTTL, - publicKey: pub, - checker: checker, - traits: req.Traits, + user: user, + ttl: sessionTTL, + publicKey: pub, + checker: checker, + traits: req.Traits, + activeRequests: services.RequestIDs{AccessRequests: req.AccessRequests}, }) if err != nil { return nil, trace.Wrap(err) diff --git a/lib/auth/auth_with_roles.go b/lib/auth/auth_with_roles.go index 38a2511fb1dc3..c45dd4e9d66f5 100644 --- a/lib/auth/auth_with_roles.go +++ b/lib/auth/auth_with_roles.go @@ -1438,6 +1438,10 @@ func (a *ServerWithRoles) generateUserCerts(ctx context.Context, req proto.UserC // If the user is generating a certificate, the roles and traits come from the logged in identity. if req.Username == a.context.User.GetName() { roles, traits, err = services.ExtractFromIdentity(a.authServer, a.context.Identity.GetIdentity()) + // we're going to extend the roles list based on the access requests, so + // we ensure that all the current requests are added to the new + // certificate (and are checked again) + req.AccessRequests = append(req.AccessRequests, a.context.Identity.GetIdentity().ActiveRequests...) if err != nil { return nil, trace.Wrap(err) } diff --git a/lib/auth/sessions.go b/lib/auth/sessions.go index e99916ccdc7c4..e69df19265e1f 100644 --- a/lib/auth/sessions.go +++ b/lib/auth/sessions.go @@ -66,11 +66,12 @@ func (s *Server) CreateAppSession(ctx context.Context, req types.CreateAppSessio return nil, trace.Wrap(err) } certs, err := s.generateUserCert(certRequest{ - user: user, - publicKey: publicKey, - checker: checker, - ttl: ttl, - traits: traits, + user: user, + publicKey: publicKey, + checker: checker, + ttl: ttl, + traits: traits, + activeRequests: services.RequestIDs{AccessRequests: identity.ActiveRequests}, // Only allow this certificate to be used for applications. usage: []string{teleport.UsageAppsOnly}, // Add in the application routing information. diff --git a/lib/auth/tls_test.go b/lib/auth/tls_test.go index 0f2b38450cf54..8d4294c848f49 100644 --- a/lib/auth/tls_test.go +++ b/lib/auth/tls_test.go @@ -1557,6 +1557,19 @@ func (s *TLSSuite) TestWebSessionWithApprovedAccessRequestAndSwitchback(c *check _, hasRole = mappedRole["test-request-role"] c.Assert(hasRole, check.Equals, true) + // certRequests extracts the active requests from a PEM encoded TLS cert. + certRequests := func(tlsCert []byte) []string { + cert, err := tlsca.ParseCertificatePEM(tlsCert) + c.Assert(err, check.IsNil) + + identity, err := tlsca.FromSubject(cert.Subject, cert.NotAfter) + c.Assert(err, check.IsNil) + + return identity.ActiveRequests + } + + c.Assert(certRequests(sess1.GetTLSCert()), check.DeepEquals, []string{accessReq.GetName()}) + // Test switch back to default role and expiry. sess2, err := web.ExtendWebSession(WebSessionReq{ User: user, @@ -1573,6 +1586,8 @@ func (s *TLSSuite) TestWebSessionWithApprovedAccessRequestAndSwitchback(c *check roles, _, err = services.ExtractFromCertificate(clt, sshcert) c.Assert(err, check.IsNil) c.Assert(roles, check.DeepEquals, []string{initialRole}) + + c.Assert(certRequests(sess2.GetTLSCert()), check.HasLen, 0) } // TestGetCertAuthority tests certificate authority permissions @@ -1719,6 +1734,17 @@ func (s *TLSSuite) TestAccessRequest(c *check.C) { return apiutils.SliceContainsStr(identity.Groups, role) } + // certRequests extracts the active requests from a PEM encoded TLS cert. + certRequests := func(tlsCert []byte) []string { + cert, err := tlsca.ParseCertificatePEM(tlsCert) + c.Assert(err, check.IsNil) + + identity, err := tlsca.FromSubject(cert.Subject, cert.NotAfter) + c.Assert(err, check.IsNil) + + return identity.ActiveRequests + } + // certLogins extracts the logins from an ssh certificate certLogins := func(sshCert []byte) []string { cert, err := sshutils.ParseCertificate(sshCert) @@ -1732,6 +1758,8 @@ func (s *TLSSuite) TestAccessRequest(c *check.C) { if certContainsRole(userCerts.TLS, role) { c.Errorf("unexpected role %s", role) } + // ensure that the default identity doesn't have any active requests + c.Assert(certRequests(userCerts.TLS), check.HasLen, 0) // verify that cert for user with no static logins is generated with // exactly one login and that it is an invalid unix login (indicated @@ -1758,6 +1786,8 @@ func (s *TLSSuite) TestAccessRequest(c *check.C) { if !certContainsRole(userCerts.TLS, role) { c.Errorf("missing requested role %s", role) } + // ensure that the request is stored in the certs + c.Assert(certRequests(userCerts.TLS), check.DeepEquals, []string{req.GetName()}) // verify that dynamically applied role granted a login, // which is is valid and has replaced the dummy login. @@ -1765,6 +1795,27 @@ func (s *TLSSuite) TestAccessRequest(c *check.C) { c.Assert(len(logins), check.Equals, 1) c.Assert(rune(logins[0][0]), check.Not(check.Equals), '-') + elevatedCert, err := tls.X509KeyPair(userCerts.TLS, priv) + c.Assert(err, check.IsNil) + elevatedClient := s.server.NewClientWithCert(elevatedCert) + + newCerts, err := elevatedClient.GenerateUserCerts(ctx, proto.UserCertsRequest{ + PublicKey: pub, + Username: user, + Expires: time.Now().Add(time.Hour).UTC(), + Format: constants.CertificateFormatStandard, + // no new access requests + AccessRequests: nil, + }) + c.Assert(err, check.IsNil) + + // in spite of having no access requests, we still have elevated roles... + if !certContainsRole(newCerts.TLS, role) { + c.Errorf("missing requested role %s", role) + } + // ...and the certificate shows the access request + c.Assert(certRequests(newCerts.TLS), check.DeepEquals, []string{req.GetName()}) + // attempt to apply request in DENIED state (should fail) c.Assert(s.server.Auth().SetAccessRequestState(ctx, types.AccessRequestUpdate{RequestID: req.GetName(), State: types.RequestState_DENIED}), check.IsNil) _, err = generateCerts(req.GetName()) @@ -1775,6 +1826,17 @@ func (s *TLSSuite) TestAccessRequest(c *check.C) { // ensure that once in the DENIED state, a request cannot be set back to APPROVED state. c.Assert(s.server.Auth().SetAccessRequestState(ctx, types.AccessRequestUpdate{RequestID: req.GetName(), State: types.RequestState_APPROVED}), check.NotNil) + + // ensure that identities with requests in the DENIED state can't reissue new certs. + _, err = elevatedClient.GenerateUserCerts(ctx, proto.UserCertsRequest{ + PublicKey: pub, + Username: user, + Expires: time.Now().Add(time.Hour).UTC(), + Format: constants.CertificateFormatStandard, + // no new access requests + AccessRequests: nil, + }) + c.Assert(err, check.NotNil) } func (s *TLSSuite) TestPluginData(c *check.C) { diff --git a/lib/tlsca/ca.go b/lib/tlsca/ca.go index afc67083b753b..9e7f3c64328bb 100644 --- a/lib/tlsca/ca.go +++ b/lib/tlsca/ca.go @@ -126,6 +126,8 @@ type Identity struct { ClientIP string // AWSRoleARNs is a list of allowed AWS role ARNs user can assume. AWSRoleARNs []string + // ActiveRequests is a list of UUIDs of active requests for this Identity. + ActiveRequests []string } // RouteToApp holds routing information for applications. @@ -281,6 +283,10 @@ var ( // ImpersonatorASN1ExtensionOID is an extension OID used when encoding/decoding // impersonator user ImpersonatorASN1ExtensionOID = asn1.ObjectIdentifier{1, 3, 9999, 2, 7} + + // ActiveRequestsASN1ExtensionOID is an extension OID used when encoding/decoding + // active access requests into certificates. + ActiveRequestsASN1ExtensionOID = asn1.ObjectIdentifier{1, 3, 9999, 2, 8} ) // Subject converts identity to X.509 subject name @@ -452,6 +458,14 @@ func (id *Identity) Subject() (pkix.Name, error) { }) } + for _, activeRequest := range id.ActiveRequests { + subject.ExtraNames = append(subject.ExtraNames, + pkix.AttributeTypeAndValue{ + Type: ActiveRequestsASN1ExtensionOID, + Value: activeRequest, + }) + } + return subject, nil } @@ -571,6 +585,11 @@ func FromSubject(subject pkix.Name, expires time.Time) (*Identity, error) { if ok { id.Impersonator = val } + case attr.Type.Equal(ActiveRequestsASN1ExtensionOID): + val, ok := attr.Value.(string) + if ok { + id.ActiveRequests = append(id.ActiveRequests, val) + } } }