-
Notifications
You must be signed in to change notification settings - Fork 301
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
Enable installing binaries from multiple domains #2123
Comments
Frankly I'm not sure we should be applying any additional checks on the content of the releases.json file. If a) we have that file, and b) we can use a MS public signature to verify the contents (this would require publishing sidecar signature files) then we should just trust it. |
That is a good point. We should be doing something similar to: I can imagine two things:
Validating the specific domain seems like tighter coupling (as demonstrated) than we need/want. |
We can certainly simplify the check to be whether the domain ends in microsoft.com. We do validate the hashes on the files but we need to make sure that the releases.json is valid as well, do we have an easy place to get hashes for that as of right now? If that is on a web store as well, then that would need to be validated. |
I'm hoping to get checksums deployed beside each asset. I have a tracking issue for it. I'm hope that will happen this month. |
Extracting the domain from a string is actually a pretty hard problem with a lot of edge cases. I think I'm going to add |
StartsWith has a lot going for it w/rt simplicity. |
Resolves dotnet#2126 Resolves dotnet#2125 Workaround fix for dotnet#2123 We have a list of possible safe domains as part of our threat model which is needed to verify the source of truth when we run executables with elevated permission. In the past releases json only hosted downloads on `download.visualstudio.microsoft.com` but now it can be `builds.dotnet.microsoft.com`. I've added some more urls to our azure front door and other CDNs in the event that we decide to change to those. The long term fix for this would be if signatures are ever published for releases.json to verify those instead, but that is not ready yet and this change needs to go in.
* Support other possible future domains for the installer files. Resolves #2126 Resolves #2125 Workaround fix for #2123 We have a list of possible safe domains as part of our threat model which is needed to verify the source of truth when we run executables with elevated permission. In the past releases json only hosted downloads on `download.visualstudio.microsoft.com` but now it can be `builds.dotnet.microsoft.com`. I've added some more urls to our azure front door and other CDNs in the event that we decide to change to those. The long term fix for this would be if signatures are ever published for releases.json to verify those instead, but that is not ready yet and this change needs to go in. * Fix the test * Change URLs to possible likely download stores Co-authored-by: Rich Lander <[email protected]> --------- Co-authored-by: Rich Lander <[email protected]>
Related: #2121
Blocking: dotnet/core#9724
The following are safe domains:
download.visualstudio.microsoft.com
builds.dotnet.microsoft.com
This code needs to change:
vscode-dotnet-runtime/vscode-dotnet-runtime-library/src/Acquisition/GlobalInstallerResolver.ts
Lines 250 to 255 in 674f368
I'm hoping we can target this change for 2.2.8. I'd propose merging the PR above one week after the extension was released. After a week, we're likely well into the long tail.
The text was updated successfully, but these errors were encountered: