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 daemon: Set code signing requirements #44639

Merged
merged 9 commits into from
Jul 29, 2024
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions build.assets/macos/tshdev/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,15 @@ launch the daemon with the following error:

After resetting the db and restarting the device, everything seemed to be working again.

In theory, it's possible to list all app bundles with a certain bundle identifier by running the
following command:

```
mdfind kMDItemCFBundleIdentifier = "com.goteleport.tshdev"
```

In practice, getting rid of all but one bundle didn't appear to solve the problem.

### Daemon does not start

List all jobs loaded into launchd. The second column is the status which you can then inspect.
Expand Down
35 changes: 23 additions & 12 deletions lib/vnet/daemon/client_darwin.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,9 @@

package daemon

// #cgo CFLAGS: -Wall -xobjective-c -fblocks -fobjc-arc -mmacosx-version-min=10.15
// #cgo LDFLAGS: -framework Foundation -framework ServiceManagement
// #include <stdlib.h>
// #include "client_darwin.h"
// #include "protocol_darwin.h"
// #include "common_darwin.h"
import "C"

import (
Expand Down Expand Up @@ -304,6 +302,28 @@ func startByCalling(ctx context.Context, bundlePath string, config Config) error
return
}

if errorDomain == nsCocoaErrorDomain && errorCode == errorCodeNSXPCConnectionInterrupted {
const clientNSXPCConnectionInterruptedDebugMsg = "The connection was interrupted when trying to " +
"reach the XPC service. If there's no clear error logs on the daemon side, it might mean that " +
"the client does not satisfy the code signing requirement enforced by the daemon. " +
"Start capturing logs in Console.app and repeat the scenario. Look for " +
"\"xpc_support_check_token: <private> error: <private> status: -67050\" in the logs to verify " +
"that the connection was interrupted due to the code signing requirement."
log.DebugContext(ctx, clientNSXPCConnectionInterruptedDebugMsg)
ravicious marked this conversation as resolved.
Show resolved Hide resolved
errC <- trace.Wrap(errXPCConnectionInterrupted)
return
}

if errorDomain == vnetErrorDomain && errorCode == errorCodeMissingCodeSigningIdentifiers {
errC <- trace.Wrap(errMissingCodeSigningIdentifiers)
return
}

if errorDomain == nsCocoaErrorDomain && errorCode == errorCodeNSXPCConnectionCodeSigningRequirementFailure {
errC <- trace.Wrap(errXPCConnectionCodeSigningRequirementFailure, "the daemon does not appear to be code signed correctly")
return
}

errC <- trace.Errorf("could not start VNet daemon: %v", C.GoString(res.error_description))
return
}
Expand All @@ -319,15 +339,6 @@ func startByCalling(ctx context.Context, bundlePath string, config Config) error
}
}

var (
// vnetErrorDomain is a custom error domain used for Objective-C errors that pertain to VNet.
vnetErrorDomain = C.GoString(C.VNEErrorDomain)
// errorCodeAlreadyRunning is returned within [vnetErrorDomain] errors to indicate that the daemon
// received a message to start after it was already running.
errorCodeAlreadyRunning = int(C.VNEAlreadyRunningError)
errAlreadyRunning = errors.New("VNet is already running")
)

