Skip to content

Commit

Permalink
[v16] VNet: Make sure daemon client has adequate permissions for pass…
Browse files Browse the repository at this point in the history
…ed `TELEPORT_HOME` (#44823)

* Setup package logger for lib/vnet

* Pass egid and euid of client process from Obj-C to Go

* Drop root privileges before reading files from TELEPORT_HOME

* Adjust message for errXPCConnectionCodeSigningRequirementFailure

* Pass egid and euid to osascript

* Add Valid field to ClientCred to make sure creds were set

* Log dropping privileges before attempting to do so

* Panic instead of erroring when changing creds fails

* Make sure privileges can't be dropped multiple times in parallel

* Implement LogValue for ClientCred
  • Loading branch information
ravicious authored Jul 30, 2024
1 parent 68dd3d4 commit b86df29
Show file tree
Hide file tree
Showing 13 changed files with 242 additions and 57 deletions.
5 changes: 2 additions & 3 deletions lib/vnet/customdnszonevalidator.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package vnet
import (
"context"
"errors"
"log/slog"
"net"
"slices"
"sync"
Expand Down Expand Up @@ -67,7 +66,7 @@ func (c *customDNSZoneValidator) validate(ctx context.Context, clusterName, cust
}

requiredTXTRecord := clusterTXTRecordPrefix + clusterName
slog.InfoContext(ctx, "Checking validity of custom DNS zone by querying for required TXT record.", "zone", customDNSZone, "record", requiredTXTRecord)
log.InfoContext(ctx, "Checking validity of custom DNS zone by querying for required TXT record.", "zone", customDNSZone, "record", requiredTXTRecord)

records, err := c.lookupTXT(ctx, customDNSZone)
if err != nil {
Expand All @@ -83,7 +82,7 @@ func (c *customDNSZoneValidator) validate(ctx context.Context, clusterName, cust
return trace.Wrap(errNoTXTRecord(customDNSZone, requiredTXTRecord))
}

slog.InfoContext(ctx, "Custom DNS zone has valid TXT record.", "zone", customDNSZone, "cluster", clusterName)
log.InfoContext(ctx, "Custom DNS zone has valid TXT record.", "zone", customDNSZone, "cluster", clusterName)

c.mu.Lock()
defer c.mu.Unlock()
Expand Down
10 changes: 9 additions & 1 deletion lib/vnet/daemon/client_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,15 @@ func startByCalling(ctx context.Context, bundlePath string, config Config) error
}

if errorDomain == nsCocoaErrorDomain && errorCode == errorCodeNSXPCConnectionCodeSigningRequirementFailure {
errC <- trace.Wrap(errXPCConnectionCodeSigningRequirementFailure, "the daemon does not appear to be code signed correctly")
// If the client submits TELEPORT_HOME to which the user doesn't have access, the daemon is
// going to shut down with an error soon after starting. Because of that, macOS won't have
// enough time to perform the verification of the code signing requirement of the daemon, as
// requested by the client.
//
// In that scenario, macOS is going to simply error that connection with
// NSXPCConnectionCodeSigningRequirementFailure. Without looking at logs, it's not possible
// to differentiate that from a "legitimate" failure caused by an incorrect requirement.
errC <- trace.Wrap(errXPCConnectionCodeSigningRequirementFailure, "either daemon is not signed correctly or it shut down before signature could be verified")
return
}

Expand Down
23 changes: 23 additions & 0 deletions lib/vnet/daemon/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package daemon

import (
"log/slog"
"time"

"github.com/gravitational/trace"
Expand All @@ -34,6 +35,26 @@ type Config struct {
DNSAddr string
// HomePath points to TELEPORT_HOME that will be used by the admin process.
HomePath string
// ClientCred are the credentials of the unprivileged process that wants to start VNet.
ClientCred ClientCred
}

// ClientCred are the credentials of the unprivileged process that wants to start VNet.
type ClientCred struct {
// Valid is set if the Euid and Egid fields have been set.
Valid bool
// Egid is the effective group ID of the unprivileged process.
Egid int
// Euid is the effective user ID of the unprivileged process.
Euid int
}

func (c ClientCred) LogValue() slog.Value {
return slog.GroupValue(
slog.Bool("creds_valid", c.Valid),
slog.Int("egid", c.Egid),
slog.Int("euid", c.Euid),
)
}

func (c *Config) CheckAndSetDefaults() error {
Expand All @@ -46,6 +67,8 @@ func (c *Config) CheckAndSetDefaults() error {
return trace.BadParameter("missing DNS address")
case c.HomePath == "":
return trace.BadParameter("missing home path")
case c.ClientCred.Valid == false:
return trace.BadParameter("missing client credentials")
}
return nil
}
Expand Down
10 changes: 9 additions & 1 deletion lib/vnet/daemon/service_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ func Start(ctx context.Context, workFn func(context.Context, Config) error) erro
"ipv6_prefix", config.IPv6Prefix,
"dns_addr", config.DNSAddr,
"home_path", config.HomePath,
"client_cred", config.ClientCred,
)

return trace.Wrap(workFn(ctx, config))
Expand All @@ -101,8 +102,10 @@ func waitForVnetConfig(ctx context.Context) (Config, error) {
C.free(unsafe.Pointer(result.home_path))
}()

var clientCred C.ClientCred

// This call gets unblocked when the daemon gets stopped through C.DaemonStop.
C.WaitForVnetConfig(&result)
C.WaitForVnetConfig(&result, &clientCred)
if !result.ok {
errC <- trace.Wrap(errors.New(C.GoString(result.error_description)))
return
Expand All @@ -113,6 +116,11 @@ func waitForVnetConfig(ctx context.Context) (Config, error) {
IPv6Prefix: C.GoString(result.ipv6_prefix),
DNSAddr: C.GoString(result.dns_addr),
HomePath: C.GoString(result.home_path),
ClientCred: ClientCred{
Valid: bool(clientCred.valid),
Egid: int(clientCred.egid),
Euid: int(clientCred.euid),
},
}
errC <- nil
}()
Expand Down
11 changes: 10 additions & 1 deletion lib/vnet/daemon/service_darwin.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,20 @@ typedef struct VnetConfigResult {
const char *home_path;
} VnetConfigResult;

typedef struct ClientCred {
// valid is set if the euid and egid fields have been set.
bool valid;
// egid is the effective group ID of the process on the other side of the XPC connection.
gid_t egid;
// euid is the effective user ID of the process on the other side of the XPC connection.
uid_t euid;
} ClientCred;

// WaitForVnetConfig blocks until a client calls the daemon with a config necessary to start VNet.
// It can be interrupted by calling DaemonStop.
//
// The caller is expected to check outResult.ok to see if the call succeeded and to free strings
// in VnetConfigResult.
void WaitForVnetConfig(VnetConfigResult *outResult);
void WaitForVnetConfig(VnetConfigResult *outResult, ClientCred *outClientCred);

#endif /* TELEPORT_LIB_VNET_DAEMON_SERVICE_DARWIN_H_ */
31 changes: 30 additions & 1 deletion lib/vnet/daemon/service_darwin.m
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,21 @@

#include <string.h>

@interface VNEClientCred : NSObject
{
BOOL valid;
gid_t egid;
uid_t euid;
}
@property(nonatomic, readwrite) BOOL valid;
@property(nonatomic, readwrite) gid_t egid;
@property(nonatomic, readwrite) uid_t euid;
@end

@implementation VNEClientCred
@synthesize valid,egid,euid;
@end

@interface VNEDaemonService () <NSXPCListenerDelegate, VNEDaemonProtocol>

@property(nonatomic, strong, readwrite) NSXPCListener *listener;
Expand All @@ -37,6 +52,7 @@ @interface VNEDaemonService () <NSXPCListenerDelegate, VNEDaemonProtocol>
@property(nonatomic, readwrite) NSString *ipv6Prefix;
@property(nonatomic, readwrite) NSString *dnsAddr;
@property(nonatomic, readwrite) NSString *homePath;
@property(nonatomic, readwrite) VNEClientCred *clientCred;
@property(nonatomic, readwrite) dispatch_semaphore_t gotVnetConfigSema;

@end
Expand Down Expand Up @@ -106,6 +122,12 @@ - (void)startVnet:(VnetConfig *)vnetConfig completion:(void (^)(NSError *error))
_dnsAddr = @(vnetConfig->dns_addr);
_homePath = @(vnetConfig->home_path);

NSXPCConnection *currentConn = [NSXPCConnection currentConnection];
_clientCred = [[VNEClientCred alloc] init];
[_clientCred setEgid:[currentConn effectiveGroupIdentifier]];
[_clientCred setEuid:[currentConn effectiveUserIdentifier]];
[_clientCred setValid:YES];

dispatch_semaphore_signal(_gotVnetConfigSema);
completion(nil);
}
Expand Down Expand Up @@ -158,7 +180,7 @@ void DaemonStop(void) {
}
}

