Skip to content

chore: enable tunnel binary verification #36

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion App/App.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
</PropertyGroup>

<ItemGroup>
<Content Include="coder.ico" />
<Content Include="coder.ico" />
</ItemGroup>

<ItemGroup>
Expand Down
38 changes: 19 additions & 19 deletions Installer/Installer.csproj
Original file line number Diff line number Diff line change
@@ -1,24 +1,24 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<AssemblyName>Coder.Desktop.Installer</AssemblyName>
<RootNamespace>Coder.Desktop.Installer</RootNamespace>
<OutputType>Exe</OutputType>
<TargetFramework>net481</TargetFramework>
<LangVersion>13.0</LangVersion>
</PropertyGroup>
<PropertyGroup>
<AssemblyName>Coder.Desktop.Installer</AssemblyName>
<RootNamespace>Coder.Desktop.Installer</RootNamespace>
<OutputType>Exe</OutputType>
<TargetFramework>net481</TargetFramework>
<LangVersion>13.0</LangVersion>
</PropertyGroup>

<ItemGroup>
<None Remove="*.msi" />
<None Remove="*.exe" />
<None Remove="*.wxs" />
<None Remove="*.wixpdb" />
<None Remove="*.wixobj" />
</ItemGroup>
<ItemGroup>
<None Remove="*.msi" />
<None Remove="*.exe" />
<None Remove="*.wxs" />
<None Remove="*.wixpdb" />
<None Remove="*.wixobj" />
</ItemGroup>

<ItemGroup>
<PackageReference Include="WixSharp_wix4" Version="2.6.0" />
<PackageReference Include="WixSharp_wix4.bin" Version="2.6.0" />
<PackageReference Include="CommandLineParser" Version="2.9.1" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="WixSharp_wix4" Version="2.6.0" />
<PackageReference Include="WixSharp_wix4.bin" Version="2.6.0" />
<PackageReference Include="CommandLineParser" Version="2.9.1" />
</ItemGroup>
</Project>
8 changes: 6 additions & 2 deletions Installer/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm understanding this correctly, the Manager: prefix corresponds to the section name (ManagerConfigSection) we bind in Program.cs:

        builder.Services.AddOptions<ManagerConfig>()
            .Bind(builder.Configuration.GetSection(ManagerConfigSection))

But that means we've got this string key coupling between these two sections of the code, which feels fragile if we were to change the name. Is there a way they could be based on the same constant?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we can without creating a new library package or adding this value to a package where it doesn't really belong like Vpn.

These values should never be changed anyways without some consideration for backwards compatibility. I can add comments to both of these places saying that it's not to be changed.


// Note: most of this control panel info will not be visible as this
// package is usually hidden in favor of the bootstrapper showing
Expand Down
29 changes: 29 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -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.
134 changes: 118 additions & 16 deletions Tests.Vpn.Service/DownloaderTest.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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<Exception>(() =>
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<Exception>(() =>
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<Exception>(() =>
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<Exception>(() =>
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<Exception>(() =>
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)}");
}
}
}

Expand All @@ -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<Exception>(() =>
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<Exception>(() =>
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<Exception>(() =>
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);
}
}

Expand Down
30 changes: 30 additions & 0 deletions Tests.Vpn.Service/Tests.Vpn.Service.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,36 @@
<IsTestProject>true</IsTestProject>
</PropertyGroup>

<ItemGroup>
<None Update="testdata\hello.exe">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="testdata\hello-invalid-version.exe">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="testdata\hello-self-signed.exe">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="testdata\hello-versioned-signed.exe">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="testdata\coder-ev.crt">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="testdata\google-llc-ev.crt">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="testdata\mozilla-corporation.crt">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="testdata\self-signed.crt">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
<None Update="testdata\self-signed-ev.crt">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</None>
</ItemGroup>

<ItemGroup>
<PackageReference Include="coverlet.collector" Version="6.0.4">
<PrivateAssets>all</PrivateAssets>
Expand Down
2 changes: 2 additions & 0 deletions Tests.Vpn.Service/testdata/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
*.go
*.pfx
68 changes: 68 additions & 0 deletions Tests.Vpn.Service/testdata/Build-Assets.ps1
Original file line number Diff line number Diff line change
@@ -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!"
Loading
Loading