From 54c6c5b05bfaa3475dbbd63dca9f618d3ba627d8 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 13 Dec 2024 11:28:02 -0600 Subject: [PATCH 1/4] chore: sign the vsix package, not the manifest --- cli/server.go | 2 + extensionsign/sigzip.go | 4 +- storage/signature.go | 79 +++++++++++++++++++++++++++++++++------ storage/signature_test.go | 2 +- storage/storage.go | 11 +++++- 5 files changed, 83 insertions(+), 15 deletions(-) diff --git a/cli/server.go b/cli/server.go index 8cfb7ea..7a63fe0 100644 --- a/cli/server.go +++ b/cli/server.go @@ -31,6 +31,8 @@ func serverFlags() (addFlags func(cmd *cobra.Command), opts *storage.Options) { 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 diff --git a/extensionsign/sigzip.go b/extensionsign/sigzip.go index c6948d1..5d9f536 100644 --- a/extensionsign/sigzip.go +++ b/extensionsign/sigzip.go @@ -28,7 +28,7 @@ func ExtractSignatureManifest(zip []byte) (SignatureManifest, error) { } // SignAndZipManifest signs a manifest and zips it up -func SignAndZipManifest(secret crypto.Signer, manifest json.RawMessage) ([]byte, error) { +func SignAndZipManifest(secret crypto.Signer, vsixData []byte, manifest json.RawMessage) ([]byte, error) { var buf bytes.Buffer w := zip.NewWriter(&buf) @@ -54,7 +54,7 @@ func SignAndZipManifest(secret crypto.Signer, manifest json.RawMessage) ([]byte, return nil, xerrors.Errorf("create signature: %w", err) } - signature, err := secret.Sign(rand.Reader, manifest, crypto.Hash(0)) + signature, err := secret.Sign(rand.Reader, vsixData, crypto.Hash(0)) if err != nil { return nil, xerrors.Errorf("sign: %w", err) } diff --git a/storage/signature.go b/storage/signature.go index 4bc3afd..ce917bc 100644 --- a/storage/signature.go +++ b/storage/signature.go @@ -7,35 +7,53 @@ import ( "io" "io/fs" "path/filepath" + "strings" "github.com/spf13/afero/mem" "golang.org/x/xerrors" + "cdr.dev/slog" + "github.com/coder/code-marketplace/extensionsign" ) var _ Storage = (*Signature)(nil) const ( - SigzipFilename = "extension.sigzip" - sigManifestName = ".signature.manifest" + SigzipFileExtension = ".signature.p7s" + sigManifestName = ".signature.manifest" ) +func SignatureZipFilename(manifest *VSIXManifest) string { + return ExtensionVSIXNameFromManifest(manifest) + SigzipFileExtension +} + // 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 Storage } -func NewSignatureStorage(signer crypto.Signer, s Storage) *Signature { +func NewSignatureStorage(logger slog.Logger, signer crypto.Signer, s Storage) *Signature { return &Signature{ Signer: signer, Storage: s, } } +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") + } + s.saveSigZips = true +} + func (s *Signature) SigningEnabled() bool { return s.Signer != nil } @@ -49,14 +67,26 @@ func (s *Signature) AddExtension(ctx context.Context, manifest *VSIXManifest, vs return "", xerrors.Errorf("generate signature manifest: %w", err) } - data, err := json.Marshal(sigManifest) + 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: data, + Content: sigManifestJSON, })...) } @@ -68,14 +98,14 @@ func (s *Signature) Manifest(ctx context.Context, publisher, name string, versio if s.SigningEnabled() { for _, asset := range manifest.Assets.Asset { - if asset.Path == SigzipFilename { + if asset.Path == SignatureZipFilename(manifest) { // Already signed return manifest, nil } } manifest.Assets.Asset = append(manifest.Assets.Asset, VSIXAsset{ Type: VSIXSignatureType, - Path: SigzipFilename, + Path: SignatureZipFilename(manifest), Addressable: "true", }) return manifest, nil @@ -84,7 +114,7 @@ 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 'SigzipFilename' or p7s.sig. +// 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 @@ -105,7 +135,10 @@ func (s *Signature) Manifest(ctx context.Context, publisher, name string, versio // source implementation. Ideally this marketplace would match Microsoft's // marketplace API. func (s *Signature) Open(ctx context.Context, fp string) (fs.File, error) { - if s.SigningEnabled() && filepath.Base(fp) == SigzipFilename { + 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 { @@ -121,15 +154,39 @@ func (s *Signature) Open(ctx context.Context, fp string) (fs.File, error) { return nil, xerrors.Errorf("read signature manifest: %w", err) } - signed, err := extensionsign.SignAndZipManifest(s.Signer, manifestData) + 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) if err != nil { return nil, xerrors.Errorf("sign and zip manifest: %w", err) } - f := mem.NewFileHandle(mem.CreateFile(SigzipFilename)) + f := mem.NewFileHandle(mem.CreateFile(fp)) _, err = f.Write(signed) return f, err } 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 --git a/storage/signature_test.go b/storage/signature_test.go index bfcef6d..9104d97 100644 --- a/storage/signature_test.go +++ b/storage/signature_test.go @@ -11,7 +11,7 @@ import ( func expectSignature(manifest *storage.VSIXManifest) { manifest.Assets.Asset = append(manifest.Assets.Asset, storage.VSIXAsset{ Type: storage.VSIXSignatureType, - Path: storage.SigzipFilename, + Path: storage.SigzipFileExtension, Addressable: "true", }) } diff --git a/storage/storage.go b/storage/storage.go index 94e5505..a81a24a 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -132,8 +132,12 @@ type Options struct { Artifactory string ExtDir string Repo string + SaveSigZips bool Logger slog.Logger ListCacheDuration time.Duration + // 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 } type extension struct { @@ -293,7 +297,12 @@ func NewStorage(ctx context.Context, options *Options) (Storage, error) { return nil, err } - return NewSignatureStorage(options.Signer, store), nil + signingStorage := NewSignatureStorage(options.Logger, options.Signer, store) + if options.SaveSigZips { + signingStorage.SaveSigZips() + } + + return signingStorage, nil } // ReadVSIXManifest reads and parses an extension manifest from a vsix file. If From 06306d88ffbc39cd875dec56a43b386507751861 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 13 Dec 2024 11:32:16 -0600 Subject: [PATCH 2/4] forgot test file fix --- storage/signature_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/storage/signature_test.go b/storage/signature_test.go index 9104d97..470ff3c 100644 --- a/storage/signature_test.go +++ b/storage/signature_test.go @@ -4,6 +4,7 @@ import ( "crypto" "testing" + "cdr.dev/slog" "github.com/coder/code-marketplace/extensionsign" "github.com/coder/code-marketplace/storage" ) @@ -28,7 +29,7 @@ func signed(signer bool, factory func(t *testing.T) testStorage) func(t *testing } return testStorage{ - storage: storage.NewSignatureStorage(key, st.storage), + storage: storage.NewSignatureStorage(slog.Make(), key, st.storage), write: st.write, exists: st.exists, expectedManifest: exp, From 78ceafa684fd39c98a333d0ce58bd72d06fa817b Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 13 Dec 2024 11:34:00 -0600 Subject: [PATCH 3/4] fix file assert --- storage/signature_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/signature_test.go b/storage/signature_test.go index 470ff3c..452bea1 100644 --- a/storage/signature_test.go +++ b/storage/signature_test.go @@ -12,7 +12,7 @@ import ( func expectSignature(manifest *storage.VSIXManifest) { manifest.Assets.Asset = append(manifest.Assets.Asset, storage.VSIXAsset{ Type: storage.VSIXSignatureType, - Path: storage.SigzipFileExtension, + Path: storage.SignatureZipFilename(manifest), Addressable: "true", }) } From e1069d9e39b2d532103a65a6cbdd2a4cb3ce9ad9 Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Fri, 13 Dec 2024 11:36:27 -0600 Subject: [PATCH 4/4] fix unused variable --- storage/storage.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/storage/storage.go b/storage/storage.go index a81a24a..ad5ed13 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -135,9 +135,6 @@ type Options struct { SaveSigZips bool Logger slog.Logger ListCacheDuration time.Duration - // 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 } type extension struct {