-
-
Notifications
You must be signed in to change notification settings - Fork 19.5k
vscode-extensions: add optional Marketplace signature verification #482821
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| # Verify VSIX signature using VS Code's bundled vsce-sign binary. | ||
| # Thin wrapper around verify-vsix-signature.sh (the shared verifier) that runs | ||
| # in preUnpackHooks, before the VSIX is unpacked. At that point $src points to | ||
| # the downloaded VSIX, i.e. the exact bytes Microsoft signed. | ||
| # | ||
| # Opt-in: this hook is only attached when `verifySignature` is enabled (per | ||
| # extension, or globally via nixpkgs config `vscodeExtensions.verifySignature`) | ||
| # and vsce-sign is available (see vscode-utils.nix), so the default build stays | ||
| # free and buildable on Hydra / with vscodium. | ||
|
|
||
| _verifyVsixSignaturePreUnpack() { | ||
| # Defensive: the hook is only attached when both are set, but guard anyway. | ||
| if [[ -z "${signatureArchive:-}" || -z "${src:-}" ]]; then | ||
| echo "WARNING: missing \$src or \$signatureArchive, skipping signature verification" >&2 | ||
| return 0 | ||
| fi | ||
|
|
||
| @verifyScript@ "@vsceSign@" "$src" "$signatureArchive" | ||
| } | ||
|
|
||
| # Run signature verification before unpacking | ||
| preUnpackHooks+=(_verifyVsixSignaturePreUnpack) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| #!/usr/bin/env bash | ||
| # Verify a VSIX package's VS Code Marketplace signature using Microsoft's | ||
| # vsce-sign binary. | ||
| # | ||
| # Single source of truth for signature verification, shared between the | ||
| # build-time setup hook (verify-vsix-signature-setup-hook.sh) and the update | ||
| # script (vscode_extension_update.py), so both honour the same verifier and | ||
| # exit-code semantics. Takes everything as arguments so it has no build-time | ||
| # or Nix dependencies and can be invoked directly (e.g. `bash this.sh ...`). | ||
| # | ||
| # Usage: verify-vsix-signature.sh <vsce-sign> <vsix> <signature-archive> | ||
| set -uo pipefail | ||
|
|
||
| vsceSign="${1:?vsce-sign path required}" | ||
| vsix="${2:?vsix path required}" | ||
| signatureArchive="${3:?signature archive path required}" | ||
|
|
||
| if [[ ! -x "$vsceSign" ]]; then | ||
| echo "ERROR: vsce-sign not found at $vsceSign" >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "Verifying VSIX signature..." | ||
| exitCode=0 | ||
| "$vsceSign" verify --package "$vsix" --signaturearchive "$signatureArchive" || exitCode=$? | ||
|
|
||
| if [[ $exitCode -eq 0 ]]; then | ||
| echo "Signature verification: PASSED" | ||
| exit 0 | ||
| fi | ||
|
|
||
| echo "Signature verification: FAILED (exit code: $exitCode)" >&2 | ||
| case $exitCode in | ||
| 30) echo "ERROR: Package integrity check failed - contents don't match signature" >&2 ;; | ||
| 31) echo "ERROR: Invalid signature format" >&2 ;; | ||
| 35) echo "ERROR: Package has been tampered with" >&2 ;; | ||
| 36) echo "ERROR: Certificate is not trusted" >&2 ;; | ||
| 37) echo "ERROR: Certificate has been revoked" >&2 ;; | ||
| *) echo "ERROR: Unknown verification error" >&2 ;; | ||
| esac | ||
| exit "$exitCode" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| { | ||
| config, | ||
| stdenv, | ||
| lib, | ||
| buildEnv, | ||
|
|
@@ -8,7 +9,7 @@ | |
| buildPackages, | ||
| unzip, | ||
| makeSetupHook, | ||
| writeScript, | ||
| writeShellScript, | ||
| jq, | ||
| vscode-extension-update-script, | ||
| }: | ||
|
|
@@ -20,6 +21,33 @@ let | |
| }; | ||
| meta.license = lib.licenses.mit; | ||
| } ./unpack-vsix-setup-hook.sh; | ||
|
|
||
| # Path to Microsoft's vsce-sign binary bundled with the (unfree) vscode. | ||
| # Exported below so the update script can verify signatures at update time | ||
| # using the same verifier the build-time hook uses. | ||
| vsceSignExe = | ||
| if stdenv.hostPlatform.isDarwin then | ||
| "${vscode}/Applications/${vscode.longName}.app/Contents/Resources/app/node_modules/@vscode/vsce-sign/bin/vsce-sign" | ||
| else | ||
| "${vscode}/lib/vscode/resources/app/node_modules/@vscode/vsce-sign/bin/vsce-sign"; | ||
|
|
||
| # Single source of truth for the verification logic, shared with the update | ||
| # script (pkgs/by-name/vs/vscode-extension-update/vscode_extension_update.py). | ||
| verifyVsixSignatureScript = writeShellScript "verify-vsix-signature" ( | ||
| builtins.readFile ./verify-vsix-signature.sh | ||
| ); | ||
|
|
||
| verifyVsixSignatureSetupHook = makeSetupHook { | ||
| name = "verify-vsix-signature-setup-hook"; | ||
| substitutions = { | ||
| vsceSign = vsceSignExe; | ||
| verifyScript = verifyVsixSignatureScript; | ||
| }; | ||
| } ./verify-vsix-signature-setup-hook.sh; | ||
|
|
||
| # Global default for `verifySignature`, set via nixpkgs config: | ||
| # `vscodeExtensions.verifySignature = true` verifies every signed extension. | ||
| globalVerifySignature = config.vscodeExtensions.verifySignature or false; | ||
| buildVscodeExtension = lib.extendMkDerivation { | ||
| constructDrv = stdenv.mkDerivation; | ||
| excludeDrvArgNames = [ | ||
|
|
@@ -73,7 +101,36 @@ let | |
| # This cannot be removed, it is used by some extensions. | ||
| installPrefix = "share/vscode/extensions/${vscodeExtUniqueId}"; | ||
|
|
||
| nativeBuildInputs = [ unpackVsixSetupHook ] ++ nativeBuildInputs; | ||
| nativeBuildInputs = [ | ||
| unpackVsixSetupHook | ||
| ] | ||
| # Opt-in: verification pulls the unfree vsce-sign (from vscode) into the | ||
| # build closure, so it is off by default to keep signed extensions free | ||
| # and buildable on Hydra / with vscodium. Enable with `verifySignature`. | ||
| ++ lib.optional ( | ||
| (finalAttrs.verifySignature or globalVerifySignature) | ||
| && (finalAttrs.signatureArchive or null) != null | ||
| && vscode.hasVsceSign | ||
| ) verifyVsixSignatureSetupHook | ||
| ++ nativeBuildInputs; | ||
|
|
||
| # vsce-sign validates Microsoft's certificate chain via Apple's | ||
| # Security.framework, which reads the system keychain and MDS store. | ||
| # Grant those reads under Darwin's relaxed sandbox. | ||
| sandboxProfile = | ||
| lib.optionalString | ||
| ( | ||
| (finalAttrs.verifySignature or globalVerifySignature) | ||
| && (finalAttrs.signatureArchive or null) != null | ||
| && stdenv.hostPlatform.isDarwin | ||
| && vscode.hasVsceSign | ||
| ) | ||
| '' | ||
| (allow file-read* | ||
| (literal "/Library/Keychains/System.keychain") | ||
| (literal "/private/var/db/mds/system/mdsDirectory.db") | ||
| (literal "/private/var/db/mds/system/mdsObject.db")) | ||
| ''; | ||
|
|
||
| installPhase = | ||
| args.installPhase or '' | ||
|
|
@@ -87,9 +144,36 @@ let | |
| }; | ||
| }; | ||
|
|
||
| mktplcAssetBaseUrl = | ||
| { | ||
| publisher, | ||
| name, | ||
| version, | ||
| ... | ||
| }: | ||
| "https://${publisher}.gallery.vsassets.io/_apis/public/gallery/publisher/${publisher}/extension/${name}/${version}/assetbyname"; | ||
|
|
||
| fetchVsixFromVscodeMarketplace = | ||
| mktplcExtRef: fetchurl (import ./mktplcExtRefToFetchArgs.nix mktplcExtRef); | ||
|
|
||
| # Fetch signature archive for cryptographic verification | ||
| fetchSignatureFromVscodeMarketplace = | ||
| mktplcExtRef: | ||
| let | ||
| inherit (mktplcExtRef) publisher name; | ||
| arch = mktplcExtRef.arch or ""; | ||
| archurl = if arch == "" then "" else "?targetPlatform=${arch}"; | ||
| signatureHash = mktplcExtRef.signatureHash or ""; | ||
| in | ||
| if signatureHash != "" then | ||
| fetchurl { | ||
| url = "${mktplcAssetBaseUrl mktplcExtRef}/Microsoft.VisualStudio.Services.VsixSignature${archurl}"; | ||
| name = "${publisher}-${name}.sigzip"; | ||
| hash = signatureHash; | ||
| } | ||
| else | ||
| null; | ||
|
Comment on lines
+159
to
+175
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This seems extremely similar to the existing I'd personally consolidate something like: {
publisher,
name,
version,
arch ? "",
sha256 ? "",
hash ? "",
signatureHash ? "",
}@mktplcExtRef:
let
archurl = (if arch == "" then "" else "?targetPlatform=${arch}");
baseUrl = "https://${publisher}.gallery.vsassets.io/_apis/public/gallery/publisher/${publisher}/extension/${name}/${version}/assetbyname";
in
fetchurl {
# VSIX package fetch arguments (for fetchurl)
url = "${baseUrl}/Microsoft.VisualStudio.Services.VSIXPackage${archurl}";
inherit sha256 hash;
# The `*.vsix` file is in the end a simple zip file. Force it using .vsix extension
# so that existing `unpackVsixSetupHook` hooks takes care of the unpacking.
name = "${publisher}-${name}.vsix";
passthru = {
${if signatureHash == "" then null else "signature"} = fetchurl {
url = "${baseUrl}/Microsoft.VisualStudio.Services.VsixSignature${archurl}";
name = "${publisher}-${name}.sigzip";
hash = signatureHash;
};
};
}Removing the overly-abstracted
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could probably be improved. I'm fairly new with nix so I probably didn't implement this as cleanly as I probably should have. I will attempt to clean this up some when I address your other findings.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MattSturgeon I made a small consolidation around the URL but avoided a larger refactor to avoid impacting vscodeExts2nix.nix which also uses this function. Switching the file to return a fetchurl derivation would require updating vscodeExts2nix.nix's call site. Which should probably go in another PR that is more focused on refactoring these patterns. Although I'm not opposed to doing that here if that is the general consensus. For the dedup itself I extracted a small mktplcAssetBaseUrl helper in vscode-utils.nix: mktplcAssetBaseUrl =
{ publisher, name, version, ... }:
"https://${publisher}.gallery.vsassets.io/_apis/public/gallery/publisher/${publisher}/extension/${name}/${version}/assetbyname";fetchSignatureFromVscodeMarketplace now uses it instead of inlining the literal. The duplication shrinks without moving the public shape of mktplcExtRefToFetchArgs.nix
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ideally we'd consolidate these two functions, but I agree it is beyond the scope of this PR. |
||
|
|
||
| buildVscodeMarketplaceExtension = lib.extendMkDerivation { | ||
| constructDrv = buildVscodeExtension; | ||
| excludeDrvArgNames = [ | ||
|
|
@@ -114,6 +198,7 @@ let | |
| vscodeExtPublisher = mktplcRef.publisher; | ||
| vscodeExtName = mktplcRef.name; | ||
| vscodeExtUniqueId = "${mktplcRef.publisher}.${mktplcRef.name}"; | ||
| signatureArchive = fetchSignatureFromVscodeMarketplace mktplcRef; | ||
| }; | ||
| }; | ||
|
|
||
|
|
@@ -124,6 +209,7 @@ let | |
| "sha256" | ||
| "hash" | ||
| "arch" | ||
| "signatureHash" | ||
| ]; | ||
|
|
||
| mktplcExtRefToExtDrv = | ||
|
|
@@ -194,6 +280,7 @@ let | |
| in | ||
| { | ||
| inherit | ||
| vsceSignExe | ||
| fetchVsixFromVscodeMarketplace | ||
| buildVscodeExtension | ||
| buildVscodeMarketplaceExtension | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -188,6 +188,7 @@ stdenv.mkDerivation ( | |
| passthru = { | ||
| inherit | ||
| executableName | ||
| hasVsceSign | ||
| longName | ||
| tests | ||
| updateScript | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.