Merge branch 'retry-named-pipe-ownership-check'

4 files changed, 49 insertions, 42 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9cfb353c35..07e9588f11 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -49,6 +49,9 @@ Line wrap the file at 100 chars.                                              Th
 - Add grace period when best default route goes away to reduce frequency of random reconnects.
 
 ### Security
+- Prevent unprivileged users from impersonating the gRPC server. This was relatively harmless
+  previously but is required due to in-app updates.
+
 #### Windows
 - Enable control flow integrity checks (CFG) for some C++ code. This excludes `wintun`,
   `wireguard-nt`, and OpenVPN. This addresses `MLLVD-CR-24-101` to the extent that we found
diff --git a/desktop/packages/mullvad-vpn/src/main/daemon-rpc.ts b/desktop/packages/mullvad-vpn/src/main/daemon-rpc.ts
index 98d941cd81..ab76aba907 100644
--- a/desktop/packages/mullvad-vpn/src/main/daemon-rpc.ts
+++ b/desktop/packages/mullvad-vpn/src/main/daemon-rpc.ts
@@ -1,5 +1,4 @@
 import * as grpc from '@grpc/grpc-js';
-import fs from 'fs';
 import { Empty } from 'google-protobuf/google/protobuf/empty_pb.js';
 import { BoolValue, StringValue } from 'google-protobuf/google/protobuf/wrappers_pb.js';
 import { types as grpcTypes } from 'management-interface';
@@ -54,8 +53,6 @@ import {
 
 const DAEMON_RPC_PATH =
   process.platform === 'win32' ? '//./pipe/Mullvad VPN' : '/var/run/mullvad-vpn';
-const DAEMON_RPC_PATH_PREFIX = 'unix://';
-const DAEMON_RPC_PATH_PREFIXED = `${DAEMON_RPC_PATH_PREFIX}${DAEMON_RPC_PATH}`;
 
 export class SubscriptionListener<T> {
   // Only meant to be used by DaemonRpc
@@ -88,7 +85,7 @@ export class DaemonRpc extends GrpcClient {
   > = new Map();
 
   public constructor(connectionObserver?: ConnectionObserver) {
-    super(DAEMON_RPC_PATH_PREFIXED, connectionObserver);
+    super(DAEMON_RPC_PATH, connectionObserver);
   }
 
   public disconnect() {
@@ -99,26 +96,6 @@ export class DaemonRpc extends GrpcClient {
     super.disconnect();
   }
 
-  public async verifyDaemonOwnership() {
-    if (process.platform === 'win32') {
-      try {
-        const { pipeIsAdminOwned } = await import('windows-utils');
-        pipeIsAdminOwned(DAEMON_RPC_PATH);
-      } catch (e) {
-        if (e && typeof e === 'object' && 'message' in e) {
-          throw new Error(`Failed to verify admin ownership of named pipe. ${e.message}`);
-        } else {
-          throw new Error('Failed to verify admin ownership of named pipe');
-        }
-      }
-    } else {
-      const stat = fs.statSync(DAEMON_RPC_PATH);
-      if (stat.uid !== 0) {
-        throw new Error('Failed to verify root ownership of socket');
-      }
-    }
-  }
-
   public subscribeAppUpgradeEventListener(listener: SubscriptionListener<DaemonAppUpgradeEvent>) {
     const call = this.isConnected && this.client.appUpgradeEventsListen(new Empty());
     if (!call) {
diff --git a/desktop/packages/mullvad-vpn/src/main/grpc-client.ts b/desktop/packages/mullvad-vpn/src/main/grpc-client.ts
index 1ef5a8e831..702bc519c1 100644
--- a/desktop/packages/mullvad-vpn/src/main/grpc-client.ts
+++ b/desktop/packages/mullvad-vpn/src/main/grpc-client.ts
@@ -1,4 +1,5 @@
 import * as grpc from '@grpc/grpc-js';
+import fs from 'fs';
 import { Empty } from 'google-protobuf/google/protobuf/empty_pb.js';
 import {
   BoolValue,
@@ -13,6 +14,8 @@ import log from '../shared/logging';
 const NETWORK_CALL_TIMEOUT = 10000;
 const CHANNEL_STATE_TIMEOUT = 1000 * 60 * 60;
 
+const RPC_PATH_PREFIX = 'unix://';
+
 type CallFunctionArgument<T, R> =
   | ((arg: T, callback: (error: Error | null, result: R) => void) => void)
   | undefined;
@@ -49,7 +52,7 @@ export class GrpcClient {
     private connectionObserver?: ConnectionObserver,
   ) {
     this.client = new ManagementServiceClient(
-      rpcPath,
+      this.prefixedRpcPath(),
       grpc.credentials.createInsecure(),
       this.channelOptions(),
     );
@@ -63,7 +66,7 @@ export class GrpcClient {
     if (this.isClosed) {
       this.isClosed = false;
       this.client = new ManagementServiceClient(
-        this.rpcPath,
+        this.prefixedRpcPath(),
         grpc.credentials.createInsecure(),
         this.channelOptions(),
       );
@@ -86,11 +89,19 @@ export class GrpcClient {
           this.ensureConnectivity();
           reject(error);
         } else {
-          this.reconnectionTimeout = undefined;
-          this.isConnectedValue = true;
-          this.connectionObserver?.onOpen();
-          this.setChannelCallback();
-          resolve();
+          this.verifyOwnership()
+            .then(() => {
+              this.reconnectionTimeout = undefined;
+              this.isConnectedValue = true;
+              this.connectionObserver?.onOpen();
+              this.setChannelCallback();
+              resolve();
+            })
+            .catch((error) => {
+              this.onClose(error);
+              this.ensureConnectivity();
+              reject(error);
+            });
         }
       });
     });
@@ -152,6 +163,10 @@ export class GrpcClient {
     }
   }
 
+  private prefixedRpcPath(): string {
+    return `${RPC_PATH_PREFIX}${this.rpcPath}`;
+  }
+
   private deadlineFromNow() {
     return Date.now() + NETWORK_CALL_TIMEOUT;
   }
@@ -243,4 +258,27 @@ export class GrpcClient {
       }
     }, 3000);
   }
+
+  // Assert that the gRPC connection is owned by an administrator
+  private async verifyOwnership() {
+    if (process.platform === 'win32') {
+      try {
+        const { pipeIsAdminOwned } = await import('windows-utils');
+        pipeIsAdminOwned(this.rpcPath);
+      } catch (e) {
+        if (e && typeof e === 'object' && 'message' in e) {
+          throw new Error(`Failed to verify admin ownership of named pipe. ${e.message}`);
+        } else {
+          throw new Error('Failed to verify admin ownership of named pipe');
+        }
+      }
+      log.info('Verified pipe ownership');
+    } else {
+      const stat = fs.statSync(this.rpcPath);
+      if (stat.uid !== 0) {
+        throw new Error('Failed to verify root ownership of socket');
+      }
+      log.info('Verified socket ownership');
+    }
+  }
 }
diff --git a/desktop/packages/mullvad-vpn/src/main/index.ts b/desktop/packages/mullvad-vpn/src/main/index.ts
index c2cc02eb10..457a192a1a 100644
--- a/desktop/packages/mullvad-vpn/src/main/index.ts
+++ b/desktop/packages/mullvad-vpn/src/main/index.ts
@@ -523,17 +523,6 @@ class ApplicationMain
 
     log.info('Connected to the daemon');
 
-    // verify daemon ownership
-    try {
-      await this.daemonRpc.verifyDaemonOwnership();
-      log.info('Verified daemon ownership');
-    } catch (e) {
-      const error = e as Error;
-      log.error(`Failed to verify daemon ownership: ${error.message}`);
-
-      return;
-    }
-
     this.notificationController.closeNotificationsInCategory(
       SystemNotificationCategory.tunnelState,
     );