void WaitForVnetConfig(VnetConfigResult *outResult) {
void WaitForVnetConfig(VnetConfigResult *outResult, ClientCred *outClientCred) {
if (!daemonService) {
outResult->error_description = strdup("daemon was not initialized yet");
return;
Expand All @@ -180,6 +202,13 @@ void WaitForVnetConfig(VnetConfigResult *outResult) {
outResult->ipv6_prefix = VNECopyNSString([daemonService ipv6Prefix]);
outResult->dns_addr = VNECopyNSString([daemonService dnsAddr]);
outResult->home_path = VNECopyNSString([daemonService homePath]);

if ([daemonService clientCred] && [[daemonService clientCred] valid]) {
outClientCred->egid = [[daemonService clientCred] egid];
outClientCred->euid = [[daemonService clientCred] euid];
outClientCred->valid = true;
}

outResult->ok = true;
}
}
71 changes: 44 additions & 27 deletions lib/vnet/osconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ package vnet

import (
"context"
"log/slog"
"net"

"github.com/gravitational/trace"
Expand All @@ -28,6 +27,7 @@ import (
"github.com/gravitational/teleport/api/utils"
"github.com/gravitational/teleport/lib/client"
"github.com/gravitational/teleport/lib/client/clientcache"
"github.com/gravitational/teleport/lib/vnet/daemon"
)

type osConfig struct {
Expand All @@ -43,14 +43,16 @@ type osConfigurator struct {
clientStore *client.Store
clientCache *clientcache.Cache
clusterConfigCache *ClusterConfigCache
tunName string
tunIPv6 string
dnsAddr string
homePath string
tunIPv4 string
// daemonClientCred are the credentials of the process that contacted the daemon.
daemonClientCred daemon.ClientCred
tunName string
tunIPv6 string
dnsAddr string
homePath string
tunIPv4 string
}

func newOSConfigurator(tunName, ipv6Prefix, dnsAddr, homePath string) (*osConfigurator, error) {
func newOSConfigurator(tunName, ipv6Prefix, dnsAddr, homePath string, daemonClientCred daemon.ClientCred) (*osConfigurator, error) {
if homePath == "" {
// This runs as root so we need to be configured with the user's home path.
return nil, trace.BadParameter("homePath must be passed from unprivileged process")
Expand All @@ -61,11 +63,12 @@ func newOSConfigurator(tunName, ipv6Prefix, dnsAddr, homePath string) (*osConfig
tunIPv6 := ipv6Prefix + "1"

configurator := &osConfigurator{
tunName: tunName,
tunIPv6: tunIPv6,
dnsAddr: dnsAddr,
homePath: homePath,
clientStore: client.NewFSClientStore(homePath),
tunName: tunName,
tunIPv6: tunIPv6,
dnsAddr: dnsAddr,
homePath: homePath,
clientStore: client.NewFSClientStore(homePath),
daemonClientCred: daemonClientCred,
}
configurator.clusterConfigCache = NewClusterConfigCache(clockwork.NewRealClock())

Expand All @@ -89,18 +92,32 @@ func (c *osConfigurator) close() error {
return trace.Wrap(c.clientCache.Clear())
}

// updateOSConfiguration reads tsh profiles out of [c.homePath]. For each profile, it reads the VNet
// config of the root cluster and of each leaf cluster. Then it proceeds to update the OS based on
// information from that config.
//
// For the duration of reading data from clusters, it drops the root privileges, only to regain them
// before configuring the OS.
func (c *osConfigurator) updateOSConfiguration(ctx context.Context) error {
var dnsZones []string
var cidrRanges []string

profileNames, err := profile.ListProfileNames(c.homePath)
if err != nil {
return trace.Wrap(err, "listing user profiles")
}
for _, profileName := range profileNames {
profileDNSZones, profileCIDRRanges := c.getDNSZonesAndCIDRRangesForProfile(ctx, profileName)
dnsZones = append(dnsZones, profileDNSZones...)
cidrRanges = append(cidrRanges, profileCIDRRanges...)
// Drop privileges to ensure that the user who spawned the daemon client has privileges necessary
// to access c.homePath that it sent when starting the daemon.
// Otherwise a client could make the daemon read a profile out of any directory.
if err := c.doWithDroppedRootPrivileges(ctx, func() error {
profileNames, err := profile.ListProfileNames(c.homePath)
if err != nil {
return trace.Wrap(err, "listing user profiles")
}
for _, profileName := range profileNames {
profileDNSZones, profileCIDRRanges := c.getDNSZonesAndCIDRRangesForProfile(ctx, profileName)
dnsZones = append(dnsZones, profileDNSZones...)
cidrRanges = append(cidrRanges, profileCIDRRanges...)
}
return nil
}); err != nil {
return trace.Wrap(err)
}

dnsZones = utils.Deduplicate(dnsZones)
Expand All @@ -114,7 +131,7 @@ func (c *osConfigurator) updateOSConfiguration(ctx context.Context) error {
}
}

err = configureOS(ctx, &osConfig{
err := configureOS(ctx, &osConfig{
tunName: c.tunName,
tunIPv6: c.tunIPv6,
tunIPv4: c.tunIPv4,
Expand All @@ -137,22 +154,22 @@ func (c *osConfigurator) getDNSZonesAndCIDRRangesForProfile(ctx context.Context,
defer func() {
if shouldClearCacheForRoot {
if err := c.clientCache.ClearForRoot(profileName); err != nil {
slog.ErrorContext(ctx, "Error while clearing client cache", "profile", profileName, "error", err)
log.ErrorContext(ctx, "Error while clearing client cache", "profile", profileName, "error", err)
}
}
}()

rootClient, err := c.clientCache.Get(ctx, profileName, "" /*leafClusterName*/)
if err != nil {
slog.WarnContext(ctx,
log.WarnContext(ctx,
"Failed to get root cluster client from cache, profile may be expired, not configuring VNet for this cluster",
"profile", profileName, "error", err)

return
}
clusterConfig, err := c.clusterConfigCache.GetClusterConfig(ctx, rootClient)
if err != nil {
slog.WarnContext(ctx,
log.WarnContext(ctx,
"Failed to load VNet configuration, profile may be expired, not configuring VNet for this cluster",
"profile", profileName, "error", err)

Expand All @@ -164,7 +181,7 @@ func (c *osConfigurator) getDNSZonesAndCIDRRangesForProfile(ctx context.Context,

leafClusters, err := getLeafClusters(ctx, rootClient)
if err != nil {
slog.WarnContext(ctx,
log.WarnContext(ctx,
"Failed to list leaf clusters, profile may be expired, not configuring VNet for leaf clusters of this cluster",
"profile", profileName, "error", err)

Expand All @@ -179,15 +196,15 @@ func (c *osConfigurator) getDNSZonesAndCIDRRangesForProfile(ctx context.Context,
for _, leafClusterName := range leafClusters {
clusterClient, err := c.clientCache.Get(ctx, profileName, leafClusterName)
if err != nil {
slog.WarnContext(ctx,
log.WarnContext(ctx,
"Failed to create leaf cluster client, not configuring VNet for this cluster",
"profile", profileName, "leaf_cluster", leafClusterName, "error", err)
continue
}

clusterConfig, err := c.clusterConfigCache.GetClusterConfig(ctx, clusterClient)
if err != nil {
slog.WarnContext(ctx,
log.WarnContext(ctx,
"Failed to load VNet configuration, not configuring VNet for this cluster",
"profile", profileName, "leaf_cluster", leafClusterName, "error", err)
continue
Expand Down
Loading

0 comments on commit b86df29

Please sign in to comment.