Skip to content

Commit

Permalink
Merge branch 'branch/v15' into joerger/v15/cli-prompt-refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
Joerger authored Oct 28, 2024
2 parents fc750a2 + eb09aa8 commit a9a6aa0
Show file tree
Hide file tree
Showing 22 changed files with 776 additions and 205 deletions.
10 changes: 7 additions & 3 deletions api/types/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -521,11 +521,15 @@ func (u UserV2) GetGCPServiceAccounts() []string {

// GetUserType indicates if the User was created by an SSO Provider or locally.
func (u UserV2) GetUserType() UserType {
if u.GetCreatedBy().Connector == nil {
return UserTypeLocal
if u.GetCreatedBy().Connector != nil ||
len(u.GetOIDCIdentities()) > 0 ||
len(u.GetGithubIdentities()) > 0 ||
len(u.GetSAMLIdentities()) > 0 {

return UserTypeSSO
}

return UserTypeSSO
return UserTypeLocal
}

// IsBot returns true if the user is a bot.
Expand Down
4 changes: 4 additions & 0 deletions constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,10 @@ const (
// HomeDirNotFound is returned when a the "teleport checkhomedir" command cannot
// find the user's home directory.
HomeDirNotFound = 254
// HomeDirNotAccessible is returned when a the "teleport checkhomedir" command has
// found the user's home directory, but the user does NOT have permissions to
// access it.
HomeDirNotAccessible = 253
)

// MaxEnvironmentFileLines is the maximum number of lines in a environment file.
Expand Down
8 changes: 7 additions & 1 deletion docs/pages/enroll-resources/desktop-access/rbac.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ desktop access:

```yaml
kind: role
version: v4
version: v5
metadata:
name: developer
spec:
Expand All @@ -31,6 +31,12 @@ spec:
# the clipboard, then it will be disabled.
desktop_clipboard: true

# Specify whether directory sharing should be allowed from the
# local machine to remote desktop (requires a supported browser). Defaults to true
# if unspecified. If one or more of the user's roles has disabled
# directory sharing, then it will be disabled.
desktop_directory_sharing: true

# Specify whether local users should be created automatically at connection
# time. By default, this feature is disabled, and the user must already exist.
# Note: this is applicable to local users only and is not supported in Active
Expand Down
5 changes: 5 additions & 0 deletions docs/pages/includes/role-spec.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,11 @@ spec:
# if unspecified. If one or more of the user's roles has disabled
# the clipboard, then it will be disabled.
desktop_clipboard: true
# Specify whether directory sharing should be allowed from the
# local machine to remote desktop (requires a supported browser). Defaults to true
# if unspecified. If one or more of the user's roles has disabled
# directory sharing, then it will be disabled.
desktop_directory_sharing: true
# enterprise-only: when enabled, the source IP that was used to log in is embedded in the user
# certificates, preventing a compromised certificate from being used on another
# network. The default is false.
Expand Down
1 change: 1 addition & 0 deletions docs/pages/reference/access-controls/roles.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ user:
| `max_kubernetes_connections` | Defines the maximum number of concurrent Kubernetes sessions per user | |
| `record_session` |Defines the [Session recording mode](../monitoring/audit.mdx).|The strictest value takes precedence.|
| `desktop_clipboard` | Allow clipboard sharing for desktop sessions | Logical "AND" i.e. evaluates to "yes" if all roles enable clipboard sharing |
| `desktop_directory_sharing` | Allows sharing local workstation directory to remote desktop | Logical "AND" i.e. evaluates to "yes" if all roles enable directory sharing |
| `pin_source_ip` | Enable source IP pinning for SSH certificates. | Logical "OR" i.e. evaluates to "yes" if at least one role requires session termination |
| `cert_extensions` | Specifies extensions to be included in SSH certificates | |
| `create_host_user_mode` | Allow users to be automatically created on a host | Logical "AND" i.e. if all roles matching a server specify host user creation (`off`, `keep`, `insecure-drop`), it will evaluate to the option specified by all of the roles. If some roles specify both `insecure-drop` or `keep` it will evaluate to `keep`|
Expand Down
7 changes: 7 additions & 0 deletions integration/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4970,6 +4970,13 @@ func testProxyHostKeyCheck(t *testing.T, suite *integrationTestSuite) {
_, err = clt.UpsertNode(context.Background(), server)
require.NoError(t, err)

// Wait for the node to be visible before continuing.
require.EventuallyWithT(t, func(t *assert.CollectT) {
found, err := clt.GetNodes(context.Background(), defaults.Namespace)
assert.NoError(t, err)
assert.Len(t, found, 2)
}, 10*time.Second, 100*time.Millisecond)

_, err = runCommand(t, instance, []string{"echo hello"}, clientConfig, 1)

// check if we were able to exec the command or not
Expand Down
4 changes: 2 additions & 2 deletions lib/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,9 +471,9 @@ func NewServer(cfg *InitConfig, opts ...ServerOption) (*Server, error) {
log.Warnf("missing connected resources gauge for keep alive %s (this is a bug)", s)
}
}),
inventory.WithOnDisconnect(func(s string) {
inventory.WithOnDisconnect(func(s string, c int) {
if g, ok := connectedResourceGauges[s]; ok {
g.Dec()
g.Sub(float64(c))
} else {
log.Warnf("missing connected resources gauge for keep alive %s (this is a bug)", s)
}
Expand Down
32 changes: 18 additions & 14 deletions lib/inventory/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ type controllerOptions struct {
maxKeepAliveErrs int
authID string
onConnectFunc func(string)
onDisconnectFunc func(string)
onDisconnectFunc func(string, int)
}

func (options *controllerOptions) SetDefaults() {
Expand All @@ -127,11 +127,11 @@ func (options *controllerOptions) SetDefaults() {
}

if options.onConnectFunc == nil {
options.onConnectFunc = func(s string) {}
options.onConnectFunc = func(string) {}
}

if options.onDisconnectFunc == nil {
options.onDisconnectFunc = func(s string) {}
options.onDisconnectFunc = func(string, int) {}
}
}

Expand All @@ -154,12 +154,12 @@ func WithOnConnect(f func(heartbeatKind string)) ControllerOption {
}
}

// WithOnDisconnect sets a function to be called every time an existing
// instance disconnects from the inventory control stream. The value
// provided to the callback is the keep alive type of the disconnected
// resource. The callback should return quickly so as not to prevent
// processing of heartbeats.
func WithOnDisconnect(f func(heartbeatKind string)) ControllerOption {
// WithOnDisconnect sets a function to be called every time an existing instance
// disconnects from the inventory control stream. The values provided to the
// callback are the keep alive type of the disconnected resource, as well as a
// count of how many resources disconnected at once. The callback should return
// quickly so as not to prevent processing of heartbeats.
func WithOnDisconnect(f func(heartbeatKind string, amount int)) ControllerOption {
return func(opts *controllerOptions) {
opts.onDisconnectFunc = f
}
Expand Down Expand Up @@ -200,7 +200,7 @@ type Controller struct {
usageReporter usagereporter.UsageReporter
testEvents chan testEvent
onConnectFunc func(string)
onDisconnectFunc func(string)
onDisconnectFunc func(string, int)
closeContext context.Context
cancel context.CancelFunc
}
Expand Down Expand Up @@ -324,7 +324,10 @@ func (c *Controller) handleControlStream(handle *upstreamHandle) {

defer func() {
if handle.goodbye.GetDeleteResources() {
log.WithField("apps", len(handle.appServers)).Debug("Cleaning up resources in response to instance termination")
log.WithFields(log.Fields{
"apps": len(handle.appServers),
"server_id": handle.Hello().ServerID,
}).Debug("Cleaning up resources in response to instance termination")
for _, app := range handle.appServers {
if err := c.auth.DeleteApplicationServer(c.closeContext, apidefaults.Namespace, app.resource.GetHostID(), app.resource.GetName()); err != nil && !trace.IsNotFound(err) {
log.Warnf("Failed to remove app server %q on termination: %v.", handle.Hello().ServerID, err)
Expand All @@ -341,11 +344,11 @@ func (c *Controller) handleControlStream(handle *upstreamHandle) {
handle.ticker.Stop()

if handle.sshServer != nil {
c.onDisconnectFunc(constants.KeepAliveNode)
c.onDisconnectFunc(constants.KeepAliveNode, 1)
}

for range handle.appServers {
c.onDisconnectFunc(constants.KeepAliveApp)
if len(handle.appServers) > 0 {
c.onDisconnectFunc(constants.KeepAliveApp, len(handle.appServers))
}

clear(handle.appServers)
Expand Down Expand Up @@ -677,6 +680,7 @@ func (c *Controller) keepAliveAppServer(handle *upstreamHandle, now time.Time) e

if shouldRemove {
c.testEvent(appKeepAliveDel)
c.onDisconnectFunc(constants.KeepAliveApp, 1)
delete(handle.appServers, name)
}
} else {
Expand Down
47 changes: 46 additions & 1 deletion lib/inventory/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,11 +144,14 @@ func TestSSHServerBasics(t *testing.T) {
expectAddr: wantAddr,
}

rc := &resourceCounter{}
controller := NewController(
auth,
usagereporter.DiscardUsageReporter{},
withServerKeepAlive(time.Millisecond*200),
withTestEventsChannel(events),
WithOnConnect(rc.onConnect),
WithOnDisconnect(rc.onDisconnect),
)
defer controller.Close()

Expand Down Expand Up @@ -282,6 +285,9 @@ func TestSSHServerBasics(t *testing.T) {
// here).
require.Equal(t, int64(0), controller.instanceHBVariableDuration.Count())

// verify that metrics have been updated correctly
require.Zero(t, 0, rc.count())

// verify that the peer address of the control stream was used to override
// zero-value IPs for heartbeats.
auth.mu.Lock()
Expand All @@ -305,11 +311,14 @@ func TestAppServerBasics(t *testing.T) {

auth := &fakeAuth{}

rc := &resourceCounter{}
controller := NewController(
auth,
usagereporter.DiscardUsageReporter{},
withServerKeepAlive(time.Millisecond*200),
withTestEventsChannel(events),
WithOnConnect(rc.onConnect),
WithOnDisconnect(rc.onDisconnect),
)
defer controller.Close()

Expand Down Expand Up @@ -500,6 +509,9 @@ func TestAppServerBasics(t *testing.T) {
// always *before* closure is propagated to downstream handle, hence being safe to load
// here).
require.Equal(t, int64(0), controller.instanceHBVariableDuration.Count())

// verify that metrics have been updated correctly
require.Zero(t, rc.count())
}

// TestInstanceHeartbeat verifies basic expected behaviors for instance heartbeat.
Expand Down Expand Up @@ -897,7 +909,6 @@ func TestGoodbye(t *testing.T) {
}

func TestGetSender(t *testing.T) {

controller := NewController(
&fakeAuth{},
usagereporter.DiscardUsageReporter{},
Expand Down Expand Up @@ -1008,3 +1019,37 @@ func awaitEvents(t *testing.T, ch <-chan testEvent, opts ...eventOption) {
}
}
}

type resourceCounter struct {
mu sync.Mutex
c map[string]int
}

func (r *resourceCounter) onConnect(typ string) {
r.mu.Lock()
defer r.mu.Unlock()
if r.c == nil {
r.c = make(map[string]int)
}
r.c[typ]++
}

func (r *resourceCounter) onDisconnect(typ string, amount int) {
r.mu.Lock()
defer r.mu.Unlock()
if r.c == nil {
r.c = make(map[string]int)
}
r.c[typ] -= amount
}

func (r *resourceCounter) count() int {
r.mu.Lock()
defer r.mu.Unlock()

var count int
for _, v := range r.c {
count += v
}
return count
}
3 changes: 3 additions & 0 deletions lib/srv/app/connections_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -755,6 +755,9 @@ func (c *ConnectionsHandler) deleteConnAuth(conn net.Conn) {
// for Teleport application proxy servers.
func CopyAndConfigureTLS(log logrus.FieldLogger, client authclient.AccessCache, config *tls.Config) *tls.Config {
tlsConfig := config.Clone()
if log == nil {
log = logrus.StandardLogger()
}

// Require clients to present a certificate
tlsConfig.ClientAuth = tls.RequireAndVerifyClientCert
Expand Down
35 changes: 28 additions & 7 deletions lib/srv/exec_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"os"
"os/exec"
"os/user"
"path/filepath"
"strconv"
"syscall"
"testing"
Expand All @@ -41,17 +42,40 @@ import (
)

func TestOSCommandPrep(t *testing.T) {
utils.RequireRoot(t)

srv := newMockServer(t)
scx := newExecServerContext(t, srv)

usr, err := user.Current()
// because CheckHomeDir now inspects access to the home directory as the actual user after a rexec,
// we need to setup a real, non-root user with a valid home directory in order for this test to
// exercise the correct paths
tempHome := t.TempDir()
require.NoError(t, os.Chmod(filepath.Dir(tempHome), 0777))

username := "test-os-command-prep"
scx.Identity.Login = username
_, err := host.UserAdd(username, nil, tempHome, "", "")
require.NoError(t, err)
t.Cleanup(func() {
// change homedir back so user deletion doesn't fail
changeHomeDir(t, username, tempHome)
_, err := host.UserDel(username)
require.NoError(t, err)
})

usr, err := user.Lookup(username)
require.NoError(t, err)

uid, err := strconv.Atoi(usr.Uid)
require.NoError(t, err)

require.NoError(t, os.Chown(tempHome, uid, -1))
expectedEnv := []string{
"LANG=en_US.UTF-8",
getDefaultEnvPath(strconv.Itoa(os.Geteuid()), defaultLoginDefsPath),
getDefaultEnvPath(usr.Uid, defaultLoginDefsPath),
fmt.Sprintf("HOME=%s", usr.HomeDir),
fmt.Sprintf("USER=%s", usr.Username),
fmt.Sprintf("USER=%s", username),
"SHELL=/bin/sh",
"SSH_CLIENT=10.0.0.5 4817 3022",
"SSH_CONNECTION=10.0.0.5 4817 127.0.0.1 3022",
Expand Down Expand Up @@ -104,12 +128,9 @@ func TestOSCommandPrep(t *testing.T) {
require.Equal(t, []string{"/bin/sh", "-c", "top"}, cmd.Args)
require.Equal(t, syscall.SIGKILL, cmd.SysProcAttr.Pdeathsig)

if os.Geteuid() != 0 {
t.Skip("skipping portion of test which must run as root")
}

// Missing home directory - HOME should still be set to the given
// home dir, but the command should set it's CWD to root instead.
changeHomeDir(t, username, "/wrong/place")
usr.HomeDir = "/wrong/place"
root := string(os.PathSeparator)
expectedEnv[2] = "HOME=/wrong/place"
Expand Down
Loading

0 comments on commit a9a6aa0

Please sign in to comment.