chore: staple empty signatures (#84)

* chore: staple empty signatures

Since signatures are not checked, or not valid (static public key
in VSCode), then signing is just incorrect.

Author: Emyrk

Diff for: cli/server.go

@@ -15,27 +15,21 @@ import (
 
 	"cdr.dev/slog"
 	"cdr.dev/slog/sloggers/sloghuman"
-	"github.com/coder/code-marketplace/extensionsign"
-
 	"github.com/coder/code-marketplace/api"
 	"github.com/coder/code-marketplace/database"
 	"github.com/coder/code-marketplace/storage"
 )
 
 func serverFlags() (addFlags func(cmd *cobra.Command), opts *storage.Options) {
 	opts = &storage.Options{}
-	var sign bool
 	return func(cmd *cobra.Command) {
 		cmd.Flags().StringVar(&opts.ExtDir, "extensions-dir", "", "The path to extensions.")
 		cmd.Flags().StringVar(&opts.Artifactory, "artifactory", "", "Artifactory server URL.")
 		cmd.Flags().StringVar(&opts.Repo, "repo", "", "Artifactory repository.")
-		cmd.Flags().BoolVar(&sign, "sign", false, "Sign extensions.")
-		_ = cmd.Flags().MarkHidden("sign") // This flag needs to import a key, not just be a bool
-		cmd.Flags().BoolVar(&opts.SaveSigZips, "save-sigs", false, "Save signed extensions to disk for debugging.")
-		_ = cmd.Flags().MarkHidden("save-sigs")
 
 		if cmd.Use == "server" {
 			// Server only flags
+			cmd.Flags().BoolVar(&opts.IncludeEmptySignatures, "sign", false, "Includes an empty signature for all extensions.")
 			cmd.Flags().DurationVar(&opts.ListCacheDuration, "list-cache-duration", time.Minute, "The duration of the extension cache.")
 		}
 
@@ -56,9 +50,6 @@ func serverFlags() (addFlags func(cmd *cobra.Command), opts *storage.Options) {
 			if before != nil {
 				return before(cmd, args)
 			}
-			if sign { // TODO: Remove this for an actual key import
-				opts.Signer, _ = extensionsign.GenerateKey()
-			}
 			return nil
 		}
 	}, opts

Diff for: extensionsign/doc.go

@@ -1,2 +1,2 @@
-// Package extensionsign is a Go implementation of https://github.com/filiptronicek/node-ovsx-sign
+// Package extensionsign provides utilities for working with extension signatures.
 package extensionsign

Diff for: extensionsign/key.go

@@ -3,8 +3,6 @@ package extensionsign
 import (
 	"archive/zip"
 	"bytes"
-	"crypto"
-	"crypto/rand"
 	"encoding/json"
 
 	"golang.org/x/xerrors"
@@ -27,8 +25,7 @@ func ExtractSignatureManifest(zip []byte) (SignatureManifest, error) {
 	return manifest, nil
 }
 
-// SignAndZipManifest signs a manifest and zips it up
-func SignAndZipManifest(secret crypto.Signer, vsixData []byte, manifest json.RawMessage) ([]byte, error) {
+func IncludeEmptySignature() ([]byte, error) {
 	var buf bytes.Buffer
 	w := zip.NewWriter(&buf)
 
@@ -37,7 +34,7 @@ func SignAndZipManifest(secret crypto.Signer, vsixData []byte, manifest json.Raw
 		return nil, xerrors.Errorf("create manifest: %w", err)
 	}
 
-	_, err = manFile.Write(manifest)
+	_, err = manFile.Write([]byte{})
 	if err != nil {
 		return nil, xerrors.Errorf("write manifest: %w", err)
 	}
@@ -48,22 +45,6 @@ func SignAndZipManifest(secret crypto.Signer, vsixData []byte, manifest json.Raw
 		return nil, xerrors.Errorf("create empty p7s signature: %w", err)
 	}
 
-	// Actual sig
-	sigFile, err := w.Create(".signature.sig")
-	if err != nil {
-		return nil, xerrors.Errorf("create signature: %w", err)
-	}
-
-	signature, err := secret.Sign(rand.Reader, vsixData, crypto.Hash(0))
-	if err != nil {
-		return nil, xerrors.Errorf("sign: %w", err)
-	}
-
-	_, err = sigFile.Write(signature)
-	if err != nil {
-		return nil, xerrors.Errorf("write signature: %w", err)
-	}
-
 	err = w.Close()
 	if err != nil {
 		return nil, xerrors.Errorf("close zip: %w", err)

Diff for: extensionsign/sigzip.go

@@ -2,9 +2,6 @@ package storage
 
 import (
 	"context"
-	"crypto"
-	"encoding/json"
-	"io"
 	"io/fs"
 	"path/filepath"
 	"strings"
@@ -30,64 +27,24 @@ func SignatureZipFilename(manifest *VSIXManifest) string {
 
 // Signature is a storage wrapper that can sign extensions on demand.
 type Signature struct {
-	// Signer if provided, will be used to sign extensions. If not provided,
-	// no extensions will be signed.
-	Signer crypto.Signer
-	Logger slog.Logger
-	// SaveSigZips is a flag that will save the signed extension to disk.
-	// This is useful for debugging, but the server will never use this file.
-	saveSigZips bool
+	Logger                 slog.Logger
+	IncludeEmptySignatures bool
 	Storage
 }
 
-func NewSignatureStorage(logger slog.Logger, signer crypto.Signer, s Storage) *Signature {
-	return &Signature{
-		Signer:  signer,
-		Storage: s,
+func NewSignatureStorage(logger slog.Logger, includeEmptySignatures bool, s Storage) *Signature {
+	if includeEmptySignatures {
+		logger.Info(context.Background(), "Signature storage enabled, if using VSCode on Windows, this will not work.")
 	}
-}
-
-func (s *Signature) SaveSigZips() {
-	if !s.saveSigZips {
-		s.Logger.Info(context.Background(), "extension signatures will be saved to disk, do not use this in production")
+	return &Signature{
+		Logger:                 logger,
+		IncludeEmptySignatures: includeEmptySignatures,
+		Storage:                s,
 	}
-	s.saveSigZips = true
 }
 
 func (s *Signature) SigningEnabled() bool {
-	return s.Signer != nil
-}
-
-// AddExtension includes the signature manifest of the vsix. Signing happens on
-// demand, so leave the manifest unsigned. This is safe to do even if
-// 'signExtensions' is disabled, as these files lay dormant until signed.
-func (s *Signature) AddExtension(ctx context.Context, manifest *VSIXManifest, vsix []byte, extra ...File) (string, error) {
-	sigManifest, err := extensionsign.GenerateSignatureManifest(vsix)
-	if err != nil {
-		return "", xerrors.Errorf("generate signature manifest: %w", err)
-	}
-
-	sigManifestJSON, err := json.Marshal(sigManifest)
-	if err != nil {
-		return "", xerrors.Errorf("encode signature manifest: %w", err)
-	}
-
-	if s.SigningEnabled() && s.saveSigZips {
-		signed, err := s.SigZip(ctx, vsix, sigManifestJSON)
-		if err != nil {
-			s.Logger.Error(ctx, "signing manifest", slog.Error(err))
-			return "", xerrors.Errorf("sign and zip manifest: %w", err)
-		}
-		extra = append(extra, File{
-			RelativePath: SignatureZipFilename(manifest),
-			Content:      signed,
-		})
-	}
-
-	return s.Storage.AddExtension(ctx, manifest, vsix, append(extra, File{
-		RelativePath: sigManifestName,
-		Content:      sigManifestJSON,
-	})...)
+	return s.IncludeEmptySignatures
 }
 
 func (s *Signature) Manifest(ctx context.Context, publisher, name string, version Version) (*VSIXManifest, error) {
@@ -116,60 +73,22 @@ func (s *Signature) Manifest(ctx context.Context, publisher, name string, versio
 // Open will intercept requests for signed extensions payload.
 // It does this by looking for 'SigzipFileExtension' or p7s.sig.
 //
-// The signed payload and signing process is taken from:
-// https://github.com/filiptronicek/node-ovsx-sign
+// The signed payload is completely empty. Nothing it actually signed.
 //
 // Some notes:
 //
 //   - VSCodium requires a signature to exist, but it does appear to actually read
 //     the signature. Meaning the signature could be empty, incorrect, or a
 //     picture of cat and it would work. There is no signature verification.
 //
-//   - VSCode requires a signature payload to exist, but the context appear
-//     to be somewhat optional.
-//     Following another open source implementation, it appears the '.signature.p7s'
-//     file must exist, but it can be empty.
-//     The signature is stored in a '.signature.sig' file, although it is unclear
-//     is VSCode ever reads this file.
-//     TODO: Properly implement the p7s file, and diverge from the other open
-//     source implementation. Ideally this marketplace would match Microsoft's
-//     marketplace API.
+//   - VSCode requires a signature payload to exist, but the content is optional
+//     for linux users.
+//     For windows users, the signature must be valid, and this implementation
+//     will not work.
 func (s *Signature) Open(ctx context.Context, fp string) (fs.File, error) {
 	if s.SigningEnabled() && strings.HasSuffix(filepath.Base(fp), SigzipFileExtension) {
-		base := filepath.Base(fp)
-		vsixPath := strings.TrimSuffix(base, SigzipFileExtension)
-
-		// hijack this request, sign the sig manifest
-		manifest, err := s.Storage.Open(ctx, filepath.Join(filepath.Dir(fp), sigManifestName))
-		if err != nil {
-			// If this file is missing, it means the extension was added before
-			// signatures were handled by the marketplace.
-			// TODO: Generate the sig manifest payload and insert it?
-			return nil, xerrors.Errorf("open signature manifest: %w", err)
-		}
-		defer manifest.Close()
-
-		manifestData, err := io.ReadAll(manifest)
-		if err != nil {
-			return nil, xerrors.Errorf("read signature manifest: %w", err)
-		}
-
-		vsix, err := s.Storage.Open(ctx, filepath.Join(filepath.Dir(fp), vsixPath+".vsix"))
-		if err != nil {
-			// If this file is missing, it means the extension was added before
-			// signatures were handled by the marketplace.
-			// TODO: Generate the sig manifest payload and insert it?
-			return nil, xerrors.Errorf("open signature manifest: %w", err)
-		}
-		defer vsix.Close()
-
-		vsixData, err := io.ReadAll(vsix)
-		if err != nil {
-			return nil, xerrors.Errorf("read signature manifest: %w", err)
-		}
-
-		// TODO: Fetch the VSIX payload from the storage
-		signed, err := s.SigZip(ctx, vsixData, manifestData)
+		// hijack this request, return an empty signature payload
+		signed, err := extensionsign.IncludeEmptySignature()
 		if err != nil {
 			return nil, xerrors.Errorf("sign and zip manifest: %w", err)
 		}
@@ -181,12 +100,3 @@ func (s *Signature) Open(ctx context.Context, fp string) (fs.File, error) {
 
 	return s.Storage.Open(ctx, fp)
 }
-
-func (s *Signature) SigZip(ctx context.Context, vsix []byte, sigManifest []byte) ([]byte, error) {
-	signed, err := extensionsign.SignAndZipManifest(s.Signer, vsix, sigManifest)
-	if err != nil {
-		s.Logger.Error(ctx, "signing manifest", slog.Error(err))
-		return nil, xerrors.Errorf("sign and zip manifest: %w", err)
-	}
-	return signed, nil
-}

Diff for: storage/signature.go

@@ -1,11 +1,9 @@
 package storage_test
 
 import (
-	"crypto"
 	"testing"
 
 	"cdr.dev/slog"
-	"github.com/coder/code-marketplace/extensionsign"
 	"github.com/coder/code-marketplace/storage"
 )
 
@@ -21,10 +19,10 @@ func expectSignature(manifest *storage.VSIXManifest) {
 func signed(signer bool, factory func(t *testing.T) testStorage) func(t *testing.T) testStorage {
 	return func(t *testing.T) testStorage {
 		st := factory(t)
-		var key crypto.Signer
+		key := false
 		var exp func(*storage.VSIXManifest)
 		if signer {
-			key, _ = extensionsign.GenerateKey()
+			key = true
 			exp = expectSignature
 		}
 

Diff for: storage/signature_test.go

@@ -2,7 +2,6 @@ package storage
 
 import (
 	"context"
-	"crypto"
 	"encoding/json"
 	"encoding/xml"
 	"fmt"
@@ -128,13 +127,12 @@ type VSIXAsset struct {
 }
 
 type Options struct {
-	Signer            crypto.Signer
-	Artifactory       string
-	ExtDir            string
-	Repo              string
-	SaveSigZips       bool
-	Logger            slog.Logger
-	ListCacheDuration time.Duration
+	IncludeEmptySignatures bool
+	Artifactory            string
+	ExtDir                 string
+	Repo                   string
+	Logger                 slog.Logger
+	ListCacheDuration      time.Duration
 }
 
 type extension struct {
@@ -294,10 +292,7 @@ func NewStorage(ctx context.Context, options *Options) (Storage, error) {
 		return nil, err
 	}
 
-	signingStorage := NewSignatureStorage(options.Logger, options.Signer, store)
-	if options.SaveSigZips {
-		signingStorage.SaveSigZips()
-	}
+	signingStorage := NewSignatureStorage(options.Logger, options.IncludeEmptySignatures, store)
 
 	return signingStorage, nil
 }