search

eclipse / openvsx

An open-source registry for VS Code extensions
https://open-vsx.org/
Eclipse Public License 2.0
1.26k stars 142 forks source link

Extension `.sigzip` assets does not match zip format #922

Closed jeanp413 closed 5 months ago

jeanp413 commented 5 months ago

VS Code added a simple format verification for the signature asset for extension, it has to be a zip file and contain a .signature.p7s (the content does not matter), the signature file returned from open-vsx fails this verification resulting in any extension failing to install.

We need to update the format of the signature asset so this verification step passes

cc @amvanbaren

jeanp413 commented 5 months ago

Not sure if this was since the beginning but now thw signature zip file has two files a manifest and the actual key with extension .p7s, this is validated by vscode

jeanp413 commented 5 months ago

i think this would take some time to fix, can we just remove the signature asset from the server response that way verification will be skiped by vscode automatically

amvanbaren commented 5 months ago

Hi @jeanp413, you mean node-ovsx-sign has changed?

mustard-mh commented 5 months ago

VS Code will verify extension signature since https://github.com/microsoft/vscode/pull/212722, code refer

The one downloaded from openvsx is not a zip file, but official one is and it includes a .signature.p7s file

cc @amvanbaren @jeanp413

amvanbaren commented 5 months ago

Microsoft's extension validation process was proprietary when I worked on extension signing. That's why @filiptronicek developed node-ovsx-sign to offer an open alternative.

Is the extension signing and validation process no longer proprietary?

mustard-mh commented 5 months ago

Microsoft's extension validation process was proprietary when I worked on extension signing. That's why @filiptronicek developed node-ovsx-sign to offer an open alternative.

Is the extension signing and validation process no longer proprietary?

Then make extension.isSigned=false seems to be our only choice?

mustard-mh commented 5 months ago

Do you think we can make a PR to vscode to default disable this feature? It makes no sense if signature needs to use their lib for oss clients @jeanp413

filiptronicek commented 5 months ago

@mustard-mh I think it makes sense to disable by default in the microsoft/vscode repository and finally implement https://github.com/filiptronicek/node-ovsx-sign in our fork and encourage other forks to do the same to not have to deal with just not verifying anything ever.

filiptronicek commented 5 months ago

There is a reference implementation I did a while back but it needs some more testing: https://github.com/gitpod-io/openvscode-server/pull/497/commits/d468864607b9904c126f2be4c448a9f112449b1d

jeanp413 commented 5 months ago

it's already disabled by default in vscode, if @vscode/vsce-sign is not present then no verification is done. The issue is that the format of the sigzip asset, vscode is verifying that it's a zip file and contains a .p7s file, it doesn't matter if it's empty, that's why it's an openvsx issue it should always aim to have the same format in all assets

cc @amvanbaren

amvanbaren commented 5 months ago

vscode is verifying that it's a zip file and contains a .p7s file

@jeanp413 vscode always does this, even if @vscode/vsce-sign is disabled?

jeanp413 commented 5 months ago

@jeanp413 vscode always does this, even if @vscode/vsce-sign is disabled?

yes

filiptronicek commented 5 months ago

I have a basic implementation here: https://github.com/filiptronicek/node-ovsx-sign/pull/16. Would be happy if anyone could take a look

filiptronicek commented 5 months ago

I've now published node-ovsx-sign@1.0.0 with the above PR's changes. Once we can get the same signature file structure produced from the server, we'll be able to land support for it at Gitpod.

For reference, the package still produces the signature, but the extension.sigzip file (Microsoft.VisualStudio.Services.VsixSignature in the Open VSX server) becomes a zip archive containing two files:

cc @amvanbaren

amvanbaren commented 5 months ago

Here's the server implementation: https://github.com/amvanbaren/openvsx/tree/feature/issue-922 I tried to test it with node-ovsx-sign@1.0.0 but it doesn't seem to do anything. The --verbose option didn't print any info either.

Steps to reproduce:

filiptronicek commented 5 months ago

@amvanbaren I have just tried testing against a Gitpod workspace running your changes:

  1. With automatically downloading the public key for the extension
  2. Providing the public key manually with -p)

It seems for both for me there's output and is behaving as expected. Can you get the CLI to give you any output for anything else?

OVSX_REGISTRY_URL=https://8080-gitpod-workspace-url node-ovsx-sign verify extension.vsix extension.sigzip --verbose
node-ovsx-sign verify extension.vsix extension.sigzip -p key.pub -v

Both of these provide an output similar to this:

Reading extension file
Getting extension id from extension manifest
Got extension metadata for vscodevim.vim
Loading public key
Reading signature archive
Verifying signature
Signature is valid

And when I try verifying with a different extension package, I get the following:

Reading extension file
Getting extension id from extension manifest
Got extension metadata for sburg.vscode-javascript-booster
Loading public key
Downloading file from https://8080-amvanbaren-openvsx-50djz8fpa91.ws.dogfood.gitpod.cloud/api/-/public-key/e7641871-82e7-461e-8c82-1a6f522a81c3 to /tmp/ovsx-XXXXXXv0N1Aa/public.pem
Reading signature archive
Verifying signature
Signature is not valid
amvanbaren commented 5 months ago

@filiptronicek Nice, I'll try again later.

jeanp413 commented 5 months ago

@amvanbaren there are a few more changes done in node-ovsx-sign

jeanp413 commented 5 months ago

@amvanbaren is it feasible to release to prod early next week as vscode will release the next stable version? Also this will need to run a background job to migrate all existing extension and this will take time right? Asking because vscodium has a large user base and better to warn them to disable validation on their fork before they do a release and users start complaining

filiptronicek commented 5 months ago

I've now published v1.1.0 of node-ovsx-sign which includes the changes mentioned above by @jeanp413.

filiptronicek commented 5 months ago

@amvanbaren if generating the .signature.manifest seems like too much work for now considering the VS Code release next week, I think we can skip implementing it, because we do not have any checks for the file yet. The only change then becomes renaming .signature to .signature.sig in your changes and we should be good to go 🚀 (also not considering DB migrations and such).

amvanbaren commented 5 months ago

Alright, I'll leave the .signature.manifest for now. I'll create a separate issue for it. The signature service has a renew function, so no extra DB migrations needed.

filiptronicek commented 5 months ago

FYI @amvanbaren both us (Gitpod) and VSCodium have disabled signature verification for this release, so we aren't rushing to get this done before 1.90 is released. We agreed internally that after your PR is raised and merged, we'd like to test signature verification throughout the month with our VS Code latest (Insiders) releases and turn it on again for everyone in time for the stable release in July.

amvanbaren commented 5 months ago

@filiptronicek Sounds good. Release v0.15.7 contains the new sigzip format. It'll be deployed to production once @kineticsquid has had a chance to review it.

filiptronicek commented 5 months ago

@amvanbaren I think we should be good to close this now. Thank you so much!

From the https://github.com/filiptronicek/node-ovsx-sign test suite:

PASS tests/ovsx-e2e.ts (5.331 s) extensionTest ✓ be able to download an extension archive, its signature and public key from Open VSX and verify the archive (4690 ms) Test Suites: 1 passed, 1 total Tests: 1 passed, 1 total Snapshots: 0 total Time: 5.351 s, estimated 10 s Ran all test suites matching /tests\/ovsx-e2e.ts/i.