Skip to content

Commit

Permalink
Preserve roles from user cert when forwarding k8s requests
Browse files Browse the repository at this point in the history
When a proxy forwards a k8s request to another proxy in a trusted leaf
cluster, it dynamically generates a new client key/cert to impersonate
the user. This key/cert is presented to the leaf proxy as if a user was
directly making a request to it.

Auth server, when processing the CSR, would load user identity from the
backend. If this user had temporary role grants via workflow API, they
would not be loaded from the backend.

This means that if a user accesses k8s through:
```
kubectl -> root proxy -> leaf proxy -> k8s
```
the second hop would drop their temporary role grants.

To fix this, preserve full user identity from the original client cert
presented to root proxy, as encoded in CSR Subject. The auth server
implicitly trusts the CSR Subject that the proxy presents.
This also requires fixing the Subject encoding on the proxy side, which
was inconsistent with how auth server does it.

One downside here is: a compromised proxy can mint k8s client certs for
known users with arbitrary roles.
  • Loading branch information
Andrew Lytvynov committed Apr 24, 2020
1 parent 1755824 commit 4242478
Show file tree
Hide file tree
Showing 4 changed files with 209 additions and 35 deletions.
44 changes: 14 additions & 30 deletions lib/auth/kube.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ limitations under the License.
package auth

import (
"github.com/gravitational/teleport"
"time"

"github.com/gravitational/teleport/lib/defaults"
"github.com/gravitational/teleport/lib/modules"
"github.com/gravitational/teleport/lib/services"
Expand Down Expand Up @@ -84,29 +85,23 @@ func (s *AuthServer) ProcessKubeCSR(req KubeCSR) (*KubeCSRResponse, error) {
return nil, trace.Wrap(err)
}

user, err := s.GetUser(req.Username, false)
csr, err := tlsca.ParseCertificateRequestPEM(req.CSR)
if err != nil {
return nil, trace.Wrap(err)
}

roles, err := services.FetchRoles(user.GetRoles(), s, user.GetTraits())
// Extract user roles from the CSR. Pass zero time for id.Expiry, it won't
// be used here.
id, err := tlsca.FromSubject(csr.Subject, time.Time{})
if err != nil {
return nil, trace.Wrap(err)
}

ttl := roles.AdjustSessionTTL(defaults.CertDuration)

// extract and encode the kubernetes groups of the authenticated
// user in the newly issued certificate
kubernetesGroups, kubernetesUsers, err := roles.CheckKubeGroupsAndUsers(0)
if err != nil {
return nil, trace.Wrap(err)
}

csr, err := tlsca.ParseCertificateRequestPEM(req.CSR)
roles, err := services.FetchRoles(id.Groups, s, id.Traits)
if err != nil {
return nil, trace.Wrap(err)
}
// Get the correct cert TTL based on roles.
ttl := roles.AdjustSessionTTL(defaults.CertDuration)

userCA, err := s.Trust.GetCertAuthority(services.CertAuthID{
Type: services.UserCA,
Expand All @@ -121,25 +116,14 @@ func (s *AuthServer) ProcessKubeCSR(req KubeCSR) (*KubeCSRResponse, error) {
return nil, trace.Wrap(err)
}

identity := tlsca.Identity{
Username: user.GetName(),
Groups: roles.RoleNames(),
// Generate a certificate restricted for
// use against a kubernetes endpoint, and not the API server endpoint
// otherwise proxies can generate certs for any user.
Usage: []string{teleport.UsageKubeOnly},
KubernetesGroups: kubernetesGroups,
KubernetesUsers: kubernetesUsers,
}
subject, err := identity.Subject()
if err != nil {
return nil, trace.Wrap(err)
}
certRequest := tlsca.CertificateRequest{
Clock: s.clock,
PublicKey: csr.PublicKey,
Subject: subject,
NotAfter: s.clock.Now().UTC().Add(ttl),
// Always trust the Subject sent by the proxy. A user may have received
// temporary extra roles via workflow API, we must preserve those. The
// storage backend doesn't record temporary granted roles.
Subject: csr.Subject,
NotAfter: s.clock.Now().UTC().Add(ttl),
}
tlsCert, err := tlsAuthority.GenerateCertificate(certRequest)
if err != nil {
Expand Down
89 changes: 89 additions & 0 deletions lib/auth/kube_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package auth

import (
"crypto/rsa"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"math/rand"
"time"

"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/services/suite"
"github.com/gravitational/teleport/lib/tlsca"
"github.com/gravitational/trace"
"gopkg.in/check.v1"
)

func (s *AuthSuite) TestProcessKubeCSR(c *check.C) {
const (
username = "bob"
roleA = "user:bob"
roleB = "requestable"
clusterName = "me.localhost"
)
c.Assert(s.a.UpsertCertAuthority(suite.NewTestCA(services.UserCA, clusterName)), check.IsNil)
c.Assert(s.a.UpsertCertAuthority(suite.NewTestCA(services.HostCA, clusterName)), check.IsNil)

// Requested user identity, presented in CSR Subject.
userID := tlsca.Identity{
Username: username,
Groups: []string{roleA, roleB},
Usage: []string{"usage a", "usage b"},
Principals: []string{"principal a", "principal b"},
KubernetesGroups: []string{"k8s group a", "k8s group b"},
Traits: map[string][]string{"trait a": []string{"b", "c"}},
}
subj, err := userID.Subject()
c.Assert(err, check.IsNil)

pemCSR, err := newTestCSR(subj)
c.Assert(err, check.IsNil)
csr := KubeCSR{
Username: username,
ClusterName: clusterName,
CSR: pemCSR,
}

// CSR with unknown roles.
_, err = s.a.ProcessKubeCSR(csr)
c.Assert(err, check.NotNil)
c.Assert(trace.IsNotFound(err), check.Equals, true)

// Create the user and allow it to request the additional role.
_, err = CreateUserRoleAndRequestable(s.a, username, roleB)
c.Assert(err, check.IsNil)

// CSR with allowed, known roles.
resp, err := s.a.ProcessKubeCSR(csr)
c.Assert(err, check.IsNil)

cert, err := tlsca.ParseCertificatePEM(resp.Cert)
c.Assert(err, check.IsNil)
// Note: we could compare cert.Subject with subj here directly.
// However, because pkix.Name encoding/decoding isn't symmetric (ExtraNames
// before encoding becomes Names after decoding), they wouldn't match.
// Therefore, convert back to Identity, which handles this oddity and
// should match.
gotUserID, err := tlsca.FromSubject(cert.Subject, time.Time{})
c.Assert(err, check.IsNil)
c.Assert(*gotUserID, check.DeepEquals, userID)
}

// newTestCSR creates and PEM-encodes an x509 CSR with given subject.
func newTestCSR(subj pkix.Name) ([]byte, error) {
// Use math/rand to avoid blocking on system entropy.
rng := rand.New(rand.NewSource(0))
priv, err := rsa.GenerateKey(rng, 2048)
if err != nil {
return nil, err
}
x509CSR := &x509.CertificateRequest{
Subject: subj,
}
derCSR, err := x509.CreateCertificateRequest(rng, x509CSR, priv)
if err != nil {
return nil, err
}
return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE REQUEST", Bytes: derCSR}), nil
}
13 changes: 8 additions & 5 deletions lib/kube/proxy/forwarder.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"crypto/rand"
"crypto/tls"
"crypto/x509"
"crypto/x509/pkix"
"encoding/pem"
"fmt"
"io/ioutil"
Expand Down Expand Up @@ -1085,11 +1084,15 @@ func (f *Forwarder) requestCertificate(ctx authContext) (*bundle, error) {
return nil, trace.Wrap(err, "failed to parse private key")
}

// Note: ctx.Identity can potentially have temporary roles granted via
// workflow API. Always use the Subject() method to preserve the roles from
// caller's certificate.
subject, err := ctx.Identity.Subject()
if err != nil {
return nil, trace.Wrap(err)
}
csr := &x509.CertificateRequest{
Subject: pkix.Name{
CommonName: ctx.User.GetName(),
Organization: utils.StringsSliceFromSet(ctx.kubeGroups),
},
Subject: subject,
}
csrBytes, err := x509.CreateCertificateRequest(rand.Reader, csr, privateKey)
if err != nil {
Expand Down
98 changes: 98 additions & 0 deletions lib/kube/proxy/forwarder_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package proxy

import (
"crypto/x509"
"encoding/pem"
"time"

"github.com/gravitational/teleport/lib/auth"
"github.com/gravitational/teleport/lib/auth/testauthority"
"github.com/gravitational/teleport/lib/reversetunnel"
"github.com/gravitational/teleport/lib/services"
"github.com/gravitational/teleport/lib/tlsca"
"gopkg.in/check.v1"
)

type ForwarderSuite struct{}

var _ = check.Suite(ForwarderSuite{})

func (s ForwarderSuite) TestRequestCertificate(c *check.C) {
cl := &mockClient{
csrResp: auth.KubeCSRResponse{
Cert: []byte("mock cert"),
CertAuthorities: [][]byte{[]byte("mock CA")},
TargetAddr: "mock addr",
},
}
f := &Forwarder{
ForwarderConfig: ForwarderConfig{
Keygen: testauthority.New(),
Client: cl,
},
}
user, err := services.NewUser("bob")
c.Assert(err, check.IsNil)
ctx := authContext{
cluster: cluster{
RemoteSite: mockRemoteSite{name: "site a"},
},
AuthContext: auth.AuthContext{
User: user,
Identity: tlsca.Identity{
Username: "bob",
Groups: []string{"group a", "group b"},
Usage: []string{"usage a", "usage b"},
Principals: []string{"principal a", "principal b"},
KubernetesGroups: []string{"k8s group a", "k8s group b"},
Traits: map[string][]string{"trait a": []string{"b", "c"}},
},
},
}

b, err := f.requestCertificate(ctx)
c.Assert(err, check.IsNil)
// All fields except b.key are predictable.
c.Assert(b.cert, check.DeepEquals, cl.csrResp.Cert)
c.Assert(b.certAuthorities, check.DeepEquals, cl.csrResp.CertAuthorities)
c.Assert(b.targetAddr, check.DeepEquals, cl.csrResp.TargetAddr)

// Check the KubeCSR fields.
c.Assert(cl.gotCSR.Username, check.DeepEquals, ctx.User.GetName())
c.Assert(cl.gotCSR.ClusterName, check.DeepEquals, ctx.cluster.GetName())

// Parse x509 CSR and check the subject.
csrBlock, _ := pem.Decode(cl.gotCSR.CSR)
c.Assert(csrBlock, check.NotNil)
csr, err := x509.ParseCertificateRequest(csrBlock.Bytes)
c.Assert(err, check.IsNil)
idFromCSR, err := tlsca.FromSubject(csr.Subject, time.Time{})
c.Assert(err, check.IsNil)
c.Assert(idFromCSR, check.DeepEquals, ctx.Identity)
}

// mockClient to intercept ProcessKubeCSR requests, record them and return a
// stub response.
type mockClient struct {
auth.ClientI

csrResp auth.KubeCSRResponse
gotCSR auth.KubeCSR
}

func (c *mockClient) ProcessKubeCSR(csr auth.KubeCSR) (*auth.KubeCSRResponse, error) {
c.gotCSR = csr
return &c.csrResp, nil
}

// mockRemoteSite is a reversetunnel.RemoteSite implementation with hardcoded
// name, because there's no easy way to construct a real
// reversetunnel.RemoteSite.
type mockRemoteSite struct {
reversetunnel.RemoteSite
name string
}

func (s mockRemoteSite) GetName() string {
return s.name
}

0 comments on commit 4242478

Please sign in to comment.