From b03f33cca169cbd457c5c20f62f4321202d5fea8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rafa=C5=82=20Cie=C5=9Blak?= Date: Wed, 31 Jul 2024 12:38:43 +0200 Subject: [PATCH] Use NSObject instead of C struct in XPC message --- lib/vnet/daemon/client_darwin.go | 25 ++++++++----------------- lib/vnet/daemon/client_darwin.h | 8 ++++++-- lib/vnet/daemon/client_darwin.m | 17 +++++++++++------ lib/vnet/daemon/common_darwin.h | 30 +++++++++++++++++++++++------- lib/vnet/daemon/common_darwin.m | 25 +++++++++++++++++++++++++ lib/vnet/daemon/service_darwin.m | 30 ++++++++++-------------------- 6 files changed, 83 insertions(+), 52 deletions(-) diff --git a/lib/vnet/daemon/client_darwin.go b/lib/vnet/daemon/client_darwin.go index 56775166e8d95..3b531604bcbc9 100644 --- a/lib/vnet/daemon/client_darwin.go +++ b/lib/vnet/daemon/client_darwin.go @@ -30,7 +30,6 @@ import ( "fmt" "os" "path/filepath" - "runtime" "strconv" "strings" "time" @@ -261,28 +260,20 @@ func startByCalling(ctx context.Context, bundlePath string, config Config) error errC := make(chan error, 1) go func() { - var pinner runtime.Pinner - defer pinner.Unpin() - req := C.StartVnetRequest{ bundle_path: C.CString(bundlePath), - vnet_config: &C.VnetConfig{ - socket_path: C.CString(config.SocketPath), - ipv6_prefix: C.CString(config.IPv6Prefix), - dns_addr: C.CString(config.DNSAddr), - home_path: C.CString(config.HomePath), - }, + socket_path: C.CString(config.SocketPath), + ipv6_prefix: C.CString(config.IPv6Prefix), + dns_addr: C.CString(config.DNSAddr), + home_path: C.CString(config.HomePath), } defer func() { C.free(unsafe.Pointer(req.bundle_path)) - C.free(unsafe.Pointer(req.vnet_config.socket_path)) - C.free(unsafe.Pointer(req.vnet_config.ipv6_prefix)) - C.free(unsafe.Pointer(req.vnet_config.dns_addr)) - C.free(unsafe.Pointer(req.vnet_config.home_path)) + C.free(unsafe.Pointer(req.socket_path)) + C.free(unsafe.Pointer(req.ipv6_prefix)) + C.free(unsafe.Pointer(req.dns_addr)) + C.free(unsafe.Pointer(req.home_path)) }() - // Structs passed directly as arguments to cgo functions are automatically pinned. - // However, structs within structs have to be pinned by hand. - pinner.Pin(req.vnet_config) var res C.StartVnetResult defer func() { diff --git a/lib/vnet/daemon/client_darwin.h b/lib/vnet/daemon/client_darwin.h index e362c4add527b..5afa0dffe7db5 100644 --- a/lib/vnet/daemon/client_darwin.h +++ b/lib/vnet/daemon/client_darwin.h @@ -38,7 +38,11 @@ void OpenSystemSettingsLoginItems(void); typedef struct StartVnetRequest { const char *bundle_path; - VnetConfig *vnet_config; + + const char *socket_path; + const char *ipv6_prefix; + const char *dns_addr; + const char *home_path; } StartVnetRequest; typedef struct StartVnetResult { @@ -73,7 +77,7 @@ void StartVnet(StartVnetRequest *request, StartVnetResult *outResult); void InvalidateDaemonClient(void); @interface VNEDaemonClient : NSObject -- (void)startVnet:(VnetConfig *)vnetConfig completion:(void (^)(NSError *error))completion; +- (void)startVnet:(VNEConfig *)config completion:(void (^)(NSError *error))completion; // invalidate executes all outstanding reply blocks, error handling blocks, // and invalidation blocks and forbids from sending or receiving new messages. - (void)invalidate; diff --git a/lib/vnet/daemon/client_darwin.m b/lib/vnet/daemon/client_darwin.m index 8d2b32afdbb56..3d5d50226570f 100644 --- a/lib/vnet/daemon/client_darwin.m +++ b/lib/vnet/daemon/client_darwin.m @@ -111,7 +111,7 @@ - (NSXPCConnection *)connection { return _connection; } -- (void)startVnet:(VnetConfig *)vnetConfig completion:(void (^)(NSError *))completion { +- (void)startVnet:(VNEConfig *)config completion:(void (^)(NSError *))completion { // This way of calling the XPC proxy ensures either the error handler or // the reply block gets called. // https://forums.developer.apple.com/forums/thread/713429 @@ -119,10 +119,9 @@ - (void)startVnet:(VnetConfig *)vnetConfig completion:(void (^)(NSError *))compl completion(error); }]; - [(id)proxy startVnet:vnetConfig - completion:^(NSError *error) { - completion(error); - }]; + [(id)proxy startVnet:config completion:^(NSError *error) { + completion(error); + }]; } - (void)invalidate { @@ -151,9 +150,15 @@ void StartVnet(StartVnetRequest *request, StartVnetResult *outResult) { daemonClient = [[VNEDaemonClient alloc] initWithBundlePath:@(request->bundle_path) codeSigningRequirement:requirement]; } + VNEConfig *config = [[VNEConfig alloc] init]; + [config setSocketPath:@(request->socket_path)]; + [config setIpv6Prefix:@(request->ipv6_prefix)]; + [config setDnsAddr:@(request->dns_addr)]; + [config setHomePath:@(request->home_path)]; + dispatch_semaphore_t sema = dispatch_semaphore_create(0); - [daemonClient startVnet:request->vnet_config + [daemonClient startVnet:config completion:^(NSError *error) { if (error) { outResult->ok = false; diff --git a/lib/vnet/daemon/common_darwin.h b/lib/vnet/daemon/common_darwin.h index 92c73b310f4b3..b8491fc5a81d2 100644 --- a/lib/vnet/daemon/common_darwin.h +++ b/lib/vnet/daemon/common_darwin.h @@ -15,12 +15,28 @@ extern const int VNEAlreadyRunningError; // 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; +// VNEConfig is used to send a config necessary to start VNet between the client and the daemon +// service. When adding or removing properties, remember to adjust the implementation of VNEConfig +// as well. +// +// Although it's not the primary use case, it's possible for the client to connect to a service +// of a different version of tsh where VNEConfig does not have the same properties. +// Thanks to the conformance to NSSecureCoding, adding and removing properties does not cause either +// end of the connection to blow up: +// +// * If the client sends a property that the daemon doesn't know about, the property will be ignored +// on the daemon side. +// * If the client does not send a property that the daemon expects, the property will not be set on +// the daemon side. +// +// In either case, the expectation is that the Obj-C side pushes the config to the Go side which +// actually validates the config. +@interface VNEConfig : NSObject +@property(copy) NSString *socketPath; +@property(copy) NSString *ipv6Prefix; +@property(copy) NSString *dnsAddr; +@property(copy) NSString *homePath; +@end @protocol VNEDaemonProtocol // startVnet passes the config back to Go code (which then starts VNet in a separate thread) @@ -29,7 +45,7 @@ typedef struct VnetConfig { // 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; +- (void)startVnet:(VNEConfig *)vnetConfig completion:(void (^)(NSError *error))completion; @end // Returns the label for the daemon by getting the identifier of the bundle diff --git a/lib/vnet/daemon/common_darwin.m b/lib/vnet/daemon/common_darwin.m index 3a405209893f8..ca9eb1d96b048 100644 --- a/lib/vnet/daemon/common_darwin.m +++ b/lib/vnet/daemon/common_darwin.m @@ -16,6 +16,7 @@ // // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +#include "common_darwin.h" #import #import @@ -118,3 +119,27 @@ bool getCodeSigningRequirement(NSString **outRequirement, NSError **outError) { return true; } + +@implementation VNEConfig ++ (BOOL)supportsSecureCoding { + return YES; +} + +- (void)encodeWithCoder:(nonnull NSCoder *)coder { + [coder encodeObject:self.socketPath forKey:@"socketPath"]; + [coder encodeObject:self.ipv6Prefix forKey:@"ipv6Prefix"]; + [coder encodeObject:self.dnsAddr forKey:@"dnsAddr"]; + [coder encodeObject:self.homePath forKey:@"homePath"]; +} + +- (nullable instancetype)initWithCoder:(nonnull NSCoder *)coder { + if (self = [super init]) { + [self setSocketPath:[coder decodeObjectOfClass:[NSString class] forKey:@"socketPath"]]; + [self setIpv6Prefix:[coder decodeObjectOfClass:[NSString class] forKey:@"ipv6Prefix"]]; + [self setDnsAddr:[coder decodeObjectOfClass:[NSString class] forKey:@"dnsAddr"]]; + [self setHomePath:[coder decodeObjectOfClass:[NSString class] forKey:@"homePath"]]; + } + return self; +} + +@end diff --git a/lib/vnet/daemon/service_darwin.m b/lib/vnet/daemon/service_darwin.m index c2d5fbabc9e50..10d586535adf7 100644 --- a/lib/vnet/daemon/service_darwin.m +++ b/lib/vnet/daemon/service_darwin.m @@ -38,19 +38,13 @@ @interface VNEDaemonService () // started describes whether the XPC listener is listening for new connections. @property(readonly) BOOL started; - -@property(readonly) NSString *socketPath; -@property(readonly) NSString *ipv6Prefix; -@property(readonly) NSString *dnsAddr; -@property(readonly) NSString *homePath; +@property(readonly) VNEConfig *config; @property(readonly) VNEClientCred *clientCred; @end @implementation VNEDaemonService { NSXPCListener *_listener; - // gotConfig describes if the daemon received a VNet config from a client. - BOOL _gotConfig; dispatch_semaphore_t _gotVnetConfigSema; } @@ -94,7 +88,7 @@ - (void)waitForVnetConfig { #pragma mark - VNEDaemonProtocol -- (void)startVnet:(VnetConfig *)vnetConfig completion:(void (^)(NSError *error))completion { +- (void)startVnet:(VNEConfig *)config completion:(void (^)(NSError *error))completion { @synchronized(self) { // startVnet is expected to be called only once per daemon's lifetime. // Between the process with the daemon client exiting and the admin process (which runs the @@ -103,7 +97,7 @@ - (void)startVnet:(VnetConfig *)vnetConfig completion:(void (^)(NSError *error)) // // In such scenarios, we want to return an error so that the client can wait for the daemon // to exit and retry the call. - if (_gotConfig) { + if (_config != nil) { NSError *error = [[NSError alloc] initWithDomain:@(VNEErrorDomain) code:VNEAlreadyRunningError userInfo:nil]; @@ -111,11 +105,7 @@ - (void)startVnet:(VnetConfig *)vnetConfig completion:(void (^)(NSError *error)) return; } - _gotConfig = YES; - _socketPath = @(vnetConfig->socket_path); - _ipv6Prefix = @(vnetConfig->ipv6_prefix); - _dnsAddr = @(vnetConfig->dns_addr); - _homePath = @(vnetConfig->home_path); + _config = config; NSXPCConnection *currentConn = [NSXPCConnection currentConnection]; _clientCred = [[VNEClientCred alloc] init]; @@ -193,14 +183,14 @@ void WaitForVnetConfig(VnetConfigResult *outResult, ClientCred *outClientCred) { } @synchronized(daemonService) { - outResult->socket_path = VNECopyNSString([daemonService socketPath]); - outResult->ipv6_prefix = VNECopyNSString([daemonService ipv6Prefix]); - outResult->dns_addr = VNECopyNSString([daemonService dnsAddr]); - outResult->home_path = VNECopyNSString([daemonService homePath]); + outResult->socket_path = VNECopyNSString(daemonService.config.socketPath); + outResult->ipv6_prefix = VNECopyNSString(daemonService.config.ipv6Prefix); + outResult->dns_addr = VNECopyNSString(daemonService.config.dnsAddr); + outResult->home_path = VNECopyNSString(daemonService.config.homePath); if (daemonService.clientCred && [daemonService.clientCred valid]) { - outClientCred->egid = [daemonService.clientCred egid]; - outClientCred->euid = [daemonService.clientCred euid]; + outClientCred->egid = daemonService.clientCred.egid; + outClientCred->euid = daemonService.clientCred.euid; outClientCred->valid = true; }