Fix short notation paths being unauthorized [VPNWIN-2627]

eaproton · eaproton · commit b123cde4d1eb · 2025-02-17T16:35:03.000Z
diff --git a/src/IssueReporting/ProtonVPN.IssueReporting/SentryInitializer.cs b/src/IssueReporting/ProtonVPN.IssueReporting/SentryInitializer.cs
@@ -19,6 +19,7 @@
 
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Linq;
 using System.Reflection;
 using System.Text;
 using ProtonVPN.Builds.Variables;
@@ -113,7 +114,7 @@ private static void LogSentryEvent(SentryEvent e)
 
         private static string GetLogs()
         {
-            IList<string> logs = _logger?.GetRecentLogs() ?? new List<string>();
+            IList<string> logs = _logger?.GetRecentLogs().Reverse().ToList() ?? [];
             StringBuilder sb = new();
             foreach (string log in logs)
             {
diff --git a/src/Logging/ProtonVPN.Logging/Log4Net/Log4NetLogger.cs b/src/Logging/ProtonVPN.Logging/Log4Net/Log4NetLogger.cs
@@ -26,6 +26,8 @@ namespace ProtonVPN.Logging.Log4Net
 {
     public class Log4NetLogger : Log4NetLoggerInitializer, ILogger
     {
+        private const int MAX_RECENT_LOG_LINES = 100;
+
         private readonly IList<string> _recentLogs = new List<string>();
 
         public Log4NetLogger(ILoggerConfiguration loggerConfiguration)
@@ -150,7 +152,7 @@ private void AddMessageToRecentLogs(string message, Exception exception = null,
             }
             _recentLogs.Add(message);
 
-            while (_recentLogs.Count > 100)
+            while (_recentLogs.Count > MAX_RECENT_LOG_LINES)
             {
                 _recentLogs.RemoveAt(0);
             }
diff --git a/src/OperatingSystems/Processes/ProtonVPN.OperatingSystems.Processes.Contracts/IPipeStreamProcessIdentifier.cs b/src/OperatingSystems/Processes/ProtonVPN.OperatingSystems.Processes.Contracts/IPipeStreamProcessIdentifier.cs
@@ -23,6 +23,6 @@ namespace ProtonVPN.OperatingSystems.Processes.Contracts;
 
 public interface IPipeStreamProcessIdentifier
 {
-    string? GetClientProcessFileName(PipeStream pipeStream);
-    string? GetServerProcessFileName(PipeStream pipeStream);
+    string? GetClientProcessFullFilePath(PipeStream pipeStream);
+    string? GetServerProcessFullFilePath(PipeStream pipeStream);
 }
diff --git a/src/OperatingSystems/Processes/ProtonVPN.OperatingSystems.Processes/PipeStreamProcessIdentifier.cs b/src/OperatingSystems/Processes/ProtonVPN.OperatingSystems.Processes/PipeStreamProcessIdentifier.cs
@@ -35,18 +35,19 @@ public PipeStreamProcessIdentifier(ILogger logger)
         _logger = logger;
     }
 
-    public string? GetClientProcessFileName(PipeStream pipeStream)
+    public string? GetClientProcessFullFilePath(PipeStream pipeStream)
     {
-        return GetProcessFileName(GetClientProcessId, pipeStream);
+        return GetProcessFullFilePath(GetClientProcessId, pipeStream);
     }
 
-    private string? GetProcessFileName(Func<PipeStream, int> processIdRequester, PipeStream pipeStream)
+    private string? GetProcessFullFilePath(Func<PipeStream, int> processIdRequester, PipeStream pipeStream)
     {
         try
         {
             int processId = processIdRequester(pipeStream);
             Process? process = processId == 0 ? null : Process.GetProcessById(processId);
-            return process?.MainModule?.FileName;
+            string? filePath = process?.MainModule?.FileName;
+            return string.IsNullOrWhiteSpace(filePath) ? null : Path.GetFullPath(filePath);
         }
         catch (Exception ex)
         {
@@ -68,9 +69,9 @@ private int GetClientProcessId(PipeStream pipeStream)
     [return: MarshalAs(UnmanagedType.Bool)]
     private static extern bool GetNamedPipeClientProcessId(IntPtr hNamedPipe, out uint clientProcessId);
 
-    public string? GetServerProcessFileName(PipeStream pipeStream)
+    public string? GetServerProcessFullFilePath(PipeStream pipeStream)
     {
-        return GetProcessFileName(GetServerProcessId, pipeStream);
+        return GetProcessFullFilePath(GetServerProcessId, pipeStream);
     }
 
     private int GetServerProcessId(PipeStream pipeStream)
diff --git a/src/ProcessCommunication/Client/ProtonVPN.ProcessCommunication.Client/ResponseHandler.cs b/src/ProcessCommunication/Client/ProtonVPN.ProcessCommunication.Client/ResponseHandler.cs
@@ -72,33 +72,36 @@ private async Task HandleResponseAsync(HttpResponseMessage response)
             string serviceProcessVersion = GetHeaderValue(response.Headers, HttpConfiguration.SERVICE_PROCESS_VERSION);
             string installedServiceVersion = GetHeaderValue(response.Headers, HttpConfiguration.INSTALLED_SERVICE_VERSION);
 
-            _logger.Error<ProcessCommunicationErrorLog>(
-                $"Received HTTP status code {response.StatusCode} from gRPC server. " +
-                $"Client Process Path: '{clientProcessPath}' Version '{clientProcessVersion}'), " +
-                $"Service Process Path: '{serviceProcessPath}' Version '{serviceProcessVersion}'), " +
-                $"Installed Service Path: '{installedServicePath}' Version '{installedServiceVersion}')");
+            _logger.Error<ProcessCommunicationErrorLog>($"Received HTTP status code {response.StatusCode} from gRPC server. " +
+                $"Client Process Path: '{clientProcessPath}' Version '{clientProcessVersion}', " +
+                $"Service Process Path: '{serviceProcessPath}' Version '{serviceProcessVersion}', " +
+                $"Installed Service Path: '{installedServicePath}' Version '{installedServiceVersion}'");
 
             if (response.StatusCode == HttpStatusCode.Unauthorized)
             {
-                await Handle401UnauthorizedAsync(clientProcessVersion, serviceProcessVersion);
+                string logMessageDetails =
+                    $"Client Process Path: '{clientProcessPath}', " +
+                    $"Service Process Path: '{serviceProcessPath}', " +
+                    $"Installed Service Path: '{installedServicePath}'";
+                await Handle401UnauthorizedAsync(clientProcessVersion, serviceProcessVersion, logMessageDetails);
             }
         }
     }
 
-    private async Task Handle401UnauthorizedAsync(string clientProcessVersion, string serviceProcessVersion)
+    private async Task Handle401UnauthorizedAsync(string clientProcessVersion, string serviceProcessVersion, string logMessageDetails)
     {
         await _semaphore.WaitAsync();
         try
         {
-            Handle401Unauthorized(clientProcessVersion, serviceProcessVersion);
+            Handle401Unauthorized(clientProcessVersion, serviceProcessVersion, logMessageDetails);
         }
         finally
         {
             _semaphore.Release();
         }
     }
 
-    private void Handle401Unauthorized(string clientProcessVersionString, string serviceProcessVersionString)
+    private void Handle401Unauthorized(string clientProcessVersionString, string serviceProcessVersionString, string logMessageDetails)
     {
         string versions = $"ClientProcessVersion: {clientProcessVersionString}, ServiceProcessVersion: {serviceProcessVersionString}";
 
@@ -116,7 +119,7 @@ private void Handle401Unauthorized(string clientProcessVersionString, string ser
 
             if (!_is401UnauthorizedWithoutBeingBelowServiceVersionHandled)
             {
-                _issueReporter.CaptureMessage(logExplanation, versions);
+                _issueReporter.CaptureMessage(logExplanation, logMessageDetails);
                 _is401UnauthorizedWithoutBeingBelowServiceVersionHandled = true;
             }
         }
diff --git a/src/ProcessCommunication/Server/ProtonVPN.ProcessCommunication.Server/NamedPipeAuthorizationMiddleware.cs b/src/ProcessCommunication/Server/ProtonVPN.ProcessCommunication.Server/NamedPipeAuthorizationMiddleware.cs
@@ -79,7 +79,7 @@ public async Task InvokeAsync(HttpContext context)
         {
             context.Response.StatusCode = authorizationResult.StatusCode;
 
-            string installedServicePath = _registryEditor.ReadString(_registryUri);
+            string installedServicePath = Path.GetFullPath(_registryEditor.ReadString(_registryUri));
 
             string clientProcessVersionString = GetVersionFromFilePath(authorizationResult.ClientProcessFileName);
             string serviceProcessVersionString = GetVersionFromFilePath(authorizationResult.ServerProcessFileName);
@@ -91,17 +91,17 @@ public async Task InvokeAsync(HttpContext context)
                 Version.TryParse(installedServiceVersionString, out Version installedServiceVersion) &&
                 installedServiceVersion > serviceProcessVersion)
             {
-                string serviceStopDescription = $"Client Process Path: '{authorizationResult.ClientProcessFileName}' Version '{clientProcessVersionString}'), " +
-                    $"Service Process Path: '{authorizationResult.ServerProcessFileName}' Version '{serviceProcessVersionString}'), " +
-                    $"Installed Service Path: '{installedServicePath}' Version '{installedServiceVersionString}')";
+                string serviceStopDescription = $"Client Process Path: '{authorizationResult.ClientProcessFileName}' Version '{clientProcessVersionString}', " +
+                    $"Service Process Path: '{authorizationResult.ServerProcessFileName}' Version '{serviceProcessVersionString}', " +
+                    $"Installed Service Path: '{installedServicePath}' Version '{installedServiceVersionString}'";
                 await StopServiceAsync(serviceStopDescription);
                 context.Response.StatusCode = StatusCodes.Status409Conflict;
             }
 
             _logger.Error<ProcessCommunicationErrorLog>($"Sending HTTP status code {context.Response.StatusCode} to gRPC client. " +
-                $"Client Process Path: '{authorizationResult.ClientProcessFileName}' Version '{clientProcessVersionString}'), " +
-                $"Service Process Path: '{authorizationResult.ServerProcessFileName}' Version '{serviceProcessVersionString}'), " +
-                $"Installed Service Path: '{installedServicePath}' Version '{installedServiceVersionString}')");
+                $"Client Process Path: '{authorizationResult.ClientProcessFileName}' Version '{clientProcessVersionString}', " +
+                $"Service Process Path: '{authorizationResult.ServerProcessFileName}' Version '{serviceProcessVersionString}', " +
+                $"Installed Service Path: '{installedServicePath}' Version '{installedServiceVersionString}'");
 
             context.Response.Headers[HttpConfiguration.CLIENT_PROCESS_PATH] = authorizationResult.ClientProcessFileName;
             context.Response.Headers[HttpConfiguration.SERVICE_PROCESS_PATH] = authorizationResult.ServerProcessFileName;
@@ -160,23 +160,25 @@ private async Task<AuthorizationResult> AuthorizeAsync(HttpContext context)
         {
             NamedPipeServerStream namedPipe = GetNamedPipe(context);
 
-            string clientProcessFileName = _pipeStreamProcessIdentifier.GetClientProcessFileName(namedPipe) ?? string.Empty;
-            string serverProcessFileName = _pipeStreamProcessIdentifier.GetServerProcessFileName(namedPipe) ?? string.Empty;
+            string clientProcessPath = _pipeStreamProcessIdentifier.GetClientProcessFullFilePath(namedPipe) ?? string.Empty;
+            string serverProcessPath = _pipeStreamProcessIdentifier.GetServerProcessFullFilePath(namedPipe) ?? string.Empty;
 
-            if (!_config.ServiceExePath.Equals(serverProcessFileName, StringComparison.InvariantCultureIgnoreCase))
+            string expectedServiceProcessPath = Path.GetFullPath(_config.ServiceExePath);
+            if (!expectedServiceProcessPath.Equals(serverProcessPath, StringComparison.InvariantCultureIgnoreCase))
             {
                 _logger.Warn<ProcessCommunicationErrorLog>($"The owner of the Named Pipe is not this Service. Dispose and recreate.");
                 await _recreateAndStartAsyncFunc();
-                return AuthorizationResult.Error(StatusCodes.Status409Conflict, clientProcessFileName, serverProcessFileName);
+                return AuthorizationResult.Error(StatusCodes.Status409Conflict, clientProcessPath, serverProcessPath);
             }
 
-            if (_config.AppExePath.Equals(clientProcessFileName, StringComparison.InvariantCultureIgnoreCase))
+            string expectedClientProcessPath = Path.GetFullPath(_config.AppExePath);
+            if (expectedClientProcessPath.Equals(clientProcessPath, StringComparison.InvariantCultureIgnoreCase))
             {
                 return AuthorizationResult.Ok();
             }
 
-            _logger.Warn<ProcessCommunicationErrorLog>($"The connected client is unauthorized. Client path: '{clientProcessFileName}'. Expected: '{_config.AppExePath}'.");
-            return AuthorizationResult.Error(StatusCodes.Status401Unauthorized, clientProcessFileName, serverProcessFileName);
+            _logger.Warn<ProcessCommunicationErrorLog>($"The connected client is unauthorized. Client path: '{clientProcessPath}'. Expected: '{expectedClientProcessPath}'.");
+            return AuthorizationResult.Error(StatusCodes.Status401Unauthorized, clientProcessPath, serverProcessPath);
         }
         catch (Exception ex)
         {

Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,7 @@`
`19`	`19`
`20`	`20`	`using System.Collections.Generic;`
`21`	`21`	`using System.Diagnostics;`
	`22`	`+using System.Linq;`
`22`	`23`	`using System.Reflection;`
`23`	`24`	`using System.Text;`
`24`	`25`	`using ProtonVPN.Builds.Variables;`
`@@ -113,7 +114,7 @@ private static void LogSentryEvent(SentryEvent e)`
`113`	`114`
`114`	`115`	`private static string GetLogs()`
`115`	`116`	`{`
`116`		`- IList<string> logs = _logger?.GetRecentLogs() ?? new List<string>();`
	`117`	`+ IList<string> logs = _logger?.GetRecentLogs().Reverse().ToList() ?? [];`
`117`	`118`	`StringBuilder sb = new();`
`118`	`119`	`foreach (string log in logs)`
`119`	`120`	`{`
Original file line number	Diff line number	Diff line change
`@@ -26,6 +26,8 @@ namespace ProtonVPN.Logging.Log4Net`
`26`	`26`	`{`
`27`	`27`	`public class Log4NetLogger : Log4NetLoggerInitializer, ILogger`
`28`	`28`	`{`
	`29`	`+ private const int MAX_RECENT_LOG_LINES = 100;`
	`30`	`+`
`29`	`31`	`private readonly IList<string> _recentLogs = new List<string>();`
`30`	`32`
`31`	`33`	`public Log4NetLogger(ILoggerConfiguration loggerConfiguration)`
`@@ -150,7 +152,7 @@ private void AddMessageToRecentLogs(string message, Exception exception = null,`
`150`	`152`	`}`
`151`	`153`	`_recentLogs.Add(message);`
`152`	`154`
`153`		`- while (_recentLogs.Count > 100)`
	`155`	`+ while (_recentLogs.Count > MAX_RECENT_LOG_LINES)`
`154`	`156`	`{`
`155`	`157`	`_recentLogs.RemoveAt(0);`
`156`	`158`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,6 +23,6 @@ namespace ProtonVPN.OperatingSystems.Processes.Contracts;`
`23`	`23`
`24`	`24`	`public interface IPipeStreamProcessIdentifier`
`25`	`25`	`{`
`26`		`- string? GetClientProcessFileName(PipeStream pipeStream);`
`27`		`- string? GetServerProcessFileName(PipeStream pipeStream);`
	`26`	`+ string? GetClientProcessFullFilePath(PipeStream pipeStream);`
	`27`	`+ string? GetServerProcessFullFilePath(PipeStream pipeStream);`
`28`	`28`	`}`
Original file line number	Diff line number	Diff line change
`@@ -35,18 +35,19 @@ public PipeStreamProcessIdentifier(ILogger logger)`
`35`	`35`	`_logger = logger;`
`36`	`36`	`}`
`37`	`37`
`38`		`- public string? GetClientProcessFileName(PipeStream pipeStream)`
	`38`	`+ public string? GetClientProcessFullFilePath(PipeStream pipeStream)`
`39`	`39`	`{`
`40`		`- return GetProcessFileName(GetClientProcessId, pipeStream);`
	`40`	`+ return GetProcessFullFilePath(GetClientProcessId, pipeStream);`
`41`	`41`	`}`
`42`	`42`
`43`		`- private string? GetProcessFileName(Func<PipeStream, int> processIdRequester, PipeStream pipeStream)`
	`43`	`+ private string? GetProcessFullFilePath(Func<PipeStream, int> processIdRequester, PipeStream pipeStream)`
`44`	`44`	`{`
`45`	`45`	`try`
`46`	`46`	`{`
`47`	`47`	`int processId = processIdRequester(pipeStream);`
`48`	`48`	`Process? process = processId == 0 ? null : Process.GetProcessById(processId);`
`49`		`- return process?.MainModule?.FileName;`
	`49`	`+ string? filePath = process?.MainModule?.FileName;`
	`50`	`+ return string.IsNullOrWhiteSpace(filePath) ? null : Path.GetFullPath(filePath);`
`50`	`51`	`}`
`51`	`52`	`catch (Exception ex)`
`52`	`53`	`{`
`@@ -68,9 +69,9 @@ private int GetClientProcessId(PipeStream pipeStream)`
`68`	`69`	`[return: MarshalAs(UnmanagedType.Bool)]`
`69`	`70`	`private static extern bool GetNamedPipeClientProcessId(IntPtr hNamedPipe, out uint clientProcessId);`
`70`	`71`
`71`		`- public string? GetServerProcessFileName(PipeStream pipeStream)`
	`72`	`+ public string? GetServerProcessFullFilePath(PipeStream pipeStream)`
`72`	`73`	`{`
`73`		`- return GetProcessFileName(GetServerProcessId, pipeStream);`
	`74`	`+ return GetProcessFullFilePath(GetServerProcessId, pipeStream);`
`74`	`75`	`}`
`75`	`76`
`76`	`77`	`private int GetServerProcessId(PipeStream pipeStream)`
Original file line number	Diff line number	Diff line change
`@@ -72,33 +72,36 @@ private async Task HandleResponseAsync(HttpResponseMessage response)`
`72`	`72`	`string serviceProcessVersion = GetHeaderValue(response.Headers, HttpConfiguration.SERVICE_PROCESS_VERSION);`
`73`	`73`	`string installedServiceVersion = GetHeaderValue(response.Headers, HttpConfiguration.INSTALLED_SERVICE_VERSION);`
`74`	`74`
`75`		`- _logger.Error<ProcessCommunicationErrorLog>(`
`76`		`- $"Received HTTP status code {response.StatusCode} from gRPC server. " +`
`77`		`- $"Client Process Path: '{clientProcessPath}' Version '{clientProcessVersion}'), " +`
`78`		`- $"Service Process Path: '{serviceProcessPath}' Version '{serviceProcessVersion}'), " +`
`79`		`- $"Installed Service Path: '{installedServicePath}' Version '{installedServiceVersion}')");`
	`75`	`+ _logger.Error<ProcessCommunicationErrorLog>($"Received HTTP status code {response.StatusCode} from gRPC server. " +`
	`76`	`+ $"Client Process Path: '{clientProcessPath}' Version '{clientProcessVersion}', " +`
	`77`	`+ $"Service Process Path: '{serviceProcessPath}' Version '{serviceProcessVersion}', " +`
	`78`	`+ $"Installed Service Path: '{installedServicePath}' Version '{installedServiceVersion}'");`
`80`	`79`
`81`	`80`	`if (response.StatusCode == HttpStatusCode.Unauthorized)`
`82`	`81`	`{`
`83`		`- await Handle401UnauthorizedAsync(clientProcessVersion, serviceProcessVersion);`
	`82`	`+ string logMessageDetails =`
	`83`	`+ $"Client Process Path: '{clientProcessPath}', " +`
	`84`	`+ $"Service Process Path: '{serviceProcessPath}', " +`
	`85`	`+ $"Installed Service Path: '{installedServicePath}'";`
	`86`	`+ await Handle401UnauthorizedAsync(clientProcessVersion, serviceProcessVersion, logMessageDetails);`
`84`	`87`	`}`
`85`	`88`	`}`
`86`	`89`	`}`
`87`	`90`
`88`		`- private async Task Handle401UnauthorizedAsync(string clientProcessVersion, string serviceProcessVersion)`
	`91`	`+ private async Task Handle401UnauthorizedAsync(string clientProcessVersion, string serviceProcessVersion, string logMessageDetails)`
`89`	`92`	`{`
`90`	`93`	`await _semaphore.WaitAsync();`
`91`	`94`	`try`
`92`	`95`	`{`
`93`		`- Handle401Unauthorized(clientProcessVersion, serviceProcessVersion);`
	`96`	`+ Handle401Unauthorized(clientProcessVersion, serviceProcessVersion, logMessageDetails);`
`94`	`97`	`}`
`95`	`98`	`finally`
`96`	`99`	`{`
`97`	`100`	`_semaphore.Release();`
`98`	`101`	`}`
`99`	`102`	`}`
`100`	`103`
`101`		`- private void Handle401Unauthorized(string clientProcessVersionString, string serviceProcessVersionString)`
	`104`	`+ private void Handle401Unauthorized(string clientProcessVersionString, string serviceProcessVersionString, string logMessageDetails)`
`102`	`105`	`{`
`103`	`106`	`string versions = $"ClientProcessVersion: {clientProcessVersionString}, ServiceProcessVersion: {serviceProcessVersionString}";`
`104`	`107`
`@@ -116,7 +119,7 @@ private void Handle401Unauthorized(string clientProcessVersionString, string ser`
`116`	`119`
`117`	`120`	`if (!_is401UnauthorizedWithoutBeingBelowServiceVersionHandled)`
`118`	`121`	`{`
`119`		`- _issueReporter.CaptureMessage(logExplanation, versions);`
	`122`	`+ _issueReporter.CaptureMessage(logExplanation, logMessageDetails);`
`120`	`123`	`_is401UnauthorizedWithoutBeingBelowServiceVersionHandled = true;`
`121`	`124`	`}`
`122`	`125`	`}`