VNet: Use NSObject instead of C struct in XPC message

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() {

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;

lib/vnet/daemon/client_darwin.m

@@ -73,15 +73,11 @@ void OpenSystemSettingsLoginItems(void) {
   }
 }
 
-@interface VNEDaemonClient ()
-
-@property(nonatomic, strong, readwrite) NSXPCConnection *connection;
-@property(nonatomic, strong, readonly) NSString *bundlePath;
-@property(nonatomic, strong, readonly) NSString *codeSigningRequirement;
-
-@end
-
-@implementation VNEDaemonClient
+@implementation VNEDaemonClient {
+  NSXPCConnection *_connection;
+  NSString *_bundlePath;
+  NSString *_codeSigningRequirement;
+}
 
 - (id)initWithBundlePath:(NSString *)bundlePath codeSigningRequirement:(NSString *)codeSigningRequirement {
   self = [super init];
@@ -115,18 +111,17 @@ - (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
   id proxy = [self.connection remoteObjectProxyWithErrorHandler:^(NSError *error) {
     completion(error);
   }];
 
-  [(id<VNEDaemonProtocol>)proxy startVnet:vnetConfig
-                               completion:^(NSError *error) {
-                                 completion(error);
-                               }];
+  [(id<VNEDaemonProtocol>)proxy startVnet:config completion:^(NSError *error) {
+    completion(error);
+  }];
 }
 
 - (void)invalidate {
@@ -155,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;

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 <NSSecureCoding>
+@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

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 <http://www.gnu.org/licenses/>.
+#include "common_darwin.h"
 
 #import <CoreFoundation/CoreFoundation.h>
 #import <Foundation/Foundation.h>
@@ -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

lib/vnet/daemon/service_darwin.m

@@ -26,38 +26,27 @@
 #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;
+@property BOOL valid;
+@property gid_t egid;
+@property uid_t euid;
 @end
 
 @implementation VNEClientCred
-@synthesize valid,egid,euid;
 @end
 
 @interface VNEDaemonService () <NSXPCListenerDelegate, VNEDaemonProtocol>
 
-@property(nonatomic, strong, readwrite) NSXPCListener *listener;
 // started describes whether the XPC listener is listening for new connections.
-@property(nonatomic, readwrite) BOOL started;
-// gotConfig describes if the daemon received a VNet config from a client.
-@property(nonatomic, readwrite) BOOL gotConfig;
-
-@property(nonatomic, readwrite) NSString *socketPath;
-@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;
+@property(readonly) BOOL started;
+@property(readonly) VNEConfig *config;
+@property(readonly) VNEClientCred *clientCred;
 
 @end
 
-@implementation VNEDaemonService
+@implementation VNEDaemonService {
+  NSXPCListener *_listener;
+  dispatch_semaphore_t _gotVnetConfigSema;
+}
 
 - (id)initWithBundlePath:(NSString *)bundlePath codeSigningRequirement:(NSString *)codeSigningRequirement {
   self = [super init];
@@ -99,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
@@ -108,19 +97,15 @@ - (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];
       completion(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];
@@ -175,7 +160,7 @@ void DaemonStart(const char *bundle_path, DaemonStartResult *outResult) {
 }
 
 void DaemonStop(void) {
-  if (daemonService && [daemonService started]) {
+  if (daemonService && daemonService.started) {
     [daemonService stop];
   }
 }
@@ -186,26 +171,26 @@ void WaitForVnetConfig(VnetConfigResult *outResult, ClientCred *outClientCred) {
     return;
   }
 
-  if (![daemonService started]) {
+  if (!daemonService.started) {
     outResult->error_description = strdup("daemon was not started yet");
   }
 
   [daemonService waitForVnetConfig];
 
-  if (![daemonService started]) {
+  if (!daemonService.started) {
     outResult->error_description = strdup("daemon was stopped while waiting for VNet config");
     return;
   }
 
   @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]);
-
-    if ([daemonService clientCred] && [[daemonService clientCred] valid]) {
-      outClientCred->egid = [[daemonService clientCred] egid];
-      outClientCred->euid = [[daemonService clientCred] euid];
+    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->valid = true;
     }