chore: enable tunnel binary verification #36
Conversation
| } | ||
|
|
||
| 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)) |
There was a problem hiding this comment.
Instead of getting this as a string and then parsing it, you should be able to directly access these numbers as info.ProductMajorPart, info.ProductMinorPart, info.ProductBuildPart and info.ProductPrivatePart (tee-hee 🍆 )
There was a problem hiding this comment.
These values seem to be set even if the version couldn't be fully parsed, e.g. invalid version 1-2-3-4 gets parsed as (1, 0, 0, 0) in these values. I think it's better to just parse the string version, which will fail immediately if it's not the correct format
There was a problem hiding this comment.
Apparently, there are 4 different "versions" that can be stored in a file. There is a file version and a product version and each can be stored in binary as 4 16-bit fields, and as a string.
https://learn.microsoft.com/en-us/windows/win32/menurc/versioninfo-resource
What you are examining here is the product version string. Things like info.ProductMajorPart examine the product version binary. I'm guessing that go-winres attempts to parse the version string, and if successful, also sets the binary values. Is this the tool we use for the real go builds? If so, then I agree it's appropriate to use the product version string and parse it like this.
There was a problem hiding this comment.
Yeah we use go-winres in the real builds
| @"[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")); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
TODO:
Closes #41
Closes #45