func sleepOrDone(ctx context.Context, d time.Duration) error {
timer := time.NewTimer(d)
defer timer.Stop()
Expand Down
10 changes: 7 additions & 3 deletions lib/vnet/daemon/client_darwin.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
#define TELEPORT_LIB_VNET_DAEMON_CLIENT_DARWIN_H_

#include "common_darwin.h"
#include "protocol_darwin.h"

#import <Foundation/Foundation.h>

Expand Down Expand Up @@ -44,10 +43,15 @@ typedef struct StartVnetRequest {

typedef struct StartVnetResult {
bool ok;
// error_domain is either VNEErrorDomain, NSOSStatusErrorDomain, or NSCocoaErrorDomain.
const char *error_domain;
// error_code is code taken from an NSError instance encountered during the call to StartVnet.
// If ok is false, error_code is greater than zero and identifies the reason behind the error.
// If error_domain is set to VNEErrorDomain, error_code is one of the VNE codes from common_darwin.h.
// If error_domain is NSOSStatusErrorDomain, error_code comes from OSStatus of Code Signing framework.
// https://developer.apple.com/documentation/security/1574088-code_signing_services_result_cod?language=objc
// If error_domain is NSCocoaErrorDomain, it's likely to be about XPC. It's best to inspect it
// on https://osstatus.com in that case.
int error_code;
// error_description includes the full representation of the error, including domain and code.
const char *error_description;
} StartVnetResult;

Expand Down
25 changes: 22 additions & 3 deletions lib/vnet/daemon/client_darwin.m
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

#include "client_darwin.h"
#include "common_darwin.h"
#include "protocol_darwin.h"

#import <Foundation/Foundation.h>
#import <ServiceManagement/ServiceManagement.h>
Expand Down Expand Up @@ -78,15 +77,17 @@ @interface VNEDaemonClient ()

@property(nonatomic, strong, readwrite) NSXPCConnection *connection;
@property(nonatomic, strong, readonly) NSString *bundlePath;
@property(nonatomic, strong, readonly) NSString *codeSigningRequirement;

@end

@implementation VNEDaemonClient

- (id)initWithBundlePath:(NSString *)bundlePath {
- (id)initWithBundlePath:(NSString *)bundlePath codeSigningRequirement:(NSString *)codeSigningRequirement {
self = [super init];
if (self) {
_bundlePath = bundlePath;
_codeSigningRequirement = codeSigningRequirement;
}
return self;
}
Expand All @@ -102,6 +103,12 @@ - (NSXPCConnection *)connection {
self->_connection = nil;
};

// The daemon won't even be started on macOS < 13.0, so we don't have to handle the else branch
// of this condition.
if (@available(macOS 13, *)) {
[_connection setCodeSigningRequirement:_codeSigningRequirement];
}

// New connections always start in a suspended state.
[_connection resume];
}
Expand Down Expand Up @@ -134,14 +141,26 @@ - (void)invalidate {

void StartVnet(StartVnetRequest *request, StartVnetResult *outResult) {
if (!daemonClient) {
daemonClient = [[VNEDaemonClient alloc] initWithBundlePath:@(request->bundle_path)];
NSString *requirement = nil;
NSError *error = nil;
bool ok = getCodeSigningRequirement(&requirement, &error);
if (!ok) {
outResult->ok = false;
outResult->error_domain = VNECopyNSString([error domain]);
outResult->error_code = (int)[error code];
outResult->error_description = VNECopyNSString([error description]);
return;
}

daemonClient = [[VNEDaemonClient alloc] initWithBundlePath:@(request->bundle_path) codeSigningRequirement:requirement];
}

dispatch_semaphore_t sema = dispatch_semaphore_create(0);

[daemonClient startVnet:request->vnet_config
completion:^(NSError *error) {
if (error) {
outResult->ok = false;
outResult->error_domain = VNECopyNSString([error domain]);
outResult->error_code = (int)[error code];
outResult->error_description = VNECopyNSString([error description]);
Expand Down
57 changes: 57 additions & 0 deletions lib/vnet/daemon/common_darwin.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
// Teleport
// Copyright (C) 2024 Gravitational, Inc.
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU Affero General Public License as published by
// the Free Software Foundation, either version 3 of the License, or
// (at your option) any later version.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU Affero General Public License for more details.
//
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.

//go:build vnetdaemon
// +build vnetdaemon

package daemon

// #cgo CFLAGS: -Wall -xobjective-c -fblocks -fobjc-arc -mmacosx-version-min=10.15
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've run into problems before with multiple #cgo declarations in the same package. You might want to have all import "C" related stuff in a single file. Just a word of caution.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed #cgo declarations from all files but common_darwin.go. bc3e9f5

// #cgo LDFLAGS: -framework Foundation -framework ServiceManagement
// #include "common_darwin.h"
import "C"

import (
"errors"
)

var (
// vnetErrorDomain is a custom error domain used for Objective-C errors that pertain to VNet.
vnetErrorDomain = C.GoString(C.VNEErrorDomain)

// errorCodeAlreadyRunning is returned within [vnetErrorDomain] errors to indicate that the daemon
// received a message to start after it was already running.
errorCodeAlreadyRunning = int(C.VNEAlreadyRunningError)
errAlreadyRunning = errors.New("VNet is already running")

// errorCodeMissingCodeSigningIdentifiers is returned within [vnetErrorDomain] Obj-C errors and
// transformed to [errMissingCodeSigningIdentifiers] in Go.
errorCodeMissingCodeSigningIdentifiers = int(C.VNEMissingCodeSigningIdentifiersError)
errMissingCodeSigningIdentifiers = errors.New("either identifier or team identifier is missing in code signing information; is the binary signed?")
)

var (
// nsCocoaErrorDomain is a generic error domain used in a lot of Apple's Cocoa frameworks.
nsCocoaErrorDomain = "NSCocoaErrorDomain"

// https://developer.apple.com/documentation/foundation/1448136-nserror_codes/nsxpcconnectioninterrupted?changes=latest_major&language=objc
errorCodeNSXPCConnectionInterrupted = int(C.NSXPCConnectionInterrupted)
errXPCConnectionInterrupted = errors.New("XPC connection interrupted")

// https://developer.apple.com/documentation/foundation/1448136-nserror_codes/nsxpcconnectioncodesigningrequirementfailure?language=objc
errorCodeNSXPCConnectionCodeSigningRequirementFailure = int(C.NSXPCConnectionCodeSigningRequirementFailure)
errXPCConnectionCodeSigningRequirementFailure = errors.New("code signing requirement failed")
)
40 changes: 40 additions & 0 deletions lib/vnet/daemon/common_darwin.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,35 @@

#import <Foundation/Foundation.h>

// VNEErrorDomain is a custom error domain used for Objective-C errors that pertain to VNet.
extern const char* const VNEErrorDomain;

// VNEAlreadyRunningError indicates that the daemon already received a VNet config.
// It won't accept a new one during its lifetime, instead it's expected to stop, after
// which the client might spawn a new instance of the daemon.
extern const int VNEAlreadyRunningError;
// VNEMissingCodeSigningIdentifiersError indicates that either the identifier or the team identifier are missing.
// This can happen if the binary is unsigned, see the docs for SecCodeCopySigningInformation.
// https://developer.apple.com/documentation/security/1395809-seccodecopysigninginformation?language=objc
extern const int VNEMissingCodeSigningIdentifiersError;

typedef struct VnetConfig {
const char *socket_path;
const char *ipv6_prefix;
const char *dns_addr;
const char *home_path;
} VnetConfig;

@protocol VNEDaemonProtocol
// startVnet passes the config back to Go code (which then starts VNet in a separate thread)
// and returns immediately.
//
// Only the first call to this method starts VNet. Subsequent calls return VNEAlreadyRunningError.
// The daemon process exits after VNet is stopped, after which it can be spawned again by calling
// this method.
- (void)startVnet:(VnetConfig *)vnetConfig completion:(void (^)(NSError *error))completion;
@end

// Returns the label for the daemon by getting the identifier of the bundle
// this executable is shipped in and appending ".vnetd" to it.
//
Expand All @@ -16,4 +45,15 @@ NSString *DaemonLabel(NSString *bundlePath);
// The caller is expected to free the returned pointer.
const char *VNECopyNSString(NSString *val);

// getCodeSigningRequirement calculates the requirement that will be matched against
// the designated requirement of the app on the other side of an XPC connection.
// It does so based on the code signing information of the current binary, as it assumes that
// both the VNet client and the VNet daemon use the same binary.
//
// On success, it returns true and sets outRequirement.
// On error, it returns false and sets outError. Returns errors of VNEErrorDomain and
// NSOSStatusErrorDomain. Errors with the latter domain are likely to match Code Signing OSStatus values.
// https://developer.apple.com/documentation/security/1574088-code_signing_services_result_cod?language=objc
bool getCodeSigningRequirement(NSString **outRequirement, NSError **outError);

#endif /* TELEPORT_LIB_VNET_DAEMON_COMMON_DARWIN_H_ */
77 changes: 77 additions & 0 deletions lib/vnet/daemon/common_darwin.m
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,17 @@
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.

#import <CoreFoundation/CoreFoundation.h>
#import <Foundation/Foundation.h>
#import <Security/CodeSigning.h>

#include <string.h>

const char* const VNEErrorDomain = "com.Gravitational.Vnet.ErrorDomain";

const int VNEAlreadyRunningError = 1;
const int VNEMissingCodeSigningIdentifiersError = 2;

NSString *DaemonLabel(NSString *bundlePath) {
NSBundle *main = [NSBundle bundleWithPath:bundlePath];
if (!main) {
Expand All @@ -41,3 +48,73 @@
}
return strdup("");
}

bool getCodeSigningRequirement(NSString **outRequirement, NSError **outError) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a C function working on Obj-C objects, which I think is fine.

From what I understand, the pattern with passing stuff by reference is common when you want to "return" more than one thing. This is especially common when it comes to NSError. https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/ProgrammingWithObjectiveC/ErrorHandling/ErrorHandling.html#//apple_ref/doc/uid/TP40011210-CH9-SW5

SecCodeRef codeObj = nil;
OSStatus status = SecCodeCopySelf(kSecCSDefaultFlags, &codeObj);
if (status != errSecSuccess) {
if (outError) {
*outError = [NSError errorWithDomain:NSOSStatusErrorDomain code:status userInfo:nil];
}
return false;
}

CFDictionaryRef cfCodeSignInfo = nil;
// kSecCSSigningInformation must be provided as a flag for the team identifier to be included
// in the returned dictionary.
status = SecCodeCopySigningInformation(codeObj, kSecCSSigningInformation, &cfCodeSignInfo);
// codeObj is no longer needed. Manually release it, as we own it since we got it from a function
// with "Copy" in its name.
// https://developer.apple.com/library/archive/documentation/CoreFoundation/Conceptual/CFMemoryMgmt/Concepts/Ownership.html#//apple_ref/doc/writerid/cfCreateRule
CFRelease(codeObj);
if (status != errSecSuccess) {
if (outError) {
*outError = [NSError errorWithDomain:NSOSStatusErrorDomain code:status userInfo:nil];
}
return false;
}

// Transfer ownership of cfCodeSignInfo to Obj-C, which means we don't have to CFRelease it manually.
// We can transfer the ownership of cfCodeSignInfo because we own it (we got it from a function
// with "Copy" in its name).
// https://developer.apple.com/documentation/foundation/1587932-cfbridgingrelease
NSDictionary *codeSignInfo = (NSDictionary *)CFBridgingRelease(cfCodeSignInfo);
// We don't own kSecCodeInfoIdentifier, so we cannot call CFBridgingRelease on it.
// __bridge transfers a pointer between Obj-C and CoreFoundation with no transfer of ownership.
// Values extracted out of codeSignInfo are cast to toll-free bridged Obj-C types.
// https://developer.apple.com/library/archive/documentation/CoreFoundation/Conceptual/CFDesignConcepts/Articles/tollFreeBridgedTypes.html#//apple_ref/doc/uid/TP40010677-SW2
// https://stackoverflow.com/questions/18067108/when-should-you-use-bridge-vs-cfbridgingrelease-cfbridgingretain
NSString *identifier = codeSignInfo[(__bridge NSString *)kSecCodeInfoIdentifier];
NSString *teamIdentifier = codeSignInfo[(__bridge NSString *)kSecCodeInfoTeamIdentifier];

if (!identifier || [identifier length] == 0 || !teamIdentifier || [teamIdentifier length] == 0) {
if (outError) {
*outError = [NSError errorWithDomain:@(VNEErrorDomain) code:VNEMissingCodeSigningIdentifiersError userInfo:nil];
}
return false;
}

// The requirement will be matched against the designated requirement of the application on
// the other side of an XPC connection. It is based on the designated requirement of tsh.app.
// To inspect the designated requirement of an app, use the following command:
//
// codesign --display -r - <path to app>
//
// Breakdown of individual parts of the requirement:
// * `identifier "foo"` is satisfied if the code signing identifier matches the provided one.
// It is not the same as the bundle identifier.
// * `anchor apple generic` is satisfied by any code signed with any code signing identity issued
// by Apple.
// * `certificate leaf[field(bunch of specific numbers)]` is satisfied by code signed with
// Developer ID Application certs.
// * `certificate leaf[subject.OU]` is satisfied by certs with a specific Team ID.
//
// Read more at:
// https://developer.apple.com/documentation/technotes/tn3127-inside-code-signing-requirements#Designated-requirement
// https://developer.apple.com/documentation/technotes/tn3127-inside-code-signing-requirements#Xcode-designated-requirement-for-Developer-ID-code
if (outRequirement) {
*outRequirement = [NSString stringWithFormat:@"identifier \"%@\" and anchor apple generic and certificate leaf[field.1.2.840.113635.100.6.1.13] and certificate leaf[subject.OU] = %@", identifier, teamIdentifier];
}

return true;
}
Loading
Loading