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

VNet: Make sure daemon client has adequate permissions for passed TELEPORT_HOME #44751

Merged
merged 10 commits into from
Jul 30, 2024
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")
espadolini marked this conversation as resolved.
Show resolved Hide resolved
}
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;
espadolini marked this conversation as resolved.
Show resolved Hide resolved
} 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
Loading