Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v12] Fix relaxed moderator joining for Kube Access #23993

Merged
merged 1 commit into from Apr 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
91 changes: 49 additions & 42 deletions lib/kube/proxy/forwarder.go
Expand Up @@ -447,10 +447,11 @@ type handlerWithAuthFunc func(ctx *authContext, w http.ResponseWriter, r *http.R
// handlerWithAuthFuncStd is http handler with passed auth context
type handlerWithAuthFuncStd func(ctx *authContext, w http.ResponseWriter, r *http.Request) (any, error)

// accessDeniedMsg is a message returned to the client when access is denied.
const accessDeniedMsg = "[00] access denied"

// authenticate function authenticates request
func (f *Forwarder) authenticate(req *http.Request) (*authContext, error) {
const accessDeniedMsg = "[00] access denied"

var isRemoteUser bool
userTypeI, err := authz.UserFromContext(req.Context())
if err != nil {
Expand Down Expand Up @@ -585,9 +586,7 @@ func (f *Forwarder) withAuthPassthrough(handler handlerWithAuthFunc) httprouter.
return httplib.MakeHandlerWithErrorWriter(func(w http.ResponseWriter, req *http.Request, p httprouter.Params) (any, error) {
authContext, err := f.authenticate(req)
if err != nil {
if !trace.IsAccessDenied(err) && !trace.IsNotFound(err) {
return nil, trace.Wrap(err)
}
return nil, trace.Wrap(err)
}
err = f.acquireConnectionLockWithIdentity(req.Context(), authContext)
if err != nil {
Expand Down Expand Up @@ -673,10 +672,8 @@ func (f *Forwarder) setupContext(authCtx authz.Context, req *http.Request, isRem
}

var (
kubeUsers, kubeGroups []string
kubeLabels map[string]string
kubeServers []types.KubeServer
err error
kubeServers []types.KubeServer
err error
)
// Only check k8s principals for local clusters.
//
Expand All @@ -687,27 +684,8 @@ func (f *Forwarder) setupContext(authCtx authz.Context, req *http.Request, isRem
if err != nil || len(kubeServers) == 0 {
return nil, trace.NotFound("cluster %q not found", kubeCluster)
}
// check signing TTL and return a list of allowed logins for local cluster based on Kubernetes service labels.
kubeAccessDetails, err := f.getKubeAccessDetails(kubeServers, roles, kubeCluster, sessionTTL, kubeResource)
if err != nil && !trace.IsNotFound(err) {
return nil, trace.Wrap(err)
// roles.CheckKubeGroupsAndUsers returns trace.NotFound if the user does
// does not have at least one configured kubernetes_users or kubernetes_groups.
} else if trace.IsNotFound(err) {
const errMsg = "Your user's Teleport role does not allow Kubernetes access." +
" Please ask cluster administrator to ensure your role has appropriate kubernetes_groups and kubernetes_users set."
return nil, trace.NotFound(errMsg)
}

kubeUsers = kubeAccessDetails.kubeUsers
kubeGroups = kubeAccessDetails.kubeGroups
kubeLabels = kubeAccessDetails.clusterLabels
}

// fillDefaultKubePrincipalDetails fills the default details in order to keep
// the correct behavior when forwarding the request to the Kubernetes API.
kubeUsers, kubeGroups = fillDefaultKubePrincipalDetails(kubeUsers, kubeGroups, authCtx.User.GetName())

clientSrc, clientDst := utils.ClientAddrFromContext(req.Context())

forwarderType := f.cfg.KubeServiceType
Expand Down Expand Up @@ -750,7 +728,6 @@ func (f *Forwarder) setupContext(authCtx authz.Context, req *http.Request, isRem
}

dialFn = func(ctx context.Context, network string, endpoint kubeClusterEndpoint) (net.Conn, error) {

// Make sure we will only send signed PROXY headers to teleport kube service node, not real kube cluster
if forwarderType != ProxyService && endpoint.serverID == "" {
clientDst = nil
Expand Down Expand Up @@ -811,9 +788,6 @@ func (f *Forwarder) setupContext(authCtx authz.Context, req *http.Request, isRem
clientIdleTimeout: roles.AdjustClientIdleTimeout(netConfig.GetClientIdleTimeout()),
sessionTTL: sessionTTL,
Context: authCtx,
kubeGroups: utils.StringsSet(kubeGroups),
kubeUsers: utils.StringsSet(kubeUsers),
kubeClusterLabels: kubeLabels,
recordingConfig: recordingConfig,
kubeClusterName: kubeCluster,
kubeResource: kubeResource,
Expand Down Expand Up @@ -1022,6 +996,38 @@ func (f *Forwarder) authorize(ctx context.Context, actx *authContext) error {
services.NewKubernetesResourceMatcher(*actx.kubeResource),
}
}
var kubeUsers, kubeGroups []string
// Only check k8s principals for local clusters.
//
// For remote clusters, everything will be remapped to new roles on the
// leaf and checked there.
if !actx.teleportCluster.isRemote {
// check signing TTL and return a list of allowed logins for local cluster based on Kubernetes service labels.
kubeAccessDetails, err := f.getKubeAccessDetails(actx.kubeServers, actx.Checker, actx.kubeClusterName, actx.sessionTTL, actx.kubeResource)
if err != nil && !trace.IsNotFound(err) {
if actx.kubeResource != nil {
return trace.AccessDenied(notFoundMessage)
}
// TODO (tigrato): should return another message here.
return trace.AccessDenied(accessDeniedMsg)
// roles.CheckKubeGroupsAndUsers returns trace.NotFound if the user does
// does not have at least one configured kubernetes_users or kubernetes_groups.
} else if trace.IsNotFound(err) {
const errMsg = "Your user's Teleport role does not allow Kubernetes access." +
" Please ask cluster administrator to ensure your role has appropriate kubernetes_groups and kubernetes_users set."
return trace.NotFound(errMsg)
}

kubeUsers = kubeAccessDetails.kubeUsers
kubeGroups = kubeAccessDetails.kubeGroups
actx.kubeClusterLabels = kubeAccessDetails.clusterLabels
}

// fillDefaultKubePrincipalDetails fills the default details in order to keep
// the correct behavior when forwarding the request to the Kubernetes API.
kubeUsers, kubeGroups = fillDefaultKubePrincipalDetails(kubeUsers, kubeGroups, actx.User.GetName())
actx.kubeUsers = utils.StringsSet(kubeUsers)
actx.kubeGroups = utils.StringsSet(kubeGroups)

// Check authz against the first match.
//
Expand Down Expand Up @@ -1127,7 +1133,7 @@ func (f *Forwarder) join(ctx *authContext, w http.ResponseWriter, req *http.Requ
// the resources as soon as we know the session is no longer active.
defer sess.close()

if err := f.setupForwardingHeaders(sess, req); err != nil {
if err := f.setupForwardingHeaders(sess, req, false /* withImpersonationHeaders */); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -1560,7 +1566,7 @@ func (f *Forwarder) exec(ctx *authContext, w http.ResponseWriter, req *http.Requ
onResize: func(remotecommand.TerminalSize) {},
}

if err := f.setupForwardingHeaders(sess, req); err != nil {
if err := f.setupForwardingHeaders(sess, req, true /* withImpersonationHeaders */); err != nil {
return nil, trace.Wrap(err)
}

Expand Down Expand Up @@ -1648,7 +1654,7 @@ func (f *Forwarder) portForward(ctx *authContext, w http.ResponseWriter, req *ht
return nil, trace.Wrap(err)
}

if err := f.setupForwardingHeaders(sess, req); err != nil {
if err := f.setupForwardingHeaders(sess, req, true /* withImpersonationHeaders */); err != nil {
f.log.Debugf("DENIED Port forward: %v.", req.URL.String())
return nil, trace.Wrap(err)
}
Expand Down Expand Up @@ -1730,11 +1736,12 @@ const (
ImpersonationRequestDeniedMessage = "impersonation request has been denied"
)

func (f *Forwarder) setupForwardingHeaders(sess *clusterSession, req *http.Request) error {
if err := setupImpersonationHeaders(f.log, sess.authContext, req.Header); err != nil {
return trace.Wrap(err)
func (f *Forwarder) setupForwardingHeaders(sess *clusterSession, req *http.Request, withImpersonationHeaders bool) error {
if withImpersonationHeaders {
if err := setupImpersonationHeaders(f.log, sess.authContext, req.Header); err != nil {
return trace.Wrap(err)
}
}

// Setup scheme, override target URL to the destination address
req.URL.Scheme = "https"
req.RequestURI = req.URL.Path + "?" + req.URL.RawQuery
Expand Down Expand Up @@ -1905,7 +1912,7 @@ func (f *Forwarder) catchAll(ctx *authContext, w http.ResponseWriter, req *http.
return nil, trace.Wrap(err)
}

if err := f.setupForwardingHeaders(sess, req); err != nil {
if err := f.setupForwardingHeaders(sess, req, true /* withImpersonationHeaders */); err != nil {
// This error goes to kubernetes client and is not visible in the logs
// of the teleport server if not logged here.
f.log.Errorf("Failed to set up forwarding headers: %v.", err)
Expand Down Expand Up @@ -2472,7 +2479,7 @@ func (f *Forwarder) listPods(ctx *authContext, w http.ResponseWriter, req *http.
return nil, trace.Wrap(err)
}

if err := f.setupForwardingHeaders(sess, req); err != nil {
if err := f.setupForwardingHeaders(sess, req, true /* withImpersonationHeaders */); err != nil {
// This error goes to kubernetes client and is not visible in the logs
// of the teleport server if not logged here.
f.log.Errorf("Failed to set up forwarding headers: %v.", err)
Expand Down Expand Up @@ -2578,7 +2585,7 @@ func (f *Forwarder) deletePodsCollection(ctx *authContext, w http.ResponseWriter
return nil, trace.Wrap(err)
}

if err := f.setupForwardingHeaders(sess, req); err != nil {
if err := f.setupForwardingHeaders(sess, req, true /* withImpersonationHeaders */); err != nil {
// This error goes to kubernetes client and is not visible in the logs
// of the teleport server if not logged here.
f.log.Errorf("Failed to set up forwarding headers: %v.", err)
Expand Down
11 changes: 2 additions & 9 deletions lib/kube/proxy/forwarder_test.go
Expand Up @@ -462,8 +462,6 @@ func TestAuthenticate(t *testing.T) {
tunnel: tun,

wantCtx: &authContext{
kubeUsers: utils.StringsSet([]string{"user-a"}),
kubeGroups: utils.StringsSet([]string{teleport.KubeSystemAuthenticated}),
certExpires: certExpiration,
teleportCluster: teleportClusterClient{
name: "remote",
Expand All @@ -481,8 +479,6 @@ func TestAuthenticate(t *testing.T) {
tunnel: tun,

wantCtx: &authContext{
kubeUsers: utils.StringsSet([]string{"user-a"}),
kubeGroups: utils.StringsSet([]string{teleport.KubeSystemAuthenticated}),
certExpires: certExpiration,
teleportCluster: teleportClusterClient{
name: "remote",
Expand All @@ -500,8 +496,6 @@ func TestAuthenticate(t *testing.T) {
tunnel: tun,

wantCtx: &authContext{
kubeUsers: utils.StringsSet([]string{"user-a"}),
kubeGroups: utils.StringsSet([]string{teleport.KubeSystemAuthenticated}),
certExpires: certExpiration,
teleportCluster: teleportClusterClient{
name: "remote",
Expand Down Expand Up @@ -709,8 +703,6 @@ func TestAuthenticate(t *testing.T) {
tunnel: tun,

wantCtx: &authContext{
kubeUsers: utils.StringsSet([]string{"user-a"}),
kubeGroups: utils.StringsSet([]string{teleport.KubeSystemAuthenticated}),
kubeClusterName: "foo",
certExpires: certExpiration,
teleportCluster: teleportClusterClient{
Expand Down Expand Up @@ -780,11 +772,12 @@ func TestAuthenticate(t *testing.T) {
require.Equal(t, trace.IsAccessDenied(err), tt.wantAuthErr)
return
}
err = f.authorize(context.Background(), gotCtx)
require.NoError(t, err)

require.Empty(t, cmp.Diff(gotCtx, tt.wantCtx,
cmp.AllowUnexported(authContext{}, teleportClusterClient{}),
cmpopts.IgnoreFields(authContext{}, "clientIdleTimeout", "sessionTTL", "Context", "recordingConfig", "disconnectExpiredCert"),
cmpopts.IgnoreFields(authContext{}, "clientIdleTimeout", "sessionTTL", "Context", "recordingConfig", "disconnectExpiredCert", "kubeCluster"),
cmpopts.IgnoreFields(teleportClusterClient{}, "dial", "isRemoteClosed"),
))

Expand Down
8 changes: 5 additions & 3 deletions lib/kube/proxy/moderated_sessions_test.go
Expand Up @@ -118,9 +118,7 @@ func TestModeratedSessions(t *testing.T) {
t,
moderatorUsername,
RoleSpec{
Name: moderatorRoleName,
KubeUsers: roleKubeUsers,
KubeGroups: roleKubeGroups,
Name: moderatorRoleName,
// sessionJoin:
SessionJoin: []*types.SessionJoinPolicy{
{
Expand All @@ -130,6 +128,10 @@ func TestModeratedSessions(t *testing.T) {
Modes: []string{string(types.SessionModeratorMode)},
},
},
SetupRoleFunc: func(r types.Role) {
// set kubernetes labels to empty to test relaxed join rules
r.SetKubernetesLabels(types.Allow, types.Labels{})
},
})

// create a userRequiringModerator with access to kubernetes thar requires
Expand Down
6 changes: 1 addition & 5 deletions lib/kube/proxy/sess.go
Expand Up @@ -828,11 +828,7 @@ func (s *session) lockedSetupLaunch(request *remoteCommandRequest, q url.Values,
// join attempts to connect a party to the session.
func (s *session) join(p *party) error {
if p.Ctx.User.GetName() != s.ctx.User.GetName() {
roleNames := p.Ctx.Identity.GetIdentity().Groups
roles, err := getRolesByName(s.forwarder, roleNames)
if err != nil {
return trace.Wrap(err)
}
roles := p.Ctx.Checker.Roles()

accessContext := auth.SessionAccessContext{
Username: p.Ctx.User.GetName(),
Expand Down
1 change: 0 additions & 1 deletion lib/web/apiserver_test.go
Expand Up @@ -2314,7 +2314,6 @@ func TestMotD(t *testing.T) {

// TestPingAutomaticUpgrades ensures /webapi/ping returns whether AutomaticUpgrades are enabled.
func TestPingAutomaticUpgrades(t *testing.T) {

t.Run("Automatic Upgrades are enabled", func(t *testing.T) {
// Enable Automatic Upgrades
modules.SetTestModules(t, &modules.TestModules{TestFeatures: modules.Features{
Expand Down