diff --git a/App/App.csproj b/App/App.csproj index cd1df42..d4f2bed 100644 --- a/App/App.csproj +++ b/App/App.csproj @@ -30,7 +30,7 @@ - + diff --git a/Installer/Installer.csproj b/Installer/Installer.csproj index 9425a01..b850f6a 100644 --- a/Installer/Installer.csproj +++ b/Installer/Installer.csproj @@ -1,24 +1,24 @@ - - Coder.Desktop.Installer - Coder.Desktop.Installer - Exe - net481 - 13.0 - + + Coder.Desktop.Installer + Coder.Desktop.Installer + Exe + net481 + 13.0 + - - - - - - - + + + + + + + - - - - - + + + + + diff --git a/Installer/Program.cs b/Installer/Program.cs index 0a34a6e..0bec102 100644 --- a/Installer/Program.cs +++ b/Installer/Program.cs @@ -250,13 +250,17 @@ private static int BuildMsiPackage(MsiOptions opts) programFiles64Folder.AddDir(installDir); project.AddDir(programFiles64Folder); - // Add registry values that are consumed by the manager. + // Add registry values that are consumed by the manager. Note that these + // should not be changed. See Vpn.Service/Program.cs and + // Vpn.Service/ManagerConfig.cs for more details. project.AddRegValues( new RegValue(RegistryHive, RegistryKey, "Manager:ServiceRpcPipeName", "Coder.Desktop.Vpn"), new RegValue(RegistryHive, RegistryKey, "Manager:TunnelBinaryPath", $"[INSTALLFOLDER]{opts.VpnDir}\\coder-vpn.exe"), new RegValue(RegistryHive, RegistryKey, "Manager:LogFileLocation", - @"[INSTALLFOLDER]coder-desktop-service.log")); + @"[INSTALLFOLDER]coder-desktop-service.log"), + new RegValue(RegistryHive, RegistryKey, "Manager:TunnelBinarySignatureSigner", "Coder Technologies Inc."), + new RegValue(RegistryHive, RegistryKey, "Manager:TunnelBinaryAllowVersionMismatch", "false")); // Note: most of this control panel info will not be visible as this // package is usually hidden in favor of the bootstrapper showing diff --git a/README.md b/README.md new file mode 100644 index 0000000..74c7101 --- /dev/null +++ b/README.md @@ -0,0 +1,29 @@ +# Coder Desktop for Windows + +This repo contains the C# source code for Coder Desktop for Windows. You can +download the latest version from the GitHub releases. + +### Contributing + +You will need: + +- Visual Studio 2022 + - .NET desktop development + - WinUI application development + - Windows 10 SDK (10.0.19041.0) +- Wix Toolset 5.0.2 (if building the installer) + +It's also recommended to use JetBrains Rider (or VS + ReSharper) for a better +experience. + +### License + +The Coder Desktop for Windows source is licensed under the GNU Affero General +Public License v3.0 (AGPL-3.0). + +Some vendored files in this repo are licensed separately. The license for these +files can be found in the same directory as the files. + +The binary distributions of Coder Desktop for Windows have some additional +license disclaimers that can be found in +[scripts/files/License.txt](scripts/files/License.txt) or during installation. diff --git a/Tests.Vpn.Service/DownloaderTest.cs b/Tests.Vpn.Service/DownloaderTest.cs index ae3a0a0..985e331 100644 --- a/Tests.Vpn.Service/DownloaderTest.cs +++ b/Tests.Vpn.Service/DownloaderTest.cs @@ -1,4 +1,6 @@ +using System.Reflection; using System.Security.Cryptography; +using System.Security.Cryptography.X509Certificates; using System.Text; using Coder.Desktop.Vpn.Service; using Microsoft.Extensions.Logging.Abstractions; @@ -27,40 +29,102 @@ public class AuthenticodeDownloadValidatorTest [CancelAfter(30_000)] public void Unsigned(CancellationToken ct) { - // TODO: this + var testBinaryPath = Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", "hello.exe"); + var ex = Assert.ThrowsAsync(() => + AuthenticodeDownloadValidator.Coder.ValidateAsync(testBinaryPath, ct)); + Assert.That(ex.Message, + Does.Contain( + "File is not signed and trusted with an Authenticode signature: State=Unsigned, StateReason=None")); } [Test(Description = "Test an untrusted binary")] [CancelAfter(30_000)] public void Untrusted(CancellationToken ct) { - // TODO: this + var testBinaryPath = + Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", "hello-self-signed.exe"); + var ex = Assert.ThrowsAsync(() => + AuthenticodeDownloadValidator.Coder.ValidateAsync(testBinaryPath, ct)); + Assert.That(ex.Message, + Does.Contain( + "File is not signed and trusted with an Authenticode signature: State=Unsigned, StateReason=UntrustedRoot")); } [Test(Description = "Test an binary with a detached signature (catalog file)")] [CancelAfter(30_000)] public void DifferentCertTrusted(CancellationToken ct) { - // notepad.exe uses a catalog file for its signature. + // rundll32.exe uses a catalog file for its signature. var ex = Assert.ThrowsAsync(() => - AuthenticodeDownloadValidator.Coder.ValidateAsync(@"C:\Windows\System32\notepad.exe", ct)); + AuthenticodeDownloadValidator.Coder.ValidateAsync(@"C:\Windows\System32\rundll32.exe", ct)); Assert.That(ex.Message, Does.Contain("File is not signed with an embedded Authenticode signature: Kind=Catalog")); } - [Test(Description = "Test a binary signed by a different certificate")] + [Test(Description = "Test a binary signed by a non-EV certificate")] + [CancelAfter(30_000)] + public void NonEvCert(CancellationToken ct) + { + // dotnet.exe is signed by .NET. During tests we can be pretty sure + // this is installed. + var ex = Assert.ThrowsAsync(() => + AuthenticodeDownloadValidator.Coder.ValidateAsync(@"C:\Program Files\dotnet\dotnet.exe", ct)); + Assert.That(ex.Message, + Does.Contain( + "File is not signed with an Extended Validation Code Signing certificate")); + } + + [Test(Description = "Test a binary signed by an EV certificate with a different name")] [CancelAfter(30_000)] - public void DifferentCertUntrusted(CancellationToken ct) + public void EvDifferentCertName(CancellationToken ct) { - // TODO: this + var testBinaryPath = Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", + "hello-versioned-signed.exe"); + var ex = Assert.ThrowsAsync(() => + new AuthenticodeDownloadValidator("Acme Corporation").ValidateAsync(testBinaryPath, ct)); + Assert.That(ex.Message, + Does.Contain( + "File is signed by an unexpected certificate: ExpectedName='Acme Corporation', ActualName='Coder Technologies Inc.'")); } [Test(Description = "Test a binary signed by Coder's certificate")] [CancelAfter(30_000)] public async Task CoderSigned(CancellationToken ct) { - // TODO: this - await Task.CompletedTask; + var testBinaryPath = Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", + "hello-versioned-signed.exe"); + await AuthenticodeDownloadValidator.Coder.ValidateAsync(testBinaryPath, ct); + } + + [Test(Description = "Test if the EV check works")] + public void IsEvCert() + { + // To avoid potential API misuse the function is private. + var method = typeof(AuthenticodeDownloadValidator).GetMethod("IsExtendedValidationCertificate", + BindingFlags.NonPublic | BindingFlags.Static); + Assert.That(method, Is.Not.Null, "Could not find IsExtendedValidationCertificate method"); + + // Call it with various certificates. + var certs = new List<(string, bool)> + { + // EV: + (Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", "coder-ev.crt"), true), + (Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", "google-llc-ev.crt"), true), + (Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", "self-signed-ev.crt"), true), + // Not EV: + (Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", "mozilla-corporation.crt"), false), + (Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", "self-signed.crt"), false), + }; + + foreach (var (certPath, isEv) in certs) + { + var x509Cert = new X509Certificate2(certPath); + var result = (bool?)method!.Invoke(null, [x509Cert]); + Assert.That(result, Is.Not.Null, + $"IsExtendedValidationCertificate returned null for {Path.GetFileName(certPath)}"); + Assert.That(result, Is.EqualTo(isEv), + $"IsExtendedValidationCertificate returned wrong result for {Path.GetFileName(certPath)}"); + } } } @@ -71,22 +135,60 @@ public class AssemblyVersionDownloadValidatorTest [CancelAfter(30_000)] public void NoVersion(CancellationToken ct) { - // TODO: this + var testBinaryPath = Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", "hello.exe"); + var ex = Assert.ThrowsAsync(() => + new AssemblyVersionDownloadValidator(1, 2, 3, 4).ValidateAsync(testBinaryPath, ct)); + Assert.That(ex.Message, Does.Contain("File ProductVersion is empty or null")); + } + + [Test(Description = "Invalid version on binary")] + [CancelAfter(30_000)] + public void InvalidVersion(CancellationToken ct) + { + var testBinaryPath = + Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", "hello-invalid-version.exe"); + var ex = Assert.ThrowsAsync(() => + new AssemblyVersionDownloadValidator(1, 2, 3, 4).ValidateAsync(testBinaryPath, ct)); + Assert.That(ex.Message, Does.Contain("File ProductVersion '1-2-3-4' is not a valid version string")); } - [Test(Description = "Version mismatch")] + [Test(Description = "Version mismatch with full version check")] [CancelAfter(30_000)] - public void VersionMismatch(CancellationToken ct) + public void VersionMismatchFull(CancellationToken ct) { - // TODO: this + var testBinaryPath = Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", + "hello-versioned-signed.exe"); + + // Try changing each version component one at a time + var expectedVersions = new[] { 1, 2, 3, 4 }; + for (var i = 0; i < 4; i++) + { + var testVersions = (int[])expectedVersions.Clone(); + testVersions[i]++; // Increment this component to make it wrong + + var ex = Assert.ThrowsAsync(() => + new AssemblyVersionDownloadValidator( + testVersions[0], testVersions[1], testVersions[2], testVersions[3] + ).ValidateAsync(testBinaryPath, ct)); + + Assert.That(ex.Message, Does.Contain( + $"File ProductVersion does not match expected version: Actual='1.2.3.4', Expected='{string.Join(".", testVersions)}'")); + } } - [Test(Description = "Version match")] + [Test(Description = "Version match with and without partial version check")] [CancelAfter(30_000)] public async Task VersionMatch(CancellationToken ct) { - // TODO: this - await Task.CompletedTask; + var testBinaryPath = Path.Combine(TestContext.CurrentContext.TestDirectory, "testdata", + "hello-versioned-signed.exe"); + + // Test with just major.minor + await new AssemblyVersionDownloadValidator(1, 2).ValidateAsync(testBinaryPath, ct); + // Test with major.minor.patch + await new AssemblyVersionDownloadValidator(1, 2, 3).ValidateAsync(testBinaryPath, ct); + // Test with major.minor.patch.build + await new AssemblyVersionDownloadValidator(1, 2, 3, 4).ValidateAsync(testBinaryPath, ct); } } diff --git a/Tests.Vpn.Service/Tests.Vpn.Service.csproj b/Tests.Vpn.Service/Tests.Vpn.Service.csproj index cc66dd1..c57f85a 100644 --- a/Tests.Vpn.Service/Tests.Vpn.Service.csproj +++ b/Tests.Vpn.Service/Tests.Vpn.Service.csproj @@ -12,6 +12,36 @@ true + + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + + + PreserveNewest + + + all diff --git a/Tests.Vpn.Service/testdata/.gitignore b/Tests.Vpn.Service/testdata/.gitignore new file mode 100644 index 0000000..1ce9d77 --- /dev/null +++ b/Tests.Vpn.Service/testdata/.gitignore @@ -0,0 +1,2 @@ +*.go +*.pfx diff --git a/Tests.Vpn.Service/testdata/Build-Assets.ps1 b/Tests.Vpn.Service/testdata/Build-Assets.ps1 new file mode 100644 index 0000000..bfbd9fa --- /dev/null +++ b/Tests.Vpn.Service/testdata/Build-Assets.ps1 @@ -0,0 +1,68 @@ +$errorActionPreference = "Stop" + +Set-Location $PSScriptRoot + +# If hello.go does not exist, write it. We don't check it into the repo to avoid +# GitHub showing that the repo contains Go code. +if (-not (Test-Path "hello.go")) { + $helloGo = @" +package main + +func main() { + println("Hello, World!") +} +"@ + Set-Content -Path "hello.go" -Value $helloGo +} + +& go.exe build -ldflags '-w -s' -o hello.exe hello.go +if ($LASTEXITCODE -ne 0) { throw "Failed to build hello.exe" } + +# hello-invalid-version.exe is used for testing versioned binaries with an +# invalid version. +Copy-Item hello.exe hello-invalid-version.exe +& go-winres.exe patch --in winres.json --delete --no-backup --product-version 1-2-3-4 --file-version 1-2-3-4 hello-invalid-version.exe +if ($LASTEXITCODE -ne 0) { throw "Failed to patch hello-invalid-version.exe with go-winres" } + +# hello-self-signed.exe is used for testing untrusted binaries. +Copy-Item hello.exe hello-self-signed.exe +$helloSelfSignedPath = (Get-Item hello-self-signed.exe).FullName + +# Create a self signed certificate for signing and then delete it. +$certStoreLocation = "Cert:\CurrentUser\My" +$password = "password" +$cert = New-SelfSignedCertificate ` + -CertStoreLocation $certStoreLocation ` + -DnsName coder.com ` + -Subject "CN=coder-desktop-windows-self-signed-cert" ` + -Type CodeSigningCert ` + -KeyUsage DigitalSignature ` + -NotAfter (Get-Date).AddDays(3650) +$pfxPath = Join-Path $PSScriptRoot "cert.pfx" +try { + $securePassword = ConvertTo-SecureString -String $password -Force -AsPlainText + Export-PfxCertificate -Cert $cert -FilePath $pfxPath -Password $securePassword + + # Sign hello-self-signed.exe with the self signed certificate + & "${env:ProgramFiles(x86)}\Windows Kits\10\bin\10.0.19041.0\x64\signtool.exe" sign /debug /f $pfxPath /p $password /tr "http://timestamp.digicert.com" /td sha256 /fd sha256 $helloSelfSignedPath + if ($LASTEXITCODE -ne 0) { throw "Failed to sign hello-self-signed.exe with signtool" } +} finally { + if ($cert.Thumbprint) { + Remove-Item -Path (Join-Path $certStoreLocation $cert.Thumbprint) -Force + } + if (Test-Path $pfxPath) { + Remove-Item -Path $pfxPath -Force + } +} + +# hello-versioned-signed.exe is used for testing versioned binaries and +# binaries signed by a real EV certificate. +Copy-Item hello.exe hello-versioned-signed.exe + +& go-winres.exe patch --in winres.json --delete --no-backup --product-version 1.2.3.4 --file-version 1.2.3.4 hello-versioned-signed.exe +if ($LASTEXITCODE -ne 0) { throw "Failed to patch hello-versioned-signed.exe with go-winres" } + +# Then sign hello-versioned-signed.exe with the same EV cert as our real +# binaries. Since this is a bit more complicated and requires some extra +# permissions, we don't do this in the build script. +Write-Host "Don't forget to sign hello-versioned-signed.exe with the EV cert!" diff --git a/Tests.Vpn.Service/testdata/README.md b/Tests.Vpn.Service/testdata/README.md new file mode 100644 index 0000000..6e81b37 --- /dev/null +++ b/Tests.Vpn.Service/testdata/README.md @@ -0,0 +1,29 @@ +# Tests.Vpn.Service testdata + +### Executables + +`Build-Assets.ps1` creates `hello.exe` and derivatives. You need `go`, +`go-winres` and Windows 10 SDK 10.0.19041.0 installed to run this. + +You must sign `hello-versioned-signed.exe` yourself with the Coder EV cert after +the script completes. + +These files are checked into the repo so they shouldn't need to be built again. + +### Certificates + +- `coder-ev.crt` is the Extended Validation Code Signing certificate used by + Coder, extracted from a signed release binary on 2025-03-07 +- `google-llc-ev.crt` is the Extended Validation Code Signing certificate used + by Google Chrome, extracted from an official binary on 2025-03-07 +- `mozilla-corporation.crt` is a regular Code Signing certificate used by + Mozilla Firefox, extracted from an official binary on 2025-03-07 +- `self-signed-ev.crt` was generated with `gen-certs.sh` using Linux OpenSSL +- `self-signed.crt` was generated with `gen-certs.sh` using Linux OpenSSL + +You can extract a certificate from an executable with the following PowerShell +one-liner: + +```powershell +Get-AuthenticodeSignature binary.exe | Select-Object -ExpandProperty SignerCertificate | Export-Certificate -Type CERT -FilePath output.crt +``` diff --git a/Tests.Vpn.Service/testdata/coder-ev.crt b/Tests.Vpn.Service/testdata/coder-ev.crt new file mode 100644 index 0000000..223fbf2 Binary files /dev/null and b/Tests.Vpn.Service/testdata/coder-ev.crt differ diff --git a/Tests.Vpn.Service/testdata/gen-certs.sh b/Tests.Vpn.Service/testdata/gen-certs.sh new file mode 100644 index 0000000..c1b9ab0 --- /dev/null +++ b/Tests.Vpn.Service/testdata/gen-certs.sh @@ -0,0 +1,49 @@ +#!/bin/bash +set -euo pipefail + +# Generate a regular code signing certificate without the EV policy OID. +openssl req \ + -x509 \ + -newkey rsa:2048 \ + -keyout /dev/null \ + -out self-signed.crt \ + -days 3650 \ + -nodes \ + -subj "/CN=Coder Self Signed" \ + -addext "keyUsage=digitalSignature" \ + -addext "extendedKeyUsage=codeSigning" + +# Generate an EV code signing certificate by adding the EV policy OID. We add +# a different OID before the EV OID to ensure the validator can handle multiple +# policies. +config=" +[req] +distinguished_name = req_distinguished_name +x509_extensions = v3_req +prompt = no + +[req_distinguished_name] +CN = Coder Self Signed EV + +[v3_req] +keyUsage = digitalSignature +extendedKeyUsage = codeSigning +certificatePolicies = @pol1,@pol2 + +[pol1] +policyIdentifier = 2.23.140.1.4.1 +CPS.1="https://coder.com" + +[pol2] +policyIdentifier = 2.23.140.1.3 +CPS.1="https://coder.com" +" + +openssl req \ + -x509 \ + -newkey rsa:2048 \ + -keyout /dev/null \ + -out self-signed-ev.crt \ + -days 3650 \ + -nodes \ + -config <(echo "$config") diff --git a/Tests.Vpn.Service/testdata/google-llc-ev.crt b/Tests.Vpn.Service/testdata/google-llc-ev.crt new file mode 100644 index 0000000..d214f20 Binary files /dev/null and b/Tests.Vpn.Service/testdata/google-llc-ev.crt differ diff --git a/Tests.Vpn.Service/testdata/hello-invalid-version.exe b/Tests.Vpn.Service/testdata/hello-invalid-version.exe new file mode 100644 index 0000000..3ef2656 Binary files /dev/null and b/Tests.Vpn.Service/testdata/hello-invalid-version.exe differ diff --git a/Tests.Vpn.Service/testdata/hello-self-signed.exe b/Tests.Vpn.Service/testdata/hello-self-signed.exe new file mode 100644 index 0000000..124fb08 Binary files /dev/null and b/Tests.Vpn.Service/testdata/hello-self-signed.exe differ diff --git a/Tests.Vpn.Service/testdata/hello-versioned-signed.exe b/Tests.Vpn.Service/testdata/hello-versioned-signed.exe new file mode 100644 index 0000000..ce8f1d1 Binary files /dev/null and b/Tests.Vpn.Service/testdata/hello-versioned-signed.exe differ diff --git a/Tests.Vpn.Service/testdata/hello.exe b/Tests.Vpn.Service/testdata/hello.exe new file mode 100644 index 0000000..4759d72 Binary files /dev/null and b/Tests.Vpn.Service/testdata/hello.exe differ diff --git a/Tests.Vpn.Service/testdata/mozilla-corporation.crt b/Tests.Vpn.Service/testdata/mozilla-corporation.crt new file mode 100644 index 0000000..6eb3454 Binary files /dev/null and b/Tests.Vpn.Service/testdata/mozilla-corporation.crt differ diff --git a/Tests.Vpn.Service/testdata/self-signed-ev.crt b/Tests.Vpn.Service/testdata/self-signed-ev.crt new file mode 100644 index 0000000..11dfd79 --- /dev/null +++ b/Tests.Vpn.Service/testdata/self-signed-ev.crt @@ -0,0 +1,21 @@ +-----BEGIN CERTIFICATE----- +MIIDcTCCAlmgAwIBAgIUEzhP7oxDynN4aXNmRqGZzPUCkxUwDQYJKoZIhvcNAQEL +BQAwHzEdMBsGA1UEAwwUQ29kZXIgU2VsZiBTaWduZWQgRVYwHhcNMjUwMzA3MDQ0 +NzQ2WhcNMzUwMzA1MDQ0NzQ2WjAfMR0wGwYDVQQDDBRDb2RlciBTZWxmIFNpZ25l +ZCBFVjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALzzLlfpFYwEfQb6 +eizMaUr9/Op2ELLBRjabDT17uXBTPFHVtHewIaZfYPv7aY3B3rQTzHfE0y9YYO0+ +1Zhd2WpNKYO3iQM9OOcd69XYuDeRh09m6vOwcK7gSkSr55D/dUe4+vnjQBG9O6Na +fby/kcJuDVNnR9rTPJpXqfnlgjrO2WNbBn3K0xJcNjVpMqFm2Iw9eYCRTVIUp559 ++iUGnwM+NT0cGAMB8242Jyz6xgEaRnSmddmxLDkfWWfivamSpWaaopR2T5+6txFW +C1vBeZ4Au+9FEne64NVefVDjdeIDU7pgwYxykPDf+Sc704L8Da5X0gEoa82pHOfw +1DNP94kCAwEAAaOBpDCBoTALBgNVHQ8EBAMCB4AwEwYDVR0lBAwwCgYIKwYBBQUH +AwMwXgYDVR0gBFcwVTApBgZngQwBBAEwHzAdBggrBgEFBQcCARYRaHR0cHM6Ly9j +b2Rlci5jb20wKAYFZ4EMAQMwHzAdBggrBgEFBQcCARYRaHR0cHM6Ly9jb2Rlci5j +b20wHQYDVR0OBBYEFL9MxyFpCw/CmWioihuEP6x8XW0zMA0GCSqGSIb3DQEBCwUA +A4IBAQC19lsc4EEOD4H3VNtQpLaBPLmNU4dD4bpWpoqv2YIYFl0T2cSkZZ1ZmSKR +PV+1D3w7HsdmOf+1wXQv8w4POy3Z/7m6pcy/Efw9ImYs5zwRr5AniFJxjRBkUYB2 +i2m3650v5OAab4qay0FWCY4/8MX866fiLrO0oyjFI6tU/Py8kWV7IgOa9RxJpNou +oITfLXLZRgXULiaXaQRA4TdD5zI9Qe/wwvj6wJH3u8qpRq+m+vo0cxfQ47tisL11 +nMM59fUZrypxdOTRK0QiGz5rJlLmZXZO27RNT3ewpJsq4qjQ3CtJ946vdjDc8+kY +ChQ9e6sS5mLBP4JXtuyG+P1Fdp5t +-----END CERTIFICATE----- diff --git a/Tests.Vpn.Service/testdata/self-signed.crt b/Tests.Vpn.Service/testdata/self-signed.crt new file mode 100644 index 0000000..0eca8eb --- /dev/null +++ b/Tests.Vpn.Service/testdata/self-signed.crt @@ -0,0 +1,20 @@ +-----BEGIN CERTIFICATE----- +MIIDOzCCAiOgAwIBAgIUJAIgmuCtIc9yUfpTo3h0Srn3C88wDQYJKoZIhvcNAQEL +BQAwHDEaMBgGA1UEAwwRQ29kZXIgU2VsZiBTaWduZWQwHhcNMjUwMzA3MDQ0NzQ2 +WhcNMzUwMzA1MDQ0NzQ2WjAcMRowGAYDVQQDDBFDb2RlciBTZWxmIFNpZ25lZDCC +ASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBAK6uYLDXy/C7TC4XaUbEmBiK +ICUAz/XDFB19aJrvG6LQYmdbR8sCV4uvoZAXYfribD+AGrysEDnpny1BzNORARLd +szBy0m1fOGbWqaBXg7Ot37rHWkU2iT2NEminHinx9UoJZLWxXsT1h1pnyJO4uzBW +jroRgYbOzkaccoDSWebDjnzAy4LA+Sfdzqm4RvJFD+5dhg/EXyJyLQApN22NWOTP +/8UXO0guUwSC+TGNFGRE6DkN96uX851HaCgrflz9zdLN5FSrSqvTsSStMTF9tHU3 +RlZFjTL+pVD6XSRMLn78xbch87sD3egaxaTvKd9Crx88GMwAnmrp8HqFAdUm4WUC +AwEAAaN1MHMwHQYDVR0OBBYEFDJM3fCTrkRAMNKQhoH3mWyLXY0JMB8GA1UdIwQY +MBaAFDJM3fCTrkRAMNKQhoH3mWyLXY0JMA8GA1UdEwEB/wQFMAMBAf8wCwYDVR0P +BAQDAgeAMBMGA1UdJQQMMAoGCCsGAQUFBwMDMA0GCSqGSIb3DQEBCwUAA4IBAQCU +xdNbMynw3YexqDjzHaKFGp4p8pXH9lXAa50e2UsYQ2C3HjBRkDQ0VGwqZaWWidMo +ZdL3TiKT3Gvd+eVEvZNVYAhOytevhztGa7RSIC54KbHM9pQiarQcCCpAVR3S1Ced +zGExk2iVL/o4TZKipvv+lj9b+FmavtoWq9kDuO0Suja0rzBfk5/UpATWbVOSGzY9 +1u0Rm2aIGgKxpOVPaxjD8JzJ+47r7Z6tSoYt4PScRy1kgf0VwKeUUdWLiGN4JxoS +dX/onACConyPh4gK1fbHuKaoxVkIV3nFRAk18AwF4jThqFDkCQmUwh7A0DSyiPiD +WeRW1iidZptmwzaOLnwz +-----END CERTIFICATE----- diff --git a/Tests.Vpn.Service/testdata/winres.json b/Tests.Vpn.Service/testdata/winres.json new file mode 100644 index 0000000..807151e --- /dev/null +++ b/Tests.Vpn.Service/testdata/winres.json @@ -0,0 +1,44 @@ +{ + "RT_MANIFEST": { + "#1": { + "0409": { + "identity": {}, + "description": "", + "minimum-os": "win7", + "execution-level": "", + "ui-access": false, + "auto-elevate": false, + "dpi-awareness": "system", + "disable-theming": false, + "disable-window-filtering": false, + "high-resolution-scrolling-aware": false, + "ultra-high-resolution-scrolling-aware": false, + "long-path-aware": false, + "printer-driver-isolation": false, + "gdi-scaling": false, + "segment-heap": false, + "use-common-controls-v6": false + } + } + }, + "RT_VERSION": { + "#1": { + "0409": { + "fixed": { + "file_version": "1.2.3.4", + "product_version": "1.2.3.4" + }, + "info": { + "0409": { + "FileDescription": "Coder", + "FileVersion": "1.2.3.4", + "LegalCopyright": "Copyright 2025 Coder Technologies Inc.", + "OriginalFilename": "coder.exe", + "ProductName": "Coder", + "ProductVersion": "1.2.3.4" + } + } + } + } + } +} \ No newline at end of file diff --git a/Vpn.Service/Downloader.cs b/Vpn.Service/Downloader.cs index 80b294f..a37a1ec 100644 --- a/Vpn.Service/Downloader.cs +++ b/Vpn.Service/Downloader.cs @@ -1,6 +1,8 @@ using System.Collections.Concurrent; using System.Diagnostics; +using System.Formats.Asn1; using System.Net; +using System.Runtime.CompilerServices; using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using Coder.Desktop.Vpn.Utilities; @@ -42,6 +44,11 @@ public class AuthenticodeDownloadValidator : IDownloadValidator { public static readonly AuthenticodeDownloadValidator Coder = new("Coder Technologies Inc."); + private static readonly Oid CertificatePoliciesOid = new("2.5.29.32", "Certificate Policies"); + + private static readonly Oid ExtendedValidationCodeSigningOid = + new("2.23.140.1.3", "Extended Validation (EV) code signing"); + private readonly string _expectedName; // ReSharper disable once ConvertToPrimaryConstructor @@ -60,30 +67,150 @@ public async Task ValidateAsync(string path, CancellationToken ct = default) if (fileSigInfo.State != SignatureState.SignedAndTrusted) throw new Exception( - $"File is not signed and trusted with an Authenticode signature: State={fileSigInfo.State}"); + $"File is not signed and trusted with an Authenticode signature: State={fileSigInfo.State}, StateReason={fileSigInfo.StateReason}"); // Coder will only use embedded signatures because we are downloading // individual binaries and not installers which can ship catalog files. if (fileSigInfo.Kind != SignatureKind.Embedded) throw new Exception($"File is not signed with an embedded Authenticode signature: Kind={fileSigInfo.Kind}"); - // TODO: check that it's an extended validation certificate + // We want to wrap any exception from IsExtendedValidationCertificate + // with a nicer error message, but we don't want to wrap the "false" + // result exception. + bool isExtendedValidation; + try + { + isExtendedValidation = IsExtendedValidationCertificate(fileSigInfo.SigningCertificate); + } + catch (Exception e) + { + throw new Exception( + "Could not check if file is signed with an Extended Validation Code Signing certificate", e); + } + + if (!isExtendedValidation) + throw new Exception( + $"File is not signed with an Extended Validation Code Signing certificate (missing policy {ExtendedValidationCodeSigningOid.Value} - {ExtendedValidationCodeSigningOid.FriendlyName})"); var actualName = fileSigInfo.SigningCertificate.GetNameInfo(X509NameType.SimpleName, false); if (actualName != _expectedName) throw new Exception( $"File is signed by an unexpected certificate: ExpectedName='{_expectedName}', ActualName='{actualName}'"); } + + /// + /// Checks if the given certificate is an Extended Validation Code Signing certificate. + /// + /// The cert to test + /// Whether the certificate is an Extended Validation Code Signing certificate + /// If the certificate extensions could not be parsed + private static bool IsExtendedValidationCertificate(X509Certificate2 cert) + { + ArgumentNullException.ThrowIfNull(cert); + + // RFC 5280 4.2: "A certificate MUST NOT include more than one instance + // of a particular extension." + var policyExtensions = cert.Extensions.Where(e => e.Oid?.Value == CertificatePoliciesOid.Value).ToList(); + if (policyExtensions.Count == 0) + return false; + Assert(policyExtensions.Count == 1, "certificate contains more than one CertificatePolicies extension"); + var certificatePoliciesExt = policyExtensions[0]; + + // RFC 5280 4.2.1.4 + // certificatePolicies ::= SEQUENCE SIZE (1..MAX) OF PolicyInformation + // + // PolicyInformation ::= SEQUENCE { + // policyIdentifier CertPolicyId, + // policyQualifiers SEQUENCE SIZE (1..MAX) OF PolicyQualifierInfo OPTIONAL + // } + try + { + AsnDecoder.ReadSequence(certificatePoliciesExt.RawData, AsnEncodingRules.DER, out var originalContentOffset, + out var contentLength, out var bytesConsumed); + Assert(bytesConsumed == certificatePoliciesExt.RawData.Length, "incorrect outer sequence length"); + Assert(originalContentOffset >= 0, "invalid outer sequence content offset"); + Assert(contentLength > 0, "invalid outer sequence content length"); + + var contentOffset = originalContentOffset; + var endOffset = originalContentOffset + contentLength; + Assert(endOffset <= certificatePoliciesExt.RawData.Length, "invalid outer sequence end offset"); + + // For each policy... + while (contentOffset < endOffset) + { + // Parse a sequence from [contentOffset:]. + var slice = certificatePoliciesExt.RawData.AsSpan(contentOffset, endOffset - contentOffset); + AsnDecoder.ReadSequence(slice, AsnEncodingRules.DER, out var innerContentOffset, + out var innerContentLength, out var innerBytesConsumed); + Assert(innerBytesConsumed <= slice.Length, "incorrect inner sequence length"); + Assert(innerContentOffset >= 0, "invalid inner sequence content offset"); + Assert(innerContentLength > 0, "invalid inner sequence content length"); + Assert(innerContentOffset + innerContentLength <= slice.Length, "invalid inner sequence end offset"); + + // Advance the outer offset by the consumed bytes. + contentOffset += innerBytesConsumed; + + // Parse the first value in the sequence as an Oid. + slice = slice.Slice(innerContentOffset, innerContentLength); + var oid = AsnDecoder.ReadObjectIdentifier(slice, AsnEncodingRules.DER, out var oidBytesConsumed); + Assert(oidBytesConsumed > 0, "invalid inner sequence OID length"); + Assert(oidBytesConsumed <= slice.Length, "invalid inner sequence OID length"); + if (oid == ExtendedValidationCodeSigningOid.Value) + return true; + + // We don't need to parse the rest of the data in the sequence, + // we can just move on to the next iteration. + } + } + catch (Exception e) + { + throw new Exception( + $"Could not parse {CertificatePoliciesOid.Value} ({CertificatePoliciesOid.FriendlyName}) extension in certificate", + e); + } + + return false; + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static void Assert(bool condition, string message) + { + if (!condition) + throw new Exception("Failed certificate parse assertion: " + message); + } } public class AssemblyVersionDownloadValidator : IDownloadValidator { - private readonly string _expectedAssemblyVersion; + private readonly int _expectedMajor; + private readonly int _expectedMinor; + private readonly int _expectedBuild; + private readonly int _expectedPrivate; + + private readonly Version _expectedVersion; // ReSharper disable once ConvertToPrimaryConstructor - public AssemblyVersionDownloadValidator(string expectedAssemblyVersion) + public AssemblyVersionDownloadValidator(int expectedMajor, int expectedMinor, int expectedBuild = -1, + int expectedPrivate = -1) { - _expectedAssemblyVersion = expectedAssemblyVersion; + _expectedMajor = expectedMajor; + _expectedMinor = expectedMinor; + _expectedBuild = expectedBuild < 0 ? -1 : expectedBuild; + _expectedPrivate = expectedPrivate < 0 ? -1 : expectedPrivate; + if (_expectedBuild == -1 && _expectedPrivate != -1) + throw new ArgumentException("Build must be set if Private is set", nameof(expectedPrivate)); + + // Unfortunately the Version constructor throws an exception if the + // build or revision is -1. You need to use the specific constructor + // with the correct number of parameters. + // + // This is only for error rendering purposes anyways. + if (_expectedBuild == -1) + _expectedVersion = new Version(_expectedMajor, _expectedMinor); + else if (_expectedPrivate == -1) + _expectedVersion = new Version(_expectedMajor, _expectedMinor, _expectedBuild); + else + _expectedVersion = new Version(_expectedMajor, _expectedMinor, _expectedBuild, _expectedPrivate); } public Task ValidateAsync(string path, CancellationToken ct = default) @@ -91,9 +218,16 @@ public Task ValidateAsync(string path, CancellationToken ct = default) var info = FileVersionInfo.GetVersionInfo(path); if (string.IsNullOrEmpty(info.ProductVersion)) throw new Exception("File ProductVersion is empty or null, was the binary compiled correctly?"); - if (info.ProductVersion != _expectedAssemblyVersion) + if (!Version.TryParse(info.ProductVersion, out var productVersion)) + throw new Exception($"File ProductVersion '{info.ProductVersion}' is not a valid version string"); + + // If the build or private are -1 on the expected version, they are ignored. + if (productVersion.Major != _expectedMajor || productVersion.Minor != _expectedMinor || + (_expectedBuild != -1 && productVersion.Build != _expectedBuild) || + (_expectedPrivate != -1 && productVersion.Revision != _expectedPrivate)) throw new Exception( - $"File ProductVersion is '{info.ProductVersion}', but expected '{_expectedAssemblyVersion}'"); + $"File ProductVersion does not match expected version: Actual='{info.ProductVersion}', Expected='{_expectedVersion}'"); + return Task.CompletedTask; } } @@ -103,12 +237,12 @@ public Task ValidateAsync(string path, CancellationToken ct = default) /// public class CombinationDownloadValidator : IDownloadValidator { - private readonly IDownloadValidator[] _validators; + private readonly List _validators; /// Validators to run public CombinationDownloadValidator(params IDownloadValidator[] validators) { - _validators = validators; + _validators = validators.ToList(); } public async Task ValidateAsync(string path, CancellationToken ct = default) @@ -116,6 +250,11 @@ public async Task ValidateAsync(string path, CancellationToken ct = default) foreach (var validator in _validators) await validator.ValidateAsync(path, ct); } + + public void Add(IDownloadValidator validator) + { + _validators.Add(validator); + } } /// diff --git a/Vpn.Service/Manager.cs b/Vpn.Service/Manager.cs index 93c08dd..9e70b31 100644 --- a/Vpn.Service/Manager.cs +++ b/Vpn.Service/Manager.cs @@ -417,15 +417,30 @@ private async Task DownloadTunnelBinaryAsync(string baseUrl, SemVersion expected _logger.LogInformation("Downloading VPN binary from '{url}' to '{DestinationPath}'", url, _config.TunnelBinaryPath); var req = new HttpRequestMessage(HttpMethod.Get, url); - var validators = new NullDownloadValidator(); - // TODO: re-enable when the binaries are signed and have versions - /* - var validators = new CombinationDownloadValidator( - AuthenticodeDownloadValidator.Coder, - new AssemblyVersionDownloadValidator( - $"{expectedVersion.Major}.{expectedVersion.Minor}.{expectedVersion.Patch}.0") - ); - */ + + var validators = new CombinationDownloadValidator(); + if (!string.IsNullOrEmpty(_config.TunnelBinarySignatureSigner)) + { + _logger.LogDebug("Adding Authenticode signature validator for signer '{Signer}'", + _config.TunnelBinarySignatureSigner); + validators.Add(new AuthenticodeDownloadValidator(_config.TunnelBinarySignatureSigner)); + } + else + { + _logger.LogDebug("Skipping Authenticode signature validation"); + } + + if (!_config.TunnelBinaryAllowVersionMismatch) + { + _logger.LogDebug("Adding version validator for version '{ExpectedVersion}'", expectedVersion); + validators.Add(new AssemblyVersionDownloadValidator((int)expectedVersion.Major, (int)expectedVersion.Minor, + (int)expectedVersion.Patch)); + } + else + { + _logger.LogDebug("Skipping tunnel binary version validation"); + } + var downloadTask = await _downloader.StartDownloadAsync(req, _config.TunnelBinaryPath, validators, ct); // TODO: monitor and report progress when we have a mechanism to do so diff --git a/Vpn.Service/ManagerConfig.cs b/Vpn.Service/ManagerConfig.cs index bfd7ff5..c7f8863 100644 --- a/Vpn.Service/ManagerConfig.cs +++ b/Vpn.Service/ManagerConfig.cs @@ -2,6 +2,11 @@ namespace Coder.Desktop.Vpn.Service; +// These values are the config option names used in the registry. Any option +// here can be configured with `(Debug)?Manager:OptionName` in the registry. +// +// They should not be changed without backwards compatibility considerations. +// If changed here, they should also be changed in the installer. public class ManagerConfig { [Required] @@ -11,4 +16,9 @@ public class ManagerConfig [Required] public string TunnelBinaryPath { get; set; } = @"C:\coder-vpn.exe"; [Required] public string LogFileLocation { get; set; } = @"C:\coder-desktop-service.log"; + + // If empty, signatures will not be verified. + [Required] public string TunnelBinarySignatureSigner { get; set; } = "Coder Technologies Inc."; + + [Required] public bool TunnelBinaryAllowVersionMismatch { get; set; } = false; } diff --git a/Vpn.Service/Program.cs b/Vpn.Service/Program.cs index e5a7d80..e22daa2 100644 --- a/Vpn.Service/Program.cs +++ b/Vpn.Service/Program.cs @@ -8,10 +8,17 @@ namespace Coder.Desktop.Vpn.Service; public static class Program { + // These values are the service name and the prefix on registry value names. + // They should not be changed without backwards compatibility + // considerations. If changed here, they should also be changed in the + // installer. #if !DEBUG private const string ServiceName = "Coder Desktop"; + private const string ManagerConfigSection = "Manager"; #else + // This value matches Create-Service.ps1. private const string ServiceName = "Coder Desktop (Debug)"; + private const string ManagerConfigSection = "DebugManager"; #endif private const string ConsoleOutputTemplate = @@ -61,7 +68,7 @@ private static async Task BuildAndRun(string[] args) // Options types (these get registered as IOptions singletons) builder.Services.AddOptions() - .Bind(builder.Configuration.GetSection("Manager")) + .Bind(builder.Configuration.GetSection(ManagerConfigSection)) .ValidateDataAnnotations() .PostConfigure(config => { diff --git a/Vpn.Service/Vpn.Service.csproj b/Vpn.Service/Vpn.Service.csproj index eed5386..acaeb3c 100644 --- a/Vpn.Service/Vpn.Service.csproj +++ b/Vpn.Service/Vpn.Service.csproj @@ -7,6 +7,7 @@ enable enable true + 13 CoderVpnService coder.ico @@ -20,7 +21,7 @@ - + diff --git a/scripts/Publish.ps1 b/scripts/Publish.ps1 index 08b30bd..248fbbc 100644 --- a/scripts/Publish.ps1 +++ b/scripts/Publish.ps1 @@ -122,6 +122,7 @@ if ($LASTEXITCODE -ne 0) { throw "Failed to build Vpn.Service" } $appPublishDir = Join-Path $buildPath "app" $msbuildBinary = & "${env:ProgramFiles(x86)}\Microsoft Visual Studio\Installer\vswhere.exe" -latest -requires Microsoft.Component.MSBuild -find MSBuild\**\Bin\MSBuild.exe if ($LASTEXITCODE -ne 0) { throw "Failed to find MSBuild" } +if (-not (Test-Path $msbuildBinary)) { throw "Failed to find MSBuild at $msbuildBinary" } & $msbuildBinary .\App\App.csproj /p:Configuration=Release /p:Platform=$arch /p:OutputPath=$appPublishDir if ($LASTEXITCODE -ne 0) { throw "Failed to build App" }