From 9ea35da0d0309b59b15c1a00cf619f3869512b61 Mon Sep 17 00:00:00 2001 From: Scott Rigby Date: Sat, 30 Aug 2025 13:25:28 -0400 Subject: [PATCH] [HIP-0026] Plugin packaging, signing, and verification (#31176) * Plugin packaging, signing and verification Signed-off-by: Scott Rigby * wrap keyring read error with more explicit message Co-authored-by: Jesse Simpson Signed-off-by: Scott Rigby * skip unnecessary check Co-authored-by: Evans Mungai Signed-off-by: Scott Rigby * Change behavior for installing plugin with missing .prov file (now warns and continues instead of failing) Signed-off-by: Scott Rigby * Add comprehensive plugin verification tests - Test missing .prov files (warns but continues) - Test invalid .prov file formats (fails verification) - Test hash mismatches in .prov files (fails verification) - Test .prov file access errors (fails appropriately) - Test directory plugins don't support verification - Test installation without verification enabled (succeeds) - Test with valid .prov files (fails on empty keyring as expected) --------- Signed-off-by: Scott Rigby Co-authored-by: Jesse Simpson Co-authored-by: Evans Mungai --- internal/plugin/installer/extractor.go | 195 ++++++++ internal/plugin/installer/http_installer.go | 215 +++------ .../plugin/installer/http_installer_test.go | 3 +- internal/plugin/installer/installer.go | 94 +++- internal/plugin/installer/local_installer.go | 84 +++- .../plugin/installer/local_installer_test.go | 83 +--- internal/plugin/installer/oci_installer.go | 97 +++- .../plugin/installer/oci_installer_test.go | 24 +- .../plugin/installer/vcs_installer_test.go | 5 +- .../plugin/installer/verification_test.go | 421 ++++++++++++++++++ internal/plugin/sign.go | 166 +++++++ internal/plugin/sign_test.go | 92 ++++ internal/plugin/signing_info.go | 178 ++++++++ internal/plugin/verify.go | 72 +++ internal/plugin/verify_test.go | 201 +++++++++ pkg/action/package.go | 16 +- pkg/cmd/plugin.go | 2 + pkg/cmd/plugin_install.go | 55 ++- pkg/cmd/plugin_list.go | 12 +- pkg/cmd/plugin_package.go | 209 +++++++++ pkg/cmd/plugin_package_test.go | 170 +++++++ pkg/cmd/plugin_verify.go | 88 ++++ pkg/cmd/plugin_verify_test.go | 264 +++++++++++ pkg/getter/ocigetter.go | 16 +- pkg/provenance/doc.go | 10 +- pkg/provenance/sign.go | 96 ++-- pkg/provenance/sign_test.go | 42 +- pkg/registry/plugin.go | 45 +- 28 files changed, 2599 insertions(+), 356 deletions(-) create mode 100644 internal/plugin/installer/extractor.go create mode 100644 internal/plugin/installer/verification_test.go create mode 100644 internal/plugin/sign.go create mode 100644 internal/plugin/sign_test.go create mode 100644 internal/plugin/signing_info.go create mode 100644 internal/plugin/verify.go create mode 100644 internal/plugin/verify_test.go create mode 100644 pkg/cmd/plugin_package.go create mode 100644 pkg/cmd/plugin_package_test.go create mode 100644 pkg/cmd/plugin_verify.go create mode 100644 pkg/cmd/plugin_verify_test.go diff --git a/internal/plugin/installer/extractor.go b/internal/plugin/installer/extractor.go new file mode 100644 index 000000000..9417a0535 --- /dev/null +++ b/internal/plugin/installer/extractor.go @@ -0,0 +1,195 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package installer // import "helm.sh/helm/v4/internal/plugin/installer" + +import ( + "archive/tar" + "bytes" + "compress/gzip" + "errors" + "fmt" + "io" + "os" + "path" + "path/filepath" + "regexp" + "slices" + "strings" + + securejoin "github.com/cyphar/filepath-securejoin" +) + +// TarGzExtractor extracts gzip compressed tar archives +type TarGzExtractor struct{} + +// Extractor provides an interface for extracting archives +type Extractor interface { + Extract(buffer *bytes.Buffer, targetDir string) error +} + +// Extractors contains a map of suffixes and matching implementations of extractor to return +var Extractors = map[string]Extractor{ + ".tar.gz": &TarGzExtractor{}, + ".tgz": &TarGzExtractor{}, +} + +// Convert a media type to an extractor extension. +// +// This should be refactored in Helm 4, combined with the extension-based mechanism. +func mediaTypeToExtension(mt string) (string, bool) { + switch strings.ToLower(mt) { + case "application/gzip", "application/x-gzip", "application/x-tgz", "application/x-gtar": + return ".tgz", true + case "application/octet-stream": + // Generic binary type - we'll need to check the URL suffix + return "", false + default: + return "", false + } +} + +// NewExtractor creates a new extractor matching the source file name +func NewExtractor(source string) (Extractor, error) { + for suffix, extractor := range Extractors { + if strings.HasSuffix(source, suffix) { + return extractor, nil + } + } + return nil, fmt.Errorf("no extractor implemented yet for %s", source) +} + +// cleanJoin resolves dest as a subpath of root. +// +// This function runs several security checks on the path, generating an error if +// the supplied `dest` looks suspicious or would result in dubious behavior on the +// filesystem. +// +// cleanJoin assumes that any attempt by `dest` to break out of the CWD is an attempt +// to be malicious. (If you don't care about this, use the securejoin-filepath library.) +// It will emit an error if it detects paths that _look_ malicious, operating on the +// assumption that we don't actually want to do anything with files that already +// appear to be nefarious. +// +// - The character `:` is considered illegal because it is a separator on UNIX and a +// drive designator on Windows. +// - The path component `..` is considered suspicions, and therefore illegal +// - The character \ (backslash) is treated as a path separator and is converted to /. +// - Beginning a path with a path separator is illegal +// - Rudimentary symlink protects are offered by SecureJoin. +func cleanJoin(root, dest string) (string, error) { + + // On Windows, this is a drive separator. On UNIX-like, this is the path list separator. + // In neither case do we want to trust a TAR that contains these. + if strings.Contains(dest, ":") { + return "", errors.New("path contains ':', which is illegal") + } + + // The Go tar library does not convert separators for us. + // We assume here, as we do elsewhere, that `\\` means a Windows path. + dest = strings.ReplaceAll(dest, "\\", "/") + + // We want to alert the user that something bad was attempted. Cleaning it + // is not a good practice. + if slices.Contains(strings.Split(dest, "/"), "..") { + return "", errors.New("path contains '..', which is illegal") + } + + // If a path is absolute, the creator of the TAR is doing something shady. + if path.IsAbs(dest) { + return "", errors.New("path is absolute, which is illegal") + } + + // SecureJoin will do some cleaning, as well as some rudimentary checking of symlinks. + // The directory needs to be cleaned prior to passing to SecureJoin or the location may end up + // being wrong or returning an error. This was introduced in v0.4.0. + root = filepath.Clean(root) + newpath, err := securejoin.SecureJoin(root, dest) + if err != nil { + return "", err + } + + return filepath.ToSlash(newpath), nil +} + +// Extract extracts compressed archives +// +// Implements Extractor. +func (g *TarGzExtractor) Extract(buffer *bytes.Buffer, targetDir string) error { + uncompressedStream, err := gzip.NewReader(buffer) + if err != nil { + return err + } + + if err := os.MkdirAll(targetDir, 0755); err != nil { + return err + } + + tarReader := tar.NewReader(uncompressedStream) + for { + header, err := tarReader.Next() + if err == io.EOF { + break + } + if err != nil { + return err + } + + path, err := cleanJoin(targetDir, header.Name) + if err != nil { + return err + } + + switch header.Typeflag { + case tar.TypeDir: + if err := os.MkdirAll(path, 0755); err != nil { + return err + } + case tar.TypeReg: + // Ensure parent directory exists + if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { + return err + } + outFile, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode)) + if err != nil { + return err + } + if _, err := io.Copy(outFile, tarReader); err != nil { + outFile.Close() + return err + } + outFile.Close() + // We don't want to process these extension header files. + case tar.TypeXGlobalHeader, tar.TypeXHeader: + continue + default: + return fmt.Errorf("unknown type: %b in %s", header.Typeflag, header.Name) + } + } + return nil +} + +// stripPluginName is a helper that relies on some sort of convention for plugin name (plugin-name-) +func stripPluginName(name string) string { + var strippedName string + for suffix := range Extractors { + if strings.HasSuffix(name, suffix) { + strippedName = strings.TrimSuffix(name, suffix) + break + } + } + re := regexp.MustCompile(`(.*)-[0-9]+\..*`) + return re.ReplaceAllString(strippedName, `$1`) +} diff --git a/internal/plugin/installer/http_installer.go b/internal/plugin/installer/http_installer.go index b68fc059a..a4687d8c9 100644 --- a/internal/plugin/installer/http_installer.go +++ b/internal/plugin/installer/http_installer.go @@ -16,22 +16,14 @@ limitations under the License. package installer // import "helm.sh/helm/v4/internal/plugin/installer" import ( - "archive/tar" "bytes" - "compress/gzip" - "errors" "fmt" - "io" "log/slog" "os" - "path" "path/filepath" - "regexp" - "slices" "strings" - securejoin "github.com/cyphar/filepath-securejoin" - + "helm.sh/helm/v4/internal/plugin" "helm.sh/helm/v4/internal/plugin/cache" "helm.sh/helm/v4/internal/third_party/dep/fs" "helm.sh/helm/v4/pkg/cli" @@ -46,45 +38,8 @@ type HTTPInstaller struct { base extractor Extractor getter getter.Getter -} - -// TarGzExtractor extracts gzip compressed tar archives -type TarGzExtractor struct{} - -// Extractor provides an interface for extracting archives -type Extractor interface { - Extract(buffer *bytes.Buffer, targetDir string) error -} - -// Extractors contains a map of suffixes and matching implementations of extractor to return -var Extractors = map[string]Extractor{ - ".tar.gz": &TarGzExtractor{}, - ".tgz": &TarGzExtractor{}, -} - -// Convert a media type to an extractor extension. -// -// This should be refactored in Helm 4, combined with the extension-based mechanism. -func mediaTypeToExtension(mt string) (string, bool) { - switch strings.ToLower(mt) { - case "application/gzip", "application/x-gzip", "application/x-tgz", "application/x-gtar": - return ".tgz", true - case "application/octet-stream": - // Generic binary type - we'll need to check the URL suffix - return "", false - default: - return "", false - } -} - -// NewExtractor creates a new extractor matching the source file name -func NewExtractor(source string) (Extractor, error) { - for suffix, extractor := range Extractors { - if strings.HasSuffix(source, suffix) { - return extractor, nil - } - } - return nil, fmt.Errorf("no extractor implemented yet for %s", source) + // Provenance data to save after installation + provData []byte } // NewHTTPInstaller creates a new HttpInstaller. @@ -114,19 +69,6 @@ func NewHTTPInstaller(source string) (*HTTPInstaller, error) { return i, nil } -// helper that relies on some sort of convention for plugin name (plugin-name-) -func stripPluginName(name string) string { - var strippedName string - for suffix := range Extractors { - if strings.HasSuffix(name, suffix) { - strippedName = strings.TrimSuffix(name, suffix) - break - } - } - re := regexp.MustCompile(`(.*)-[0-9]+\..*`) - return re.ReplaceAllString(strippedName, `$1`) -} - // Install downloads and extracts the tarball into the cache directory // and installs into the plugin directory. // @@ -137,6 +79,31 @@ func (i *HTTPInstaller) Install() error { return err } + // Save the original tarball to plugins directory for verification + // Extract metadata to get the actual plugin name and version + pluginBytes := pluginData.Bytes() + metadata, err := plugin.ExtractPluginMetadataFromReader(bytes.NewReader(pluginBytes)) + if err != nil { + return fmt.Errorf("failed to extract plugin metadata from tarball: %w", err) + } + filename := fmt.Sprintf("%s-%s.tgz", metadata.Name, metadata.Version) + tarballPath := helmpath.DataPath("plugins", filename) + if err := os.MkdirAll(filepath.Dir(tarballPath), 0755); err != nil { + return fmt.Errorf("failed to create plugins directory: %w", err) + } + if err := os.WriteFile(tarballPath, pluginBytes, 0644); err != nil { + return fmt.Errorf("failed to save tarball: %w", err) + } + + // Try to download .prov file if it exists + provURL := i.Source + ".prov" + if provData, err := i.getter.Get(provURL); err == nil { + provPath := tarballPath + ".prov" + if err := os.WriteFile(provPath, provData.Bytes(), 0644); err != nil { + slog.Debug("failed to save provenance file", "error", err) + } + } + if err := i.extractor.Extract(pluginData, i.CacheDir); err != nil { return fmt.Errorf("extracting files from archive: %w", err) } @@ -175,111 +142,57 @@ func (i HTTPInstaller) Path() string { return helmpath.DataPath("plugins", i.PluginName) } -// cleanJoin resolves dest as a subpath of root. -// -// This function runs several security checks on the path, generating an error if -// the supplied `dest` looks suspicious or would result in dubious behavior on the -// filesystem. -// -// cleanJoin assumes that any attempt by `dest` to break out of the CWD is an attempt -// to be malicious. (If you don't care about this, use the securejoin-filepath library.) -// It will emit an error if it detects paths that _look_ malicious, operating on the -// assumption that we don't actually want to do anything with files that already -// appear to be nefarious. -// -// - The character `:` is considered illegal because it is a separator on UNIX and a -// drive designator on Windows. -// - The path component `..` is considered suspicions, and therefore illegal -// - The character \ (backslash) is treated as a path separator and is converted to /. -// - Beginning a path with a path separator is illegal -// - Rudimentary symlink protects are offered by SecureJoin. -func cleanJoin(root, dest string) (string, error) { +// SupportsVerification returns true if the HTTP installer can verify plugins +func (i *HTTPInstaller) SupportsVerification() bool { + // Only support verification for tarball URLs + return strings.HasSuffix(i.Source, ".tgz") || strings.HasSuffix(i.Source, ".tar.gz") +} - // On Windows, this is a drive separator. On UNIX-like, this is the path list separator. - // In neither case do we want to trust a TAR that contains these. - if strings.Contains(dest, ":") { - return "", errors.New("path contains ':', which is illegal") +// PrepareForVerification downloads the plugin and signature files for verification +func (i *HTTPInstaller) PrepareForVerification() (string, func(), error) { + if !i.SupportsVerification() { + return "", nil, fmt.Errorf("verification not supported for this source") } - // The Go tar library does not convert separators for us. - // We assume here, as we do elsewhere, that `\\` means a Windows path. - dest = strings.ReplaceAll(dest, "\\", "/") - - // We want to alert the user that something bad was attempted. Cleaning it - // is not a good practice. - if slices.Contains(strings.Split(dest, "/"), "..") { - return "", errors.New("path contains '..', which is illegal") + // Create temporary directory for downloads + tempDir, err := os.MkdirTemp("", "helm-plugin-verify-*") + if err != nil { + return "", nil, fmt.Errorf("failed to create temp directory: %w", err) } - // If a path is absolute, the creator of the TAR is doing something shady. - if path.IsAbs(dest) { - return "", errors.New("path is absolute, which is illegal") + cleanup := func() { + os.RemoveAll(tempDir) } - // SecureJoin will do some cleaning, as well as some rudimentary checking of symlinks. - // The directory needs to be cleaned prior to passing to SecureJoin or the location may end up - // being wrong or returning an error. This was introduced in v0.4.0. - root = filepath.Clean(root) - newpath, err := securejoin.SecureJoin(root, dest) + // Download plugin tarball + pluginFile := filepath.Join(tempDir, filepath.Base(i.Source)) + + g, err := getter.All(new(cli.EnvSettings)).ByScheme("http") if err != nil { - return "", err + cleanup() + return "", nil, err } - return filepath.ToSlash(newpath), nil -} - -// Extract extracts compressed archives -// -// Implements Extractor. -func (g *TarGzExtractor) Extract(buffer *bytes.Buffer, targetDir string) error { - uncompressedStream, err := gzip.NewReader(buffer) + data, err := g.Get(i.Source, getter.WithURL(i.Source)) if err != nil { - return err + cleanup() + return "", nil, fmt.Errorf("failed to download plugin: %w", err) } - if err := os.MkdirAll(targetDir, 0755); err != nil { - return err + if err := os.WriteFile(pluginFile, data.Bytes(), 0644); err != nil { + cleanup() + return "", nil, fmt.Errorf("failed to write plugin file: %w", err) } - tarReader := tar.NewReader(uncompressedStream) - for { - header, err := tarReader.Next() - if err == io.EOF { - break - } - if err != nil { - return err - } - - path, err := cleanJoin(targetDir, header.Name) - if err != nil { - return err - } - - switch header.Typeflag { - case tar.TypeDir: - if err := os.MkdirAll(path, 0755); err != nil { - return err - } - case tar.TypeReg: - // Ensure parent directory exists - if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil { - return err - } - outFile, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode)) - if err != nil { - return err - } - defer outFile.Close() - if _, err := io.Copy(outFile, tarReader); err != nil { - return err - } - // We don't want to process these extension header files. - case tar.TypeXGlobalHeader, tar.TypeXHeader: - continue - default: - return fmt.Errorf("unknown type: %b in %s", header.Typeflag, header.Name) + // Try to download signature file - don't fail if it doesn't exist + if provData, err := g.Get(i.Source+".prov", getter.WithURL(i.Source+".prov")); err == nil { + if err := os.WriteFile(pluginFile+".prov", provData.Bytes(), 0644); err == nil { + // Store the provenance data so we can save it after installation + i.provData = provData.Bytes() } } - return nil + // Note: We don't fail if .prov file can't be downloaded - the verification logic + // in InstallWithOptions will handle missing .prov files appropriately + + return pluginFile, cleanup, nil } diff --git a/internal/plugin/installer/http_installer_test.go b/internal/plugin/installer/http_installer_test.go index 453021b76..be40b1b90 100644 --- a/internal/plugin/installer/http_installer_test.go +++ b/internal/plugin/installer/http_installer_test.go @@ -49,7 +49,7 @@ func (t *TestHTTPGetter) Get(_ string, _ ...getter.Option) (*bytes.Buffer, error } // Fake plugin tarball data -var fakePluginB64 = "H4sIAKRj51kAA+3UX0vCUBgGcC9jn+Iwuk3Peza3GeyiUlJQkcogCOzgli7dJm4TvYk+a5+k479UqquUCJ/fLs549sLO2TnvWnJa9aXnjwujYdYLovxMhsPcfnHOLdNkOXthM/IVQQYjg2yyLLJ4kXGhLp5j0z3P41tZksqxmspL3B/O+j/XtZu1y8rdYzkOZRCxduKPk53ny6Wwz/GfIIf1As8lxzGJSmoHNLJZphKHG4YpTCE0wVk3DULfpSJ3DMMqkj3P5JfMYLdX1Vr9Ie/5E5cstcdC8K04iGLX5HaJuKpWL17F0TCIBi5pf/0pjtLhun5j3f9v6r7wfnI/H0eNp9d1/5P6Gez0vzo7wsoxfrAZbTny/o9k6J8z/VkO/LPlWdC1iVpbEEcq5nmeJ13LEtmbV0k2r2PrOs9PuuNglC5rL1Y5S/syXRQmutaNw1BGnnp8Wq3UG51WvX1da3bKtZtCN/R09DwAAAAAAAAAAAAAAAAAAADAb30AoMczDwAoAAA=" +var fakePluginB64 = "H4sIAAAAAAAAA+3SQUvDMBgG4Jz7K0LwapdvSxrwJig6mCKC5xHabBaXdDSt4L+3cQ56mV42ZPg+lw+SF5LwZmXf3OV206/rMGEnIgdG6zTJaDmee4y01FOlZpqGHJGZSsb1qS401sfOtpyz0FTup9xv+2dqNep/N/IP6zdHPSMVXCh1sH8yhtGMDBUFFTL1r4iIcXnUWxzwz/sP1rsrLkbfQGTvro11E4ZlmcucRNZHu04py1OO73OVi2Vbb7td9vp7nXevtvsKRpGVjfc2VMP2xf3t4mH5tHi5mz8ub+bPk9JXIvvr5wMAAAAAAAAAAAAAAAAAAAAAnLVPqwHcXQAoAAA=" func TestStripName(t *testing.T) { if stripPluginName("fake-plugin-0.0.1.tar.gz") != "fake-plugin" { @@ -515,6 +515,7 @@ func TestExtractWithExistingDirectory(t *testing.T) { } func TestExtractPluginInSubdirectory(t *testing.T) { + ensure.HelmHome(t) source := "https://repo.localdomain/plugins/subdir-plugin-1.0.0.tar.gz" tempDir := t.TempDir() diff --git a/internal/plugin/installer/installer.go b/internal/plugin/installer/installer.go index 7900f6745..dd169397e 100644 --- a/internal/plugin/installer/installer.go +++ b/internal/plugin/installer/installer.go @@ -17,12 +17,14 @@ package installer import ( "errors" + "fmt" "net/http" "os" "path/filepath" "strings" "helm.sh/helm/v4/internal/plugin" + "helm.sh/helm/v4/pkg/registry" ) // ErrMissingMetadata indicates that plugin.yaml is missing. @@ -31,6 +33,14 @@ var ErrMissingMetadata = errors.New("plugin metadata (plugin.yaml) missing") // Debug enables verbose output. var Debug bool +// Options contains options for plugin installation. +type Options struct { + // Verify enables signature verification before installation + Verify bool + // Keyring is the path to the keyring for verification + Keyring string +} + // Installer provides an interface for installing helm client plugins. type Installer interface { // Install adds a plugin. @@ -41,15 +51,89 @@ type Installer interface { Update() error } +// Verifier provides an interface for installers that support verification. +type Verifier interface { + // SupportsVerification returns true if this installer can verify plugins + SupportsVerification() bool + // PrepareForVerification downloads necessary files for verification + PrepareForVerification() (pluginPath string, cleanup func(), err error) +} + // Install installs a plugin. func Install(i Installer) error { + _, err := InstallWithOptions(i, Options{}) + return err +} + +// VerificationResult contains the result of plugin verification +type VerificationResult struct { + SignedBy []string + Fingerprint string + FileHash string +} + +// InstallWithOptions installs a plugin with options. +func InstallWithOptions(i Installer, opts Options) (*VerificationResult, error) { + if err := os.MkdirAll(filepath.Dir(i.Path()), 0755); err != nil { - return err + return nil, err } if _, pathErr := os.Stat(i.Path()); !os.IsNotExist(pathErr) { - return errors.New("plugin already exists") + return nil, errors.New("plugin already exists") + } + + var result *VerificationResult + + // If verification is requested, check if installer supports it + if opts.Verify { + verifier, ok := i.(Verifier) + if !ok || !verifier.SupportsVerification() { + return nil, fmt.Errorf("--verify is only supported for plugin tarballs (.tgz files)") + } + + // Prepare for verification (download files if needed) + pluginPath, cleanup, err := verifier.PrepareForVerification() + if err != nil { + return nil, fmt.Errorf("failed to prepare for verification: %w", err) + } + if cleanup != nil { + defer cleanup() + } + + // Check if provenance file exists + provFile := pluginPath + ".prov" + if _, err := os.Stat(provFile); err != nil { + if os.IsNotExist(err) { + // No .prov file found - emit warning but continue installation + fmt.Fprintf(os.Stderr, "WARNING: No provenance file found for plugin. Plugin is not signed and cannot be verified.\n") + } else { + // Other error accessing .prov file + return nil, fmt.Errorf("failed to access provenance file: %w", err) + } + } else { + // Provenance file exists - verify the plugin + verification, err := plugin.VerifyPlugin(pluginPath, opts.Keyring) + if err != nil { + return nil, fmt.Errorf("plugin verification failed: %w", err) + } + + // Collect verification info + result = &VerificationResult{ + SignedBy: make([]string, 0), + Fingerprint: fmt.Sprintf("%X", verification.SignedBy.PrimaryKey.Fingerprint), + FileHash: verification.FileHash, + } + for name := range verification.SignedBy.Identities { + result.SignedBy = append(result.SignedBy, name) + } + } } - return i.Install() + + if err := i.Install(); err != nil { + return nil, err + } + + return result, nil } // Update updates a plugin. @@ -62,6 +146,10 @@ func Update(i Installer) error { // NewForSource determines the correct Installer for the given source. func NewForSource(source, version string) (Installer, error) { + // Check if source is an OCI registry reference + if strings.HasPrefix(source, fmt.Sprintf("%s://", registry.OCIScheme)) { + return NewOCIInstaller(source) + } // Check if source is a local directory if isLocalReference(source) { return NewLocalInstaller(source) diff --git a/internal/plugin/installer/local_installer.go b/internal/plugin/installer/local_installer.go index 87b9eaf97..0e00c93d0 100644 --- a/internal/plugin/installer/local_installer.go +++ b/internal/plugin/installer/local_installer.go @@ -24,7 +24,9 @@ import ( "path/filepath" "strings" + "helm.sh/helm/v4/internal/plugin" "helm.sh/helm/v4/internal/third_party/dep/fs" + "helm.sh/helm/v4/pkg/helmpath" ) // ErrPluginNotAFolder indicates that the plugin path is not a folder. @@ -35,6 +37,7 @@ type LocalInstaller struct { base isArchive bool extractor Extractor + provData []byte // Provenance data to save after installation } // NewLocalInstaller creates a new LocalInstaller. @@ -105,6 +108,30 @@ func (i *LocalInstaller) installFromArchive() error { return fmt.Errorf("failed to read archive: %w", err) } + // Copy the original tarball to plugins directory for verification + // Extract metadata to get the actual plugin name and version + metadata, err := plugin.ExtractPluginMetadataFromReader(bytes.NewReader(data)) + if err != nil { + return fmt.Errorf("failed to extract plugin metadata from tarball: %w", err) + } + filename := fmt.Sprintf("%s-%s.tgz", metadata.Name, metadata.Version) + tarballPath := helmpath.DataPath("plugins", filename) + if err := os.MkdirAll(filepath.Dir(tarballPath), 0755); err != nil { + return fmt.Errorf("failed to create plugins directory: %w", err) + } + if err := os.WriteFile(tarballPath, data, 0644); err != nil { + return fmt.Errorf("failed to save tarball: %w", err) + } + + // Check for and copy .prov file if it exists + provSource := i.Source + ".prov" + if provData, err := os.ReadFile(provSource); err == nil { + provPath := tarballPath + ".prov" + if err := os.WriteFile(provPath, provData, 0644); err != nil { + slog.Debug("failed to save provenance file", "error", err) + } + } + // Create a temporary directory for extraction tempDir, err := os.MkdirTemp("", "helm-plugin-extract-") if err != nil { @@ -118,31 +145,60 @@ func (i *LocalInstaller) installFromArchive() error { return fmt.Errorf("failed to extract archive: %w", err) } - // Detect where the plugin.yaml actually is - pluginRoot, err := detectPluginRoot(tempDir) - if err != nil { - return err + // Plugin directory should be named after the plugin at the archive root + pluginName := stripPluginName(filepath.Base(i.Source)) + pluginDir := filepath.Join(tempDir, pluginName) + if _, err = os.Stat(filepath.Join(pluginDir, "plugin.yaml")); err != nil { + return fmt.Errorf("plugin.yaml not found in expected directory %s: %w", pluginDir, err) } // Copy to the final destination - slog.Debug("copying", "source", pluginRoot, "path", i.Path()) - return fs.CopyDir(pluginRoot, i.Path()) + slog.Debug("copying", "source", pluginDir, "path", i.Path()) + return fs.CopyDir(pluginDir, i.Path()) +} + +// Update updates a local repository +func (i *LocalInstaller) Update() error { + slog.Debug("local repository is auto-updated") + return nil } -// Path returns the path where the plugin will be installed. -// For archive sources, strips the version from the filename. +// Path is overridden to handle archive plugin names properly func (i *LocalInstaller) Path() string { if i.Source == "" { return "" } + + pluginName := filepath.Base(i.Source) if i.isArchive { - return filepath.Join(i.PluginsDirectory, stripPluginName(filepath.Base(i.Source))) + // Strip archive extension to get plugin name + pluginName = stripPluginName(pluginName) } - return filepath.Join(i.PluginsDirectory, filepath.Base(i.Source)) + + return helmpath.DataPath("plugins", pluginName) } -// Update updates a local repository -func (i *LocalInstaller) Update() error { - slog.Debug("local repository is auto-updated") - return nil +// SupportsVerification returns true if the local installer can verify plugins +func (i *LocalInstaller) SupportsVerification() bool { + // Only support verification for local tarball files + return i.isArchive +} + +// PrepareForVerification returns the local path for verification +func (i *LocalInstaller) PrepareForVerification() (string, func(), error) { + if !i.SupportsVerification() { + return "", nil, fmt.Errorf("verification not supported for directories") + } + + // For local files, try to read the .prov file if it exists + provFile := i.Source + ".prov" + if provData, err := os.ReadFile(provFile); err == nil { + // Store the provenance data so we can save it after installation + i.provData = provData + } + // Note: We don't fail if .prov file doesn't exist - the verification logic + // in InstallWithOptions will handle missing .prov files appropriately + + // Return the source path directly, no cleanup needed + return i.Source, nil, nil } diff --git a/internal/plugin/installer/local_installer_test.go b/internal/plugin/installer/local_installer_test.go index 05118e183..339028ef3 100644 --- a/internal/plugin/installer/local_installer_test.go +++ b/internal/plugin/installer/local_installer_test.go @@ -86,8 +86,8 @@ func TestLocalInstallerTarball(t *testing.T) { Body string Mode int64 }{ - {"plugin.yaml", "name: test-plugin\nversion: 1.0.0\nusage: test\ndescription: test\ncommand: echo", 0644}, - {"bin/test-plugin", "#!/bin/bash\necho test", 0755}, + {"test-plugin/plugin.yaml", "name: test-plugin\nversion: 1.0.0\nusage: test\ndescription: test\ncommand: echo", 0644}, + {"test-plugin/bin/test-plugin", "#!/bin/bash\necho test", 0755}, } for _, file := range files { @@ -146,82 +146,3 @@ func TestLocalInstallerTarball(t *testing.T) { t.Fatalf("plugin not found at %s: %v", i.Path(), err) } } - -func TestLocalInstallerTarballWithSubdirectory(t *testing.T) { - ensure.HelmHome(t) - - // Create a test tarball with subdirectory - tempDir := t.TempDir() - tarballPath := filepath.Join(tempDir, "subdir-plugin-1.0.0.tar.gz") - - // Create tarball content - var buf bytes.Buffer - gw := gzip.NewWriter(&buf) - tw := tar.NewWriter(gw) - - files := []struct { - Name string - Body string - Mode int64 - IsDir bool - }{ - {"my-plugin/", "", 0755, true}, - {"my-plugin/plugin.yaml", "name: my-plugin\nversion: 1.0.0\nusage: test\ndescription: test\ncommand: echo", 0644, false}, - {"my-plugin/bin/", "", 0755, true}, - {"my-plugin/bin/my-plugin", "#!/bin/bash\necho test", 0755, false}, - } - - for _, file := range files { - hdr := &tar.Header{ - Name: file.Name, - Mode: file.Mode, - } - if file.IsDir { - hdr.Typeflag = tar.TypeDir - } else { - hdr.Size = int64(len(file.Body)) - } - - if err := tw.WriteHeader(hdr); err != nil { - t.Fatal(err) - } - if !file.IsDir { - if _, err := tw.Write([]byte(file.Body)); err != nil { - t.Fatal(err) - } - } - } - - if err := tw.Close(); err != nil { - t.Fatal(err) - } - if err := gw.Close(); err != nil { - t.Fatal(err) - } - - // Write tarball to file - if err := os.WriteFile(tarballPath, buf.Bytes(), 0644); err != nil { - t.Fatal(err) - } - - // Test installation - i, err := NewForSource(tarballPath, "") - if err != nil { - t.Fatalf("unexpected error: %s", err) - } - - if err := Install(i); err != nil { - t.Fatal(err) - } - - expectedPath := helmpath.DataPath("plugins", "subdir-plugin") - if i.Path() != expectedPath { - t.Fatalf("expected path %q, got %q", expectedPath, i.Path()) - } - - // Verify plugin was installed from subdirectory - pluginYaml := filepath.Join(i.Path(), "plugin.yaml") - if _, err := os.Stat(pluginYaml); err != nil { - t.Fatalf("plugin.yaml not found at %s: %v", pluginYaml, err) - } -} diff --git a/internal/plugin/installer/oci_installer.go b/internal/plugin/installer/oci_installer.go index a96a94ee1..c33ef13d5 100644 --- a/internal/plugin/installer/oci_installer.go +++ b/internal/plugin/installer/oci_installer.go @@ -25,6 +25,7 @@ import ( "os" "path/filepath" + "helm.sh/helm/v4/internal/plugin" "helm.sh/helm/v4/internal/plugin/cache" "helm.sh/helm/v4/internal/third_party/dep/fs" "helm.sh/helm/v4/pkg/cli" @@ -33,6 +34,9 @@ import ( "helm.sh/helm/v4/pkg/registry" ) +// Ensure OCIInstaller implements Verifier +var _ Verifier = (*OCIInstaller)(nil) + // OCIInstaller installs plugins from OCI registries type OCIInstaller struct { CacheDir string @@ -85,17 +89,44 @@ func (i *OCIInstaller) Install() error { return fmt.Errorf("failed to pull plugin from %s: %w", i.Source, err) } - // Create cache directory - if err := os.MkdirAll(i.CacheDir, 0755); err != nil { - return fmt.Errorf("failed to create cache directory: %w", err) + // Save the original tarball to plugins directory for verification + // For OCI plugins, extract version from plugin.yaml inside the tarball + pluginBytes := pluginData.Bytes() + + // Extract metadata to get the actual plugin name and version + metadata, err := plugin.ExtractPluginMetadataFromReader(bytes.NewReader(pluginBytes)) + if err != nil { + return fmt.Errorf("failed to extract plugin metadata from tarball: %w", err) + } + filename := fmt.Sprintf("%s-%s.tgz", metadata.Name, metadata.Version) + + tarballPath := helmpath.DataPath("plugins", filename) + if err := os.MkdirAll(filepath.Dir(tarballPath), 0755); err != nil { + return fmt.Errorf("failed to create plugins directory: %w", err) + } + if err := os.WriteFile(tarballPath, pluginBytes, 0644); err != nil { + return fmt.Errorf("failed to save tarball: %w", err) + } + + // Try to download and save .prov file alongside the tarball + provSource := i.Source + ".prov" + if provData, err := i.getter.Get(provSource); err == nil { + provPath := tarballPath + ".prov" + if err := os.WriteFile(provPath, provData.Bytes(), 0644); err != nil { + slog.Debug("failed to save provenance file", "error", err) + } } // Check if this is a gzip compressed file - pluginBytes := pluginData.Bytes() if len(pluginBytes) < 2 || pluginBytes[0] != 0x1f || pluginBytes[1] != 0x8b { return fmt.Errorf("plugin data is not a gzip compressed archive") } + // Create cache directory + if err := os.MkdirAll(i.CacheDir, 0755); err != nil { + return fmt.Errorf("failed to create cache directory: %w", err) + } + // Extract as gzipped tar if err := extractTarGz(bytes.NewReader(pluginBytes), i.CacheDir); err != nil { return fmt.Errorf("failed to extract plugin: %w", err) @@ -214,3 +245,61 @@ func extractTar(r io.Reader, targetDir string) error { return nil } + +// SupportsVerification returns true since OCI plugins can be verified +func (i *OCIInstaller) SupportsVerification() bool { + return true +} + +// PrepareForVerification downloads the plugin tarball and provenance to a temporary directory +func (i *OCIInstaller) PrepareForVerification() (pluginPath string, cleanup func(), err error) { + slog.Debug("preparing OCI plugin for verification", "source", i.Source) + + // Create temporary directory for verification + tempDir, err := os.MkdirTemp("", "helm-oci-verify-") + if err != nil { + return "", nil, fmt.Errorf("failed to create temp directory: %w", err) + } + + cleanup = func() { + os.RemoveAll(tempDir) + } + + // Download the plugin tarball + pluginData, err := i.getter.Get(i.Source) + if err != nil { + cleanup() + return "", nil, fmt.Errorf("failed to pull plugin from %s: %w", i.Source, err) + } + + // Extract metadata to get the actual plugin name and version + pluginBytes := pluginData.Bytes() + metadata, err := plugin.ExtractPluginMetadataFromReader(bytes.NewReader(pluginBytes)) + if err != nil { + cleanup() + return "", nil, fmt.Errorf("failed to extract plugin metadata from tarball: %w", err) + } + filename := fmt.Sprintf("%s-%s.tgz", metadata.Name, metadata.Version) + + // Save plugin tarball to temp directory + pluginTarball := filepath.Join(tempDir, filename) + if err := os.WriteFile(pluginTarball, pluginBytes, 0644); err != nil { + cleanup() + return "", nil, fmt.Errorf("failed to save plugin tarball: %w", err) + } + + // Try to download the provenance file - don't fail if it doesn't exist + provSource := i.Source + ".prov" + if provData, err := i.getter.Get(provSource); err == nil { + // Save provenance to temp directory + provFile := filepath.Join(tempDir, filename+".prov") + if err := os.WriteFile(provFile, provData.Bytes(), 0644); err == nil { + slog.Debug("prepared plugin for verification", "plugin", pluginTarball, "provenance", provFile) + } + } + // Note: We don't fail if .prov file can't be downloaded - the verification logic + // in InstallWithOptions will handle missing .prov files appropriately + + slog.Debug("prepared plugin for verification", "plugin", pluginTarball) + return pluginTarball, cleanup, nil +} diff --git a/internal/plugin/installer/oci_installer_test.go b/internal/plugin/installer/oci_installer_test.go index 1ed10ff8e..1280cf97d 100644 --- a/internal/plugin/installer/oci_installer_test.go +++ b/internal/plugin/installer/oci_installer_test.go @@ -34,6 +34,7 @@ import ( "github.com/opencontainers/go-digest" ocispec "github.com/opencontainers/image-spec/specs-go/v1" + "helm.sh/helm/v4/internal/test/ensure" "helm.sh/helm/v4/pkg/cli" "helm.sh/helm/v4/pkg/getter" "helm.sh/helm/v4/pkg/helmpath" @@ -125,7 +126,7 @@ func mockOCIRegistryWithArtifactType(t *testing.T, pluginName string) (*httptest Digest: digest.Digest(layerDigest), Size: int64(len(pluginData)), Annotations: map[string]string{ - ocispec.AnnotationTitle: pluginName + ".tgz", // Layer named properly + ocispec.AnnotationTitle: pluginName + "-1.0.0.tgz", // Layer named with version }, }, }, @@ -316,9 +317,8 @@ func TestOCIInstaller_Path(t *testing.T) { } func TestOCIInstaller_Install(t *testing.T) { - // Set up isolated test environment FIRST - testPluginsDir := t.TempDir() - t.Setenv("HELM_PLUGINS", testPluginsDir) + // Set up isolated test environment + ensure.HelmHome(t) pluginName := "test-plugin-basic" server, registryHost := mockOCIRegistryWithArtifactType(t, pluginName) @@ -333,15 +333,10 @@ func TestOCIInstaller_Install(t *testing.T) { t.Fatalf("Expected no error, got %v", err) } - // The OCI installer uses helmpath.DataPath, which now points to our test directory + // The OCI installer uses helmpath.DataPath, which is isolated by ensure.HelmHome(t) actualPath := installer.Path() t.Logf("Installer will use path: %s", actualPath) - // Verify the path is actually in our test directory - if !strings.HasPrefix(actualPath, testPluginsDir) { - t.Fatalf("Expected path %s to be under test directory %s", actualPath, testPluginsDir) - } - // Install the plugin if err := Install(installer); err != nil { t.Fatalf("Expected installation to succeed, got error: %v", err) @@ -399,8 +394,7 @@ func TestOCIInstaller_Install_WithGetterOptions(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { // Set up isolated test environment for each subtest - testPluginsDir := t.TempDir() - t.Setenv("HELM_PLUGINS", testPluginsDir) + ensure.HelmHome(t) server, registryHost := mockOCIRegistryWithArtifactType(t, tc.pluginName) defer server.Close() @@ -440,8 +434,7 @@ func TestOCIInstaller_Install_WithGetterOptions(t *testing.T) { func TestOCIInstaller_Install_AlreadyExists(t *testing.T) { // Set up isolated test environment - testPluginsDir := t.TempDir() - t.Setenv("HELM_PLUGINS", testPluginsDir) + ensure.HelmHome(t) pluginName := "test-plugin-exists" server, registryHost := mockOCIRegistryWithArtifactType(t, pluginName) @@ -474,8 +467,7 @@ func TestOCIInstaller_Install_AlreadyExists(t *testing.T) { func TestOCIInstaller_Update(t *testing.T) { // Set up isolated test environment - testPluginsDir := t.TempDir() - t.Setenv("HELM_PLUGINS", testPluginsDir) + ensure.HelmHome(t) pluginName := "test-plugin-update" server, registryHost := mockOCIRegistryWithArtifactType(t, pluginName) diff --git a/internal/plugin/installer/vcs_installer_test.go b/internal/plugin/installer/vcs_installer_test.go index f024b4b40..d542a0f75 100644 --- a/internal/plugin/installer/vcs_installer_test.go +++ b/internal/plugin/installer/vcs_installer_test.go @@ -83,8 +83,9 @@ func TestVCSInstaller(t *testing.T) { if repo.current != "0.1.1" { t.Fatalf("expected version '0.1.1', got %q", repo.current) } - if i.Path() != helmpath.DataPath("plugins", "helm-env") { - t.Fatalf("expected path '$XDG_CONFIG_HOME/helm/plugins/helm-env', got %q", i.Path()) + expectedPath := helmpath.DataPath("plugins", "helm-env") + if i.Path() != expectedPath { + t.Fatalf("expected path %q, got %q", expectedPath, i.Path()) } // Install again to test plugin exists error diff --git a/internal/plugin/installer/verification_test.go b/internal/plugin/installer/verification_test.go new file mode 100644 index 000000000..22f0a8308 --- /dev/null +++ b/internal/plugin/installer/verification_test.go @@ -0,0 +1,421 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package installer + +import ( + "bytes" + "crypto/sha256" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "helm.sh/helm/v4/internal/plugin" + "helm.sh/helm/v4/internal/test/ensure" +) + +func TestInstallWithOptions_VerifyMissingProvenance(t *testing.T) { + ensure.HelmHome(t) + + // Create a temporary plugin tarball without .prov file + pluginDir := createTestPluginDir(t) + pluginTgz := createTarballFromPluginDir(t, pluginDir) + defer os.Remove(pluginTgz) + + // Create local installer + installer, err := NewLocalInstaller(pluginTgz) + if err != nil { + t.Fatalf("Failed to create installer: %v", err) + } + defer os.RemoveAll(installer.Path()) + + // Capture stderr to check warning message + oldStderr := os.Stderr + r, w, _ := os.Pipe() + os.Stderr = w + + // Install with verification enabled (should warn but succeed) + result, err := InstallWithOptions(installer, Options{Verify: true, Keyring: "dummy"}) + + // Restore stderr and read captured output + w.Close() + os.Stderr = oldStderr + var buf bytes.Buffer + io.Copy(&buf, r) + output := buf.String() + + // Should succeed with nil result (no verification performed) + if err != nil { + t.Fatalf("Expected installation to succeed despite missing .prov file, got error: %v", err) + } + if result != nil { + t.Errorf("Expected nil verification result when .prov file is missing, got: %+v", result) + } + + // Should contain warning message + expectedWarning := "WARNING: No provenance file found for plugin" + if !strings.Contains(output, expectedWarning) { + t.Errorf("Expected warning message '%s' in output, got: %s", expectedWarning, output) + } + + // Plugin should be installed + if _, err := os.Stat(installer.Path()); os.IsNotExist(err) { + t.Errorf("Plugin should be installed at %s", installer.Path()) + } +} + +func TestInstallWithOptions_VerifyWithValidProvenance(t *testing.T) { + ensure.HelmHome(t) + + // Create a temporary plugin tarball with valid .prov file + pluginDir := createTestPluginDir(t) + pluginTgz := createTarballFromPluginDir(t, pluginDir) + + provFile := pluginTgz + ".prov" + createProvFile(t, provFile, pluginTgz, "") + defer os.Remove(provFile) + + // Create keyring with test key (empty for testing) + keyring := createTestKeyring(t) + defer os.Remove(keyring) + + // Create local installer + installer, err := NewLocalInstaller(pluginTgz) + if err != nil { + t.Fatalf("Failed to create installer: %v", err) + } + defer os.RemoveAll(installer.Path()) + + // Install with verification enabled + // This will fail signature verification but pass hash validation + result, err := InstallWithOptions(installer, Options{Verify: true, Keyring: keyring}) + + // Should fail due to invalid signature (empty keyring) but we test that it gets past the hash check + if err == nil { + t.Fatalf("Expected installation to fail with empty keyring") + } + if !strings.Contains(err.Error(), "plugin verification failed") { + t.Errorf("Expected plugin verification failed error, got: %v", err) + } + if result != nil { + t.Errorf("Expected nil verification result when verification fails, got: %+v", result) + } + + // Plugin should not be installed due to verification failure + if _, err := os.Stat(installer.Path()); !os.IsNotExist(err) { + t.Errorf("Plugin should not be installed when verification fails") + } +} + +func TestInstallWithOptions_VerifyWithInvalidProvenance(t *testing.T) { + ensure.HelmHome(t) + + // Create a temporary plugin tarball with invalid .prov file + pluginDir := createTestPluginDir(t) + pluginTgz := createTarballFromPluginDir(t, pluginDir) + defer os.Remove(pluginTgz) + + provFile := pluginTgz + ".prov" + createProvFileInvalidFormat(t, provFile) + defer os.Remove(provFile) + + // Create keyring with test key + keyring := createTestKeyring(t) + defer os.Remove(keyring) + + // Create local installer + installer, err := NewLocalInstaller(pluginTgz) + if err != nil { + t.Fatalf("Failed to create installer: %v", err) + } + defer os.RemoveAll(installer.Path()) + + // Install with verification enabled (should fail) + result, err := InstallWithOptions(installer, Options{Verify: true, Keyring: keyring}) + + // Should fail with verification error + if err == nil { + t.Fatalf("Expected installation with invalid .prov file to fail") + } + if result != nil { + t.Errorf("Expected nil verification result when verification fails, got: %+v", result) + } + + // Should contain verification failure message + expectedError := "plugin verification failed" + if !strings.Contains(err.Error(), expectedError) { + t.Errorf("Expected error message '%s', got: %s", expectedError, err.Error()) + } + + // Plugin should not be installed + if _, err := os.Stat(installer.Path()); !os.IsNotExist(err) { + t.Errorf("Plugin should not be installed when verification fails") + } +} + +func TestInstallWithOptions_NoVerifyRequested(t *testing.T) { + ensure.HelmHome(t) + + // Create a temporary plugin tarball without .prov file + pluginDir := createTestPluginDir(t) + pluginTgz := createTarballFromPluginDir(t, pluginDir) + defer os.Remove(pluginTgz) + + // Create local installer + installer, err := NewLocalInstaller(pluginTgz) + if err != nil { + t.Fatalf("Failed to create installer: %v", err) + } + defer os.RemoveAll(installer.Path()) + + // Install without verification (should succeed without any verification) + result, err := InstallWithOptions(installer, Options{Verify: false}) + + // Should succeed with no verification + if err != nil { + t.Fatalf("Expected installation without verification to succeed, got error: %v", err) + } + if result != nil { + t.Errorf("Expected nil verification result when verification is disabled, got: %+v", result) + } + + // Plugin should be installed + if _, err := os.Stat(installer.Path()); os.IsNotExist(err) { + t.Errorf("Plugin should be installed at %s", installer.Path()) + } +} + +func TestInstallWithOptions_VerifyDirectoryNotSupported(t *testing.T) { + ensure.HelmHome(t) + + // Create a directory-based plugin (not an archive) + pluginDir := createTestPluginDir(t) + + // Create local installer for directory + installer, err := NewLocalInstaller(pluginDir) + if err != nil { + t.Fatalf("Failed to create installer: %v", err) + } + defer os.RemoveAll(installer.Path()) + + // Install with verification should fail (directories don't support verification) + result, err := InstallWithOptions(installer, Options{Verify: true, Keyring: "dummy"}) + + // Should fail with verification not supported error + if err == nil { + t.Fatalf("Expected installation to fail with verification not supported error") + } + if !strings.Contains(err.Error(), "--verify is only supported for plugin tarballs") { + t.Errorf("Expected verification not supported error, got: %v", err) + } + if result != nil { + t.Errorf("Expected nil verification result when verification fails, got: %+v", result) + } +} + +func TestInstallWithOptions_VerifyMismatchedProvenance(t *testing.T) { + ensure.HelmHome(t) + + // Create plugin tarball + pluginDir := createTestPluginDir(t) + pluginTgz := createTarballFromPluginDir(t, pluginDir) + defer os.Remove(pluginTgz) + + provFile := pluginTgz + ".prov" + // Create provenance file with wrong hash (for a different file) + createProvFile(t, provFile, pluginTgz, "sha256:wronghash") + defer os.Remove(provFile) + + // Create keyring with test key + keyring := createTestKeyring(t) + defer os.Remove(keyring) + + // Create local installer + installer, err := NewLocalInstaller(pluginTgz) + if err != nil { + t.Fatalf("Failed to create installer: %v", err) + } + defer os.RemoveAll(installer.Path()) + + // Install with verification should fail due to hash mismatch + result, err := InstallWithOptions(installer, Options{Verify: true, Keyring: keyring}) + + // Should fail with verification error + if err == nil { + t.Fatalf("Expected installation to fail with hash mismatch") + } + if !strings.Contains(err.Error(), "plugin verification failed") { + t.Errorf("Expected plugin verification failed error, got: %v", err) + } + if result != nil { + t.Errorf("Expected nil verification result when verification fails, got: %+v", result) + } +} + +func TestInstallWithOptions_VerifyProvenanceAccessError(t *testing.T) { + ensure.HelmHome(t) + + // Create plugin tarball + pluginDir := createTestPluginDir(t) + pluginTgz := createTarballFromPluginDir(t, pluginDir) + defer os.Remove(pluginTgz) + + // Create a .prov file but make it inaccessible (simulate permission error) + provFile := pluginTgz + ".prov" + if err := os.WriteFile(provFile, []byte("test"), 0000); err != nil { + t.Fatalf("Failed to create inaccessible provenance file: %v", err) + } + defer os.Remove(provFile) + + // Create keyring + keyring := createTestKeyring(t) + defer os.Remove(keyring) + + // Create local installer + installer, err := NewLocalInstaller(pluginTgz) + if err != nil { + t.Fatalf("Failed to create installer: %v", err) + } + defer os.RemoveAll(installer.Path()) + + // Install with verification should fail due to access error + result, err := InstallWithOptions(installer, Options{Verify: true, Keyring: keyring}) + + // Should fail with access error (either at stat level or during verification) + if err == nil { + t.Fatalf("Expected installation to fail with provenance file access error") + } + // The error could be either "failed to access provenance file" or "plugin verification failed" + // depending on when the permission error occurs + if !strings.Contains(err.Error(), "failed to access provenance file") && + !strings.Contains(err.Error(), "plugin verification failed") { + t.Errorf("Expected provenance file access or verification error, got: %v", err) + } + if result != nil { + t.Errorf("Expected nil verification result when verification fails, got: %+v", result) + } +} + +// Helper functions for test setup + +func createTestPluginDir(t *testing.T) string { + t.Helper() + + // Create temporary directory with plugin structure + tmpDir := t.TempDir() + pluginDir := filepath.Join(tmpDir, "test-plugin") + if err := os.MkdirAll(pluginDir, 0755); err != nil { + t.Fatalf("Failed to create plugin directory: %v", err) + } + + // Create plugin.yaml using the standardized v1 format + pluginYaml := `apiVersion: v1 +name: test-plugin +type: cli/v1 +runtime: subprocess +version: 1.0.0 +runtimeConfig: + platformCommand: + - command: echo` + if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(pluginYaml), 0644); err != nil { + t.Fatalf("Failed to create plugin.yaml: %v", err) + } + + return pluginDir +} + +func createTarballFromPluginDir(t *testing.T, pluginDir string) string { + t.Helper() + + // Create tarball using the plugin package helper + tmpDir := filepath.Dir(pluginDir) + tgzPath := filepath.Join(tmpDir, "test-plugin-1.0.0.tgz") + tarFile, err := os.Create(tgzPath) + if err != nil { + t.Fatalf("Failed to create tarball file: %v", err) + } + defer tarFile.Close() + + if err := plugin.CreatePluginTarball(pluginDir, "test-plugin", tarFile); err != nil { + t.Fatalf("Failed to create tarball: %v", err) + } + + return tgzPath +} + +func createProvFile(t *testing.T, provFile, pluginTgz, hash string) { + t.Helper() + + var hashStr string + if hash == "" { + // Calculate actual hash of the tarball for realistic testing + data, err := os.ReadFile(pluginTgz) + if err != nil { + t.Fatalf("Failed to read tarball for hashing: %v", err) + } + hashSum := sha256.Sum256(data) + hashStr = fmt.Sprintf("sha256:%x", hashSum) + } else { + // Use provided hash (could be wrong for testing) + hashStr = hash + } + + // Create properly formatted provenance file with specified hash + provContent := fmt.Sprintf(`-----BEGIN PGP SIGNED MESSAGE----- +Hash: SHA256 + +name: test-plugin +version: 1.0.0 +description: Test plugin for verification +files: + test-plugin-1.0.0.tgz: %s +-----BEGIN PGP SIGNATURE----- +Version: GnuPG v1 + +iQEcBAEBCAAGBQJktest... +-----END PGP SIGNATURE----- +`, hashStr) + if err := os.WriteFile(provFile, []byte(provContent), 0644); err != nil { + t.Fatalf("Failed to create provenance file: %v", err) + } +} + +func createProvFileInvalidFormat(t *testing.T, provFile string) { + t.Helper() + + // Create an invalid provenance file (not PGP signed format) + invalidProv := "This is not a valid PGP signed message" + if err := os.WriteFile(provFile, []byte(invalidProv), 0644); err != nil { + t.Fatalf("Failed to create invalid provenance file: %v", err) + } +} + +func createTestKeyring(t *testing.T) string { + t.Helper() + + // Create a temporary keyring file + tmpDir := t.TempDir() + keyringPath := filepath.Join(tmpDir, "pubring.gpg") + + // Create empty keyring for testing + if err := os.WriteFile(keyringPath, []byte{}, 0644); err != nil { + t.Fatalf("Failed to create test keyring: %v", err) + } + + return keyringPath +} diff --git a/internal/plugin/sign.go b/internal/plugin/sign.go new file mode 100644 index 000000000..134c640e7 --- /dev/null +++ b/internal/plugin/sign.go @@ -0,0 +1,166 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugin + +import ( + "archive/tar" + "compress/gzip" + "errors" + "fmt" + "io" + "os" + "path/filepath" + + "sigs.k8s.io/yaml" + + "helm.sh/helm/v4/pkg/provenance" +) + +// SignPlugin signs a plugin using the SHA256 hash of the tarball. +// +// This is used when packaging and signing a plugin from a tarball file. +// It creates a signature that includes the tarball hash and plugin metadata, +// allowing verification of the original tarball later. +func SignPlugin(tarballPath string, signer *provenance.Signatory) (string, error) { + // Extract plugin metadata from tarball + pluginMeta, err := extractPluginMetadata(tarballPath) + if err != nil { + return "", fmt.Errorf("failed to extract plugin metadata: %w", err) + } + + // Marshal plugin metadata to YAML bytes + metadataBytes, err := yaml.Marshal(pluginMeta) + if err != nil { + return "", fmt.Errorf("failed to marshal plugin metadata: %w", err) + } + + // Use the generic provenance signing function + return signer.ClearSign(tarballPath, metadataBytes) +} + +// extractPluginMetadata extracts plugin metadata from a tarball +func extractPluginMetadata(tarballPath string) (*Metadata, error) { + f, err := os.Open(tarballPath) + if err != nil { + return nil, err + } + defer f.Close() + + return ExtractPluginMetadataFromReader(f) +} + +// ExtractPluginMetadataFromReader extracts plugin metadata from a tarball reader +func ExtractPluginMetadataFromReader(r io.Reader) (*Metadata, error) { + gzr, err := gzip.NewReader(r) + if err != nil { + return nil, err + } + defer gzr.Close() + + tr := tar.NewReader(gzr) + for { + header, err := tr.Next() + if err == io.EOF { + break + } + if err != nil { + return nil, err + } + + // Look for plugin.yaml file + if filepath.Base(header.Name) == "plugin.yaml" { + data, err := io.ReadAll(tr) + if err != nil { + return nil, err + } + + // Parse the plugin metadata + metadata, err := loadMetadata(data) + if err != nil { + return nil, err + } + + return metadata, nil + } + } + + return nil, errors.New("plugin.yaml not found in tarball") +} + +// parsePluginMessageBlock parses a signed message block to extract plugin metadata and checksums +func parsePluginMessageBlock(data []byte) (*Metadata, *provenance.SumCollection, error) { + sc := &provenance.SumCollection{} + + // We only need the checksums for verification, not the full metadata + if err := provenance.ParseMessageBlock(data, nil, sc); err != nil { + return nil, sc, err + } + return nil, sc, nil +} + +// CreatePluginTarball creates a gzipped tarball from a plugin directory +func CreatePluginTarball(sourceDir, pluginName string, w io.Writer) error { + gzw := gzip.NewWriter(w) + defer gzw.Close() + + tw := tar.NewWriter(gzw) + defer tw.Close() + + // Use the plugin name as the base directory in the tarball + baseDir := pluginName + + // Walk the directory tree + return filepath.Walk(sourceDir, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Create header + header, err := tar.FileInfoHeader(info, "") + if err != nil { + return err + } + + // Update the name to be relative to the source directory + relPath, err := filepath.Rel(sourceDir, path) + if err != nil { + return err + } + + // Include the base directory name in the tarball + header.Name = filepath.Join(baseDir, relPath) + + // Write header + if err := tw.WriteHeader(header); err != nil { + return err + } + + // If it's a regular file, write its content + if info.Mode().IsRegular() { + file, err := os.Open(path) + if err != nil { + return err + } + defer file.Close() + + if _, err := io.Copy(tw, file); err != nil { + return err + } + } + + return nil + }) +} diff --git a/internal/plugin/sign_test.go b/internal/plugin/sign_test.go new file mode 100644 index 000000000..a60970cdc --- /dev/null +++ b/internal/plugin/sign_test.go @@ -0,0 +1,92 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugin + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "helm.sh/helm/v4/pkg/provenance" +) + +func TestSignPlugin(t *testing.T) { + // Create a test plugin directory + tempDir := t.TempDir() + pluginDir := filepath.Join(tempDir, "test-plugin") + if err := os.MkdirAll(pluginDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a plugin.yaml file + pluginYAML := `apiVersion: v1 +name: test-plugin +type: cli/v1 +runtime: subprocess +version: 1.0.0 +runtimeConfig: + platformCommand: + - command: echo` + if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(pluginYAML), 0644); err != nil { + t.Fatal(err) + } + + // Create a tarball + tarballPath := filepath.Join(tempDir, "test-plugin.tgz") + tarFile, err := os.Create(tarballPath) + if err != nil { + t.Fatal(err) + } + if err := CreatePluginTarball(pluginDir, "test-plugin", tarFile); err != nil { + tarFile.Close() + t.Fatal(err) + } + tarFile.Close() + + // Create a test key for signing + keyring := "../../pkg/cmd/testdata/helm-test-key.secret" + signer, err := provenance.NewFromKeyring(keyring, "helm-test") + if err != nil { + t.Fatal(err) + } + if err := signer.DecryptKey(func(_ string) ([]byte, error) { + return []byte(""), nil + }); err != nil { + t.Fatal(err) + } + + // Sign the plugin tarball + sig, err := SignPlugin(tarballPath, signer) + if err != nil { + t.Fatalf("failed to sign plugin: %v", err) + } + + // Verify the signature contains the expected content + if !strings.Contains(sig, "-----BEGIN PGP SIGNED MESSAGE-----") { + t.Error("signature does not contain PGP header") + } + + // Verify the tarball hash is in the signature + expectedHash, err := provenance.DigestFile(tarballPath) + if err != nil { + t.Fatal(err) + } + // The signature should contain the tarball hash + if !strings.Contains(sig, "sha256:"+expectedHash) { + t.Errorf("signature does not contain expected tarball hash: sha256:%s", expectedHash) + } +} diff --git a/internal/plugin/signing_info.go b/internal/plugin/signing_info.go new file mode 100644 index 000000000..43d01c893 --- /dev/null +++ b/internal/plugin/signing_info.go @@ -0,0 +1,178 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugin + +import ( + "crypto/sha256" + "fmt" + "io" + "os" + "path/filepath" + "strings" + + "golang.org/x/crypto/openpgp/clearsign" //nolint + + "helm.sh/helm/v4/pkg/helmpath" +) + +// SigningInfo contains information about a plugin's signing status +type SigningInfo struct { + // Status can be: + // - "local dev": Plugin is a symlink (development mode) + // - "unsigned": No provenance file found + // - "invalid provenance": Provenance file is malformed + // - "mismatched provenance": Provenance file does not match the installed tarball + // - "signed": Valid signature exists for the installed tarball + Status string + IsSigned bool // True if plugin has a valid signature (even if not verified against keyring) +} + +// GetPluginSigningInfo returns signing information for an installed plugin +func GetPluginSigningInfo(metadata Metadata) (*SigningInfo, error) { + pluginName := metadata.Name + pluginDir := helmpath.DataPath("plugins", pluginName) + + // Check if plugin directory exists + fi, err := os.Lstat(pluginDir) + if err != nil { + return nil, fmt.Errorf("plugin %s not found: %w", pluginName, err) + } + + // Check if it's a symlink (local development) + if fi.Mode()&os.ModeSymlink != 0 { + return &SigningInfo{ + Status: "local dev", + IsSigned: false, + }, nil + } + + // Find the exact tarball file for this plugin + pluginsDir := helmpath.DataPath("plugins") + tarballPath := filepath.Join(pluginsDir, fmt.Sprintf("%s-%s.tgz", metadata.Name, metadata.Version)) + if _, err := os.Stat(tarballPath); err != nil { + return &SigningInfo{ + Status: "unsigned", + IsSigned: false, + }, nil + } + + // Check for .prov file associated with the tarball + provFile := tarballPath + ".prov" + provData, err := os.ReadFile(provFile) + if err != nil { + if os.IsNotExist(err) { + return &SigningInfo{ + Status: "unsigned", + IsSigned: false, + }, nil + } + return nil, fmt.Errorf("failed to read provenance file: %w", err) + } + + // Parse the provenance file to check validity + block, _ := clearsign.Decode(provData) + if block == nil { + return &SigningInfo{ + Status: "invalid provenance", + IsSigned: false, + }, nil + } + + // Check if provenance matches the actual tarball + blockContent := string(block.Plaintext) + if !validateProvenanceHash(blockContent, tarballPath) { + return &SigningInfo{ + Status: "mismatched provenance", + IsSigned: false, + }, nil + } + + // We have a provenance file that is valid for this plugin + // Without a keyring, we can't verify the signature, but we know: + // 1. A .prov file exists + // 2. It's a valid clearsigned document (cryptographically signed) + // 3. The provenance contains valid checksums + return &SigningInfo{ + Status: "signed", + IsSigned: true, + }, nil +} + +func validateProvenanceHash(blockContent string, tarballPath string) bool { + // Parse provenance to get the expected hash + _, sums, err := parsePluginMessageBlock([]byte(blockContent)) + if err != nil { + return false + } + + // Must have file checksums + if len(sums.Files) == 0 { + return false + } + + // Calculate actual hash of the tarball + actualHash, err := calculateFileHash(tarballPath) + if err != nil { + return false + } + + // Check if the actual hash matches the expected hash in the provenance + for filename, expectedHash := range sums.Files { + if strings.Contains(filename, filepath.Base(tarballPath)) && expectedHash == actualHash { + return true + } + } + + return false +} + +// calculateFileHash calculates the SHA256 hash of a file +func calculateFileHash(filePath string) (string, error) { + file, err := os.Open(filePath) + if err != nil { + return "", err + } + defer file.Close() + + hasher := sha256.New() + if _, err := io.Copy(hasher, file); err != nil { + return "", err + } + + return fmt.Sprintf("sha256:%x", hasher.Sum(nil)), nil +} + +// GetSigningInfoForPlugins returns signing info for multiple plugins +func GetSigningInfoForPlugins(plugins []Plugin) map[string]*SigningInfo { + result := make(map[string]*SigningInfo) + + for _, p := range plugins { + m := p.Metadata() + + info, err := GetPluginSigningInfo(m) + if err != nil { + // If there's an error, treat as unsigned + result[m.Name] = &SigningInfo{ + Status: "unknown", + IsSigned: false, + } + } else { + result[m.Name] = info + } + } + + return result +} diff --git a/internal/plugin/verify.go b/internal/plugin/verify.go new file mode 100644 index 000000000..e9656a3a6 --- /dev/null +++ b/internal/plugin/verify.go @@ -0,0 +1,72 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugin + +import ( + "errors" + "fmt" + "os" + "path/filepath" + + "helm.sh/helm/v4/pkg/provenance" +) + +// VerifyPlugin verifies a plugin tarball against a signature. +// +// This function verifies that a plugin tarball has a valid provenance file +// and that the provenance file is signed by a trusted entity. +func VerifyPlugin(pluginPath, keyring string) (*provenance.Verification, error) { + // Verify the plugin path exists + fi, err := os.Stat(pluginPath) + if err != nil { + return nil, err + } + + // Only support tarball verification + if fi.IsDir() { + return nil, errors.New("directory verification not supported - only plugin tarballs can be verified") + } + + // Verify it's a tarball + if !isTarball(pluginPath) { + return nil, errors.New("plugin file must be a gzipped tarball (.tar.gz or .tgz)") + } + + // Look for provenance file + provFile := pluginPath + ".prov" + if _, err := os.Stat(provFile); err != nil { + return nil, fmt.Errorf("could not find provenance file %s: %w", provFile, err) + } + + // Create signatory from keyring + sig, err := provenance.NewFromKeyring(keyring, "") + if err != nil { + return nil, err + } + + return verifyPluginTarball(pluginPath, provFile, sig) +} + +// verifyPluginTarball verifies a plugin tarball against its signature +func verifyPluginTarball(pluginPath, provPath string, sig *provenance.Signatory) (*provenance.Verification, error) { + // Reuse chart verification logic from pkg/provenance + return sig.Verify(pluginPath, provPath) +} + +// isTarball checks if a file has a tarball extension +func isTarball(filename string) bool { + return filepath.Ext(filename) == ".gz" || filepath.Ext(filename) == ".tgz" +} diff --git a/internal/plugin/verify_test.go b/internal/plugin/verify_test.go new file mode 100644 index 000000000..a09b35ec9 --- /dev/null +++ b/internal/plugin/verify_test.go @@ -0,0 +1,201 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package plugin + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "helm.sh/helm/v4/pkg/provenance" +) + +const testKeyFile = "../../pkg/cmd/testdata/helm-test-key.secret" +const testPubFile = "../../pkg/cmd/testdata/helm-test-key.pub" + +const testPluginYAML = `apiVersion: v1 +name: test-plugin +type: cli/v1 +runtime: subprocess +version: 1.0.0 +runtimeConfig: + platformCommand: + - command: echo` + +func TestVerifyPlugin(t *testing.T) { + // Create a test plugin and sign it + tempDir := t.TempDir() + + // Create plugin directory + pluginDir := filepath.Join(tempDir, "verify-test-plugin") + if err := os.MkdirAll(pluginDir, 0755); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(testPluginYAML), 0644); err != nil { + t.Fatal(err) + } + + // Create tarball + tarballPath := filepath.Join(tempDir, "verify-test-plugin.tar.gz") + tarFile, err := os.Create(tarballPath) + if err != nil { + t.Fatal(err) + } + + if err := CreatePluginTarball(pluginDir, "test-plugin", tarFile); err != nil { + tarFile.Close() + t.Fatal(err) + } + tarFile.Close() + + // Sign the plugin with source directory + signer, err := provenance.NewFromKeyring(testKeyFile, "helm-test") + if err != nil { + t.Fatal(err) + } + if err := signer.DecryptKey(func(_ string) ([]byte, error) { + return []byte(""), nil + }); err != nil { + t.Fatal(err) + } + + sig, err := SignPlugin(tarballPath, signer) + if err != nil { + t.Fatal(err) + } + + // Write the signature to .prov file + provFile := tarballPath + ".prov" + if err := os.WriteFile(provFile, []byte(sig), 0644); err != nil { + t.Fatal(err) + } + + // Now verify the plugin + verification, err := VerifyPlugin(tarballPath, testPubFile) + if err != nil { + t.Fatalf("Failed to verify plugin: %v", err) + } + + // Check verification results + if verification.SignedBy == nil { + t.Error("SignedBy is nil") + } + + if verification.FileName != "verify-test-plugin.tar.gz" { + t.Errorf("Expected filename 'verify-test-plugin.tar.gz', got %s", verification.FileName) + } + + if verification.FileHash == "" { + t.Error("FileHash is empty") + } +} + +func TestVerifyPluginBadSignature(t *testing.T) { + tempDir := t.TempDir() + + // Create a plugin tarball + pluginDir := filepath.Join(tempDir, "bad-plugin") + if err := os.MkdirAll(pluginDir, 0755); err != nil { + t.Fatal(err) + } + + if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(testPluginYAML), 0644); err != nil { + t.Fatal(err) + } + + tarballPath := filepath.Join(tempDir, "bad-plugin.tar.gz") + tarFile, err := os.Create(tarballPath) + if err != nil { + t.Fatal(err) + } + + if err := CreatePluginTarball(pluginDir, "test-plugin", tarFile); err != nil { + tarFile.Close() + t.Fatal(err) + } + tarFile.Close() + + // Create a bad signature (just some text) + badSig := `-----BEGIN PGP SIGNED MESSAGE----- +Hash: SHA512 + +This is not a real signature +-----BEGIN PGP SIGNATURE----- + +InvalidSignatureData + +-----END PGP SIGNATURE-----` + + provFile := tarballPath + ".prov" + if err := os.WriteFile(provFile, []byte(badSig), 0644); err != nil { + t.Fatal(err) + } + + // Try to verify - should fail + _, err = VerifyPlugin(tarballPath, testPubFile) + if err == nil { + t.Error("Expected verification to fail with bad signature") + } +} + +func TestVerifyPluginMissingProvenance(t *testing.T) { + tempDir := t.TempDir() + tarballPath := filepath.Join(tempDir, "no-prov.tar.gz") + + // Create a minimal tarball + if err := os.WriteFile(tarballPath, []byte("dummy"), 0644); err != nil { + t.Fatal(err) + } + + // Try to verify without .prov file + _, err := VerifyPlugin(tarballPath, testPubFile) + if err == nil { + t.Error("Expected verification to fail without provenance file") + } +} + +func TestVerifyPluginDirectory(t *testing.T) { + // Create a test plugin directory + tempDir := t.TempDir() + pluginDir := filepath.Join(tempDir, "test-plugin") + if err := os.MkdirAll(pluginDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a plugin.yaml file + if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(testPluginYAML), 0644); err != nil { + t.Fatal(err) + } + + // Attempt to verify the directory - should fail + _, err := VerifyPlugin(pluginDir, testPubFile) + if err == nil { + t.Error("Expected directory verification to fail, but it succeeded") + } + + expectedError := "directory verification not supported" + if !containsString(err.Error(), expectedError) { + t.Errorf("Expected error to contain %q, got %q", expectedError, err.Error()) + } +} + +func containsString(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(s) > len(substr) && + (s[:len(substr)] == substr || s[len(s)-len(substr):] == substr || + strings.Contains(s, substr))) +} diff --git a/pkg/action/package.go b/pkg/action/package.go index e57ce4921..c59efcdb3 100644 --- a/pkg/action/package.go +++ b/pkg/action/package.go @@ -25,6 +25,7 @@ import ( "github.com/Masterminds/semver/v3" "golang.org/x/term" + "sigs.k8s.io/yaml" "helm.sh/helm/v4/pkg/chart/v2/loader" chartutil "helm.sh/helm/v4/pkg/chart/v2/util" @@ -143,7 +144,20 @@ func (p *Package) Clearsign(filename string) error { return err } - sig, err := signer.ClearSign(filename) + // Load the chart archive to extract metadata + chart, err := loader.LoadFile(filename) + if err != nil { + return fmt.Errorf("failed to load chart for signing: %w", err) + } + + // Marshal chart metadata to YAML bytes + metadataBytes, err := yaml.Marshal(chart.Metadata) + if err != nil { + return fmt.Errorf("failed to marshal chart metadata: %w", err) + } + + // Use the generic provenance signing function + sig, err := signer.ClearSign(filename, metadataBytes) if err != nil { return err } diff --git a/pkg/cmd/plugin.go b/pkg/cmd/plugin.go index b03000ad4..393e9672c 100644 --- a/pkg/cmd/plugin.go +++ b/pkg/cmd/plugin.go @@ -38,6 +38,8 @@ func newPluginCmd(out io.Writer) *cobra.Command { newPluginListCmd(out), newPluginUninstallCmd(out), newPluginUpdateCmd(out), + newPluginPackageCmd(out), + newPluginVerifyCmd(out), ) return cmd } diff --git a/pkg/cmd/plugin_install.go b/pkg/cmd/plugin_install.go index 960404a76..0abefa76b 100644 --- a/pkg/cmd/plugin_install.go +++ b/pkg/cmd/plugin_install.go @@ -33,6 +33,9 @@ import ( type pluginInstallOptions struct { source string version string + // signing options + verify bool + keyring string // OCI-specific options certFile string keyFile string @@ -45,6 +48,13 @@ type pluginInstallOptions struct { const pluginInstallDesc = ` This command allows you to install a plugin from a url to a VCS repo or a local path. + +By default, plugin signatures are verified before installation when installing from +tarballs (.tgz or .tar.gz). This requires a corresponding .prov file to be available +alongside the tarball. +For local development, plugins installed from local directories are automatically +treated as "local dev" and do not require signatures. +Use --verify=false to skip signature verification for remote plugins. ` func newPluginInstallCmd(out io.Writer) *cobra.Command { @@ -71,6 +81,8 @@ func newPluginInstallCmd(out io.Writer) *cobra.Command { }, } cmd.Flags().StringVar(&o.version, "version", "", "specify a version constraint. If this is not specified, the latest version is installed") + cmd.Flags().BoolVar(&o.verify, "verify", true, "verify the plugin signature before installing") + cmd.Flags().StringVar(&o.keyring, "keyring", defaultKeyring(), "location of public keys used for verification") // Add OCI-specific flags cmd.Flags().StringVar(&o.certFile, "cert-file", "", "identify registry client using this SSL certificate file") @@ -113,10 +125,51 @@ func (o *pluginInstallOptions) run(out io.Writer) error { if err != nil { return err } - if err := installer.Install(i); err != nil { + + // Determine if we should verify based on installer type and flags + shouldVerify := o.verify + + // Check if this is a local directory installation (for development) + if localInst, ok := i.(*installer.LocalInstaller); ok && !localInst.SupportsVerification() { + // Local directory installations are allowed without verification + shouldVerify = false + fmt.Fprintf(out, "Installing plugin from local directory (development mode)\n") + } else if shouldVerify { + // For remote installations, check if verification is supported + if verifier, ok := i.(installer.Verifier); !ok || !verifier.SupportsVerification() { + return fmt.Errorf("plugin source does not support verification. Use --verify=false to skip verification") + } + } else { + // User explicitly disabled verification + fmt.Fprintf(out, "WARNING: Skipping plugin signature verification\n") + } + + // Set up installation options + opts := installer.Options{ + Verify: shouldVerify, + Keyring: o.keyring, + } + + // If verify is requested, show verification output + if shouldVerify { + fmt.Fprintf(out, "Verifying plugin signature...\n") + } + + // Install the plugin with options + verifyResult, err := installer.InstallWithOptions(i, opts) + if err != nil { return err } + // If verification was successful, show the details + if verifyResult != nil { + for _, signer := range verifyResult.SignedBy { + fmt.Fprintf(out, "Signed by: %s\n", signer) + } + fmt.Fprintf(out, "Using Key With Fingerprint: %s\n", verifyResult.Fingerprint) + fmt.Fprintf(out, "Plugin Hash Verified: %s\n", verifyResult.FileHash) + } + slog.Debug("loading plugin", "path", i.Path()) p, err := plugin.LoadDir(i.Path()) if err != nil { diff --git a/pkg/cmd/plugin_list.go b/pkg/cmd/plugin_list.go index 31a76330d..9b2895441 100644 --- a/pkg/cmd/plugin_list.go +++ b/pkg/cmd/plugin_list.go @@ -46,15 +46,23 @@ func newPluginListCmd(out io.Writer) *cobra.Command { return err } + // Get signing info for all plugins + signingInfo := plugin.GetSigningInfoForPlugins(plugins) + table := uitable.New() - table.AddRow("NAME", "VERSION", "TYPE", "APIVERSION", "SOURCE") + table.AddRow("NAME", "VERSION", "TYPE", "APIVERSION", "PROVENANCE", "SOURCE") for _, p := range plugins { m := p.Metadata() sourceURL := m.SourceURL if sourceURL == "" { sourceURL = "unknown" } - table.AddRow(m.Name, m.Version, m.Type, m.APIVersion, sourceURL) + // Get signing status + signedStatus := "unknown" + if info, ok := signingInfo[m.Name]; ok { + signedStatus = info.Status + } + table.AddRow(m.Name, m.Version, m.Type, m.APIVersion, signedStatus, sourceURL) } fmt.Fprintln(out, table) return nil diff --git a/pkg/cmd/plugin_package.go b/pkg/cmd/plugin_package.go new file mode 100644 index 000000000..5da6c624e --- /dev/null +++ b/pkg/cmd/plugin_package.go @@ -0,0 +1,209 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd + +import ( + "bytes" + "errors" + "fmt" + "io" + "os" + "path/filepath" + "syscall" + + "github.com/spf13/cobra" + "golang.org/x/term" + + "helm.sh/helm/v4/internal/plugin" + "helm.sh/helm/v4/pkg/cmd/require" + "helm.sh/helm/v4/pkg/provenance" +) + +const pluginPackageDesc = ` +This command packages a Helm plugin directory into a tarball. + +By default, the command will generate a provenance file signed with a PGP key. +This ensures the plugin can be verified after installation. + +Use --sign=false to skip signing (not recommended for distribution). +` + +type pluginPackageOptions struct { + sign bool + keyring string + key string + passphraseFile string + pluginPath string + destination string +} + +func newPluginPackageCmd(out io.Writer) *cobra.Command { + o := &pluginPackageOptions{} + + cmd := &cobra.Command{ + Use: "package [PATH]", + Short: "package a plugin directory into a plugin archive", + Long: pluginPackageDesc, + Args: require.ExactArgs(1), + RunE: func(_ *cobra.Command, args []string) error { + o.pluginPath = args[0] + return o.run(out) + }, + } + + f := cmd.Flags() + f.BoolVar(&o.sign, "sign", true, "use a PGP private key to sign this plugin") + f.StringVar(&o.key, "key", "", "name of the key to use when signing. Used if --sign is true") + f.StringVar(&o.keyring, "keyring", defaultKeyring(), "location of a public keyring") + f.StringVar(&o.passphraseFile, "passphrase-file", "", "location of a file which contains the passphrase for the signing key. Use \"-\" to read from stdin.") + f.StringVarP(&o.destination, "destination", "d", ".", "location to write the plugin tarball.") + + return cmd +} + +func (o *pluginPackageOptions) run(out io.Writer) error { + // Check if the plugin path exists and is a directory + fi, err := os.Stat(o.pluginPath) + if err != nil { + return err + } + if !fi.IsDir() { + return fmt.Errorf("plugin package only supports directories, not tarballs") + } + + // Load and validate plugin metadata + pluginMeta, err := plugin.LoadDir(o.pluginPath) + if err != nil { + return fmt.Errorf("invalid plugin directory: %w", err) + } + + // Create destination directory if needed + if err := os.MkdirAll(o.destination, 0755); err != nil { + return err + } + + // If signing is requested, prepare the signer first + var signer *provenance.Signatory + if o.sign { + // Load the signing key + signer, err = provenance.NewFromKeyring(o.keyring, o.key) + if err != nil { + return fmt.Errorf("error reading from keyring: %w", err) + } + + // Get passphrase + passphraseFetcher := o.promptUser + if o.passphraseFile != "" { + passphraseFetcher, err = o.passphraseFileFetcher() + if err != nil { + return err + } + } + + // Decrypt the key + if err := signer.DecryptKey(passphraseFetcher); err != nil { + return err + } + } else { + // User explicitly disabled signing + fmt.Fprintf(out, "WARNING: Skipping plugin signing. This is not recommended for plugins intended for distribution.\n") + } + + // Now create the tarball (only after signing prerequisites are met) + // Use plugin metadata for filename: PLUGIN_NAME-SEMVER.tgz + metadata := pluginMeta.Metadata() + filename := fmt.Sprintf("%s-%s.tgz", metadata.Name, metadata.Version) + tarballPath := filepath.Join(o.destination, filename) + + tarFile, err := os.Create(tarballPath) + if err != nil { + return fmt.Errorf("failed to create tarball: %w", err) + } + defer tarFile.Close() + + if err := plugin.CreatePluginTarball(o.pluginPath, metadata.Name, tarFile); err != nil { + os.Remove(tarballPath) + return fmt.Errorf("failed to create plugin tarball: %w", err) + } + tarFile.Close() // Ensure file is closed before signing + + // If signing was requested, sign the tarball + if o.sign { + // Sign the plugin tarball (not the source directory) + sig, err := plugin.SignPlugin(tarballPath, signer) + if err != nil { + os.Remove(tarballPath) + return fmt.Errorf("failed to sign plugin: %w", err) + } + + // Write the signature + provFile := tarballPath + ".prov" + if err := os.WriteFile(provFile, []byte(sig), 0644); err != nil { + os.Remove(tarballPath) + return err + } + + fmt.Fprintf(out, "Successfully signed. Signature written to: %s\n", provFile) + } + + fmt.Fprintf(out, "Successfully packaged plugin and saved it to: %s\n", tarballPath) + + return nil +} + +func (o *pluginPackageOptions) promptUser(name string) ([]byte, error) { + fmt.Printf("Password for key %q > ", name) + pw, err := term.ReadPassword(int(syscall.Stdin)) + fmt.Println() + return pw, err +} + +func (o *pluginPackageOptions) passphraseFileFetcher() (provenance.PassphraseFetcher, error) { + file, err := openPassphraseFile(o.passphraseFile, os.Stdin) + if err != nil { + return nil, err + } + defer file.Close() + + // Read the entire passphrase + passphrase, err := io.ReadAll(file) + if err != nil { + return nil, err + } + + // Trim any trailing newline characters (both \n and \r\n) + passphrase = bytes.TrimRight(passphrase, "\r\n") + + return func(_ string) ([]byte, error) { + return passphrase, nil + }, nil +} + +// copied from action.openPassphraseFile +// TODO: should we move this to pkg/action so we can reuse the func from there? +func openPassphraseFile(passphraseFile string, stdin *os.File) (*os.File, error) { + if passphraseFile == "-" { + stat, err := stdin.Stat() + if err != nil { + return nil, err + } + if (stat.Mode() & os.ModeNamedPipe) == 0 { + return nil, errors.New("specified reading passphrase from stdin, without input on stdin") + } + return stdin, nil + } + return os.Open(passphraseFile) +} diff --git a/pkg/cmd/plugin_package_test.go b/pkg/cmd/plugin_package_test.go new file mode 100644 index 000000000..df6cdd849 --- /dev/null +++ b/pkg/cmd/plugin_package_test.go @@ -0,0 +1,170 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" +) + +// Common plugin.yaml content for v1 format tests +const testPluginYAML = `apiVersion: v1 +name: test-plugin +version: 1.0.0 +type: cli/v1 +runtime: subprocess +config: + usage: test-plugin [flags] + shortHelp: A test plugin + longHelp: A test plugin for testing purposes +runtimeConfig: + platformCommands: + - os: linux + command: echo + args: ["test"]` + +func TestPluginPackageWithoutSigning(t *testing.T) { + // Create a test plugin directory + tempDir := t.TempDir() + pluginDir := filepath.Join(tempDir, "test-plugin") + if err := os.MkdirAll(pluginDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a plugin.yaml file + if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(testPluginYAML), 0644); err != nil { + t.Fatal(err) + } + + // Create package options with sign=false + o := &pluginPackageOptions{ + sign: false, // Explicitly disable signing + pluginPath: pluginDir, + destination: tempDir, + } + + // Run the package command + out := &bytes.Buffer{} + err := o.run(out) + + // Should succeed without error + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + // Check that tarball was created with plugin name and version + tarballPath := filepath.Join(tempDir, "test-plugin-1.0.0.tgz") + if _, err := os.Stat(tarballPath); os.IsNotExist(err) { + t.Error("tarball should exist when sign=false") + } + + // Check that no .prov file was created + provPath := tarballPath + ".prov" + if _, err := os.Stat(provPath); !os.IsNotExist(err) { + t.Error("provenance file should not exist when sign=false") + } + + // Output should contain warning about skipping signing + output := out.String() + if !strings.Contains(output, "WARNING: Skipping plugin signing") { + t.Error("should print warning when signing is skipped") + } + if !strings.Contains(output, "Successfully packaged") { + t.Error("should print success message") + } +} + +func TestPluginPackageDefaultRequiresSigning(t *testing.T) { + // Create a test plugin directory + tempDir := t.TempDir() + pluginDir := filepath.Join(tempDir, "test-plugin") + if err := os.MkdirAll(pluginDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a plugin.yaml file + if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(testPluginYAML), 0644); err != nil { + t.Fatal(err) + } + + // Create package options with default sign=true and invalid keyring + o := &pluginPackageOptions{ + sign: true, // This is now the default + keyring: "/non/existent/keyring", + pluginPath: pluginDir, + destination: tempDir, + } + + // Run the package command + out := &bytes.Buffer{} + err := o.run(out) + + // Should fail because signing is required by default + if err == nil { + t.Error("expected error when signing fails with default settings") + } + + // Check that no tarball was created + tarballPath := filepath.Join(tempDir, "test-plugin.tgz") + if _, err := os.Stat(tarballPath); !os.IsNotExist(err) { + t.Error("tarball should not exist when signing fails") + } +} + +func TestPluginPackageSigningFailure(t *testing.T) { + // Create a test plugin directory + tempDir := t.TempDir() + pluginDir := filepath.Join(tempDir, "test-plugin") + if err := os.MkdirAll(pluginDir, 0755); err != nil { + t.Fatal(err) + } + + // Create a plugin.yaml file + if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(testPluginYAML), 0644); err != nil { + t.Fatal(err) + } + + // Create package options with sign flag but invalid keyring + o := &pluginPackageOptions{ + sign: true, + keyring: "/non/existent/keyring", // This will cause signing to fail + pluginPath: pluginDir, + destination: tempDir, + } + + // Run the package command + out := &bytes.Buffer{} + err := o.run(out) + + // Should get an error + if err == nil { + t.Error("expected error when signing fails, got nil") + } + + // Check that no tarball was created + tarballPath := filepath.Join(tempDir, "test-plugin.tgz") + if _, err := os.Stat(tarballPath); !os.IsNotExist(err) { + t.Error("tarball should not exist when signing fails") + } + + // Output should not contain success message + if bytes.Contains(out.Bytes(), []byte("Successfully packaged")) { + t.Error("should not print success message when signing fails") + } +} diff --git a/pkg/cmd/plugin_verify.go b/pkg/cmd/plugin_verify.go new file mode 100644 index 000000000..4772fcc33 --- /dev/null +++ b/pkg/cmd/plugin_verify.go @@ -0,0 +1,88 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd + +import ( + "fmt" + "io" + + "github.com/spf13/cobra" + + "helm.sh/helm/v4/internal/plugin" + "helm.sh/helm/v4/pkg/cmd/require" +) + +const pluginVerifyDesc = ` +This command verifies that a Helm plugin has a valid provenance file, +and that the provenance file is signed by a trusted PGP key. + +It supports both: +- Plugin tarballs (.tgz or .tar.gz files) +- Installed plugin directories + +For installed plugins, use the path shown by 'helm env HELM_PLUGINS' followed +by the plugin name. For example: + helm plugin verify ~/.local/share/helm/plugins/example-cli + +To generate a signed plugin, use the 'helm plugin package --sign' command. +` + +type pluginVerifyOptions struct { + keyring string + pluginPath string +} + +func newPluginVerifyCmd(out io.Writer) *cobra.Command { + o := &pluginVerifyOptions{} + + cmd := &cobra.Command{ + Use: "verify [PATH]", + Short: "verify that a plugin at the given path has been signed and is valid", + Long: pluginVerifyDesc, + Args: require.ExactArgs(1), + RunE: func(_ *cobra.Command, args []string) error { + o.pluginPath = args[0] + return o.run(out) + }, + } + + cmd.Flags().StringVar(&o.keyring, "keyring", defaultKeyring(), "keyring containing public keys") + + return cmd +} + +func (o *pluginVerifyOptions) run(out io.Writer) error { + // Verify the plugin + verification, err := plugin.VerifyPlugin(o.pluginPath, o.keyring) + if err != nil { + return err + } + + // Output verification details + for name := range verification.SignedBy.Identities { + fmt.Fprintf(out, "Signed by: %v\n", name) + } + fmt.Fprintf(out, "Using Key With Fingerprint: %X\n", verification.SignedBy.PrimaryKey.Fingerprint) + + // Only show hash for tarballs + if verification.FileHash != "" { + fmt.Fprintf(out, "Plugin Hash Verified: %s\n", verification.FileHash) + } else { + fmt.Fprintf(out, "Plugin Metadata Verified: %s\n", verification.FileName) + } + + return nil +} diff --git a/pkg/cmd/plugin_verify_test.go b/pkg/cmd/plugin_verify_test.go new file mode 100644 index 000000000..e631814dd --- /dev/null +++ b/pkg/cmd/plugin_verify_test.go @@ -0,0 +1,264 @@ +/* +Copyright The Helm Authors. +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cmd + +import ( + "bytes" + "crypto/sha256" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "helm.sh/helm/v4/internal/plugin" + "helm.sh/helm/v4/internal/test/ensure" +) + +func TestPluginVerifyCmd_NoArgs(t *testing.T) { + ensure.HelmHome(t) + + out := &bytes.Buffer{} + cmd := newPluginVerifyCmd(out) + cmd.SetArgs([]string{}) + + err := cmd.Execute() + if err == nil { + t.Error("expected error when no arguments provided") + } + if !strings.Contains(err.Error(), "requires 1 argument") { + t.Errorf("expected 'requires 1 argument' error, got: %v", err) + } +} + +func TestPluginVerifyCmd_TooManyArgs(t *testing.T) { + ensure.HelmHome(t) + + out := &bytes.Buffer{} + cmd := newPluginVerifyCmd(out) + cmd.SetArgs([]string{"plugin1", "plugin2"}) + + err := cmd.Execute() + if err == nil { + t.Error("expected error when too many arguments provided") + } + if !strings.Contains(err.Error(), "requires 1 argument") { + t.Errorf("expected 'requires 1 argument' error, got: %v", err) + } +} + +func TestPluginVerifyCmd_NonexistentFile(t *testing.T) { + ensure.HelmHome(t) + + out := &bytes.Buffer{} + cmd := newPluginVerifyCmd(out) + cmd.SetArgs([]string{"/nonexistent/plugin.tgz"}) + + err := cmd.Execute() + if err == nil { + t.Error("expected error when plugin file doesn't exist") + } +} + +func TestPluginVerifyCmd_MissingProvenance(t *testing.T) { + ensure.HelmHome(t) + + // Create a plugin tarball without .prov file + pluginTgz := createTestPluginTarball(t) + defer os.Remove(pluginTgz) + + out := &bytes.Buffer{} + cmd := newPluginVerifyCmd(out) + cmd.SetArgs([]string{pluginTgz}) + + err := cmd.Execute() + if err == nil { + t.Error("expected error when .prov file is missing") + } + if !strings.Contains(err.Error(), "could not find provenance file") { + t.Errorf("expected 'could not find provenance file' error, got: %v", err) + } +} + +func TestPluginVerifyCmd_InvalidProvenance(t *testing.T) { + ensure.HelmHome(t) + + // Create a plugin tarball with invalid .prov file + pluginTgz := createTestPluginTarball(t) + defer os.Remove(pluginTgz) + + // Create invalid .prov file + provFile := pluginTgz + ".prov" + if err := os.WriteFile(provFile, []byte("invalid provenance"), 0644); err != nil { + t.Fatal(err) + } + defer os.Remove(provFile) + + out := &bytes.Buffer{} + cmd := newPluginVerifyCmd(out) + cmd.SetArgs([]string{pluginTgz}) + + err := cmd.Execute() + if err == nil { + t.Error("expected error when .prov file is invalid") + } +} + +func TestPluginVerifyCmd_DirectoryNotSupported(t *testing.T) { + ensure.HelmHome(t) + + // Create a plugin directory + pluginDir := createTestPluginDir(t) + + out := &bytes.Buffer{} + cmd := newPluginVerifyCmd(out) + cmd.SetArgs([]string{pluginDir}) + + err := cmd.Execute() + if err == nil { + t.Error("expected error when verifying directory") + } + if !strings.Contains(err.Error(), "directory verification not supported") { + t.Errorf("expected 'directory verification not supported' error, got: %v", err) + } +} + +func TestPluginVerifyCmd_KeyringFlag(t *testing.T) { + ensure.HelmHome(t) + + // Create a plugin tarball with .prov file + pluginTgz := createTestPluginTarball(t) + defer os.Remove(pluginTgz) + + // Create .prov file + provFile := pluginTgz + ".prov" + createProvFile(t, provFile, pluginTgz, "") + defer os.Remove(provFile) + + // Create empty keyring file + keyring := createTestKeyring(t) + defer os.Remove(keyring) + + out := &bytes.Buffer{} + cmd := newPluginVerifyCmd(out) + cmd.SetArgs([]string{"--keyring", keyring, pluginTgz}) + + // Should fail with keyring error but command parsing should work + err := cmd.Execute() + if err == nil { + t.Error("expected error with empty keyring") + } + // The important thing is that the keyring flag was parsed and used +} + +func TestPluginVerifyOptions_Run_Success(t *testing.T) { + // Skip this test as it would require real PGP keys and valid signatures + // The core verification logic is thoroughly tested in internal/plugin/verify_test.go + t.Skip("Success case requires real PGP keys - core logic tested in internal/plugin/verify_test.go") +} + +// Helper functions for test setup + +func createTestPluginDir(t *testing.T) string { + t.Helper() + + // Create temporary directory with plugin structure + tmpDir := t.TempDir() + pluginDir := filepath.Join(tmpDir, "test-plugin") + if err := os.MkdirAll(pluginDir, 0755); err != nil { + t.Fatalf("Failed to create plugin directory: %v", err) + } + + // Use the same plugin YAML as other cmd tests + if err := os.WriteFile(filepath.Join(pluginDir, "plugin.yaml"), []byte(testPluginYAML), 0644); err != nil { + t.Fatalf("Failed to create plugin.yaml: %v", err) + } + + return pluginDir +} + +func createTestPluginTarball(t *testing.T) string { + t.Helper() + + pluginDir := createTestPluginDir(t) + + // Create tarball using the plugin package helper + tmpDir := filepath.Dir(pluginDir) + tgzPath := filepath.Join(tmpDir, "test-plugin-1.0.0.tgz") + tarFile, err := os.Create(tgzPath) + if err != nil { + t.Fatalf("Failed to create tarball file: %v", err) + } + defer tarFile.Close() + + if err := plugin.CreatePluginTarball(pluginDir, "test-plugin", tarFile); err != nil { + t.Fatalf("Failed to create tarball: %v", err) + } + + return tgzPath +} + +func createProvFile(t *testing.T, provFile, pluginTgz, hash string) { + t.Helper() + + var hashStr string + if hash == "" { + // Calculate actual hash of the tarball + data, err := os.ReadFile(pluginTgz) + if err != nil { + t.Fatalf("Failed to read tarball for hashing: %v", err) + } + hashSum := sha256.Sum256(data) + hashStr = fmt.Sprintf("sha256:%x", hashSum) + } else { + // Use provided hash + hashStr = hash + } + + // Create properly formatted provenance file with specified hash + provContent := fmt.Sprintf(`-----BEGIN PGP SIGNED MESSAGE----- +Hash: SHA256 + +name: test-plugin +version: 1.0.0 +description: Test plugin for verification +files: + test-plugin-1.0.0.tgz: %s +-----BEGIN PGP SIGNATURE----- +Version: GnuPG v1 + +iQEcBAEBCAAGBQJktest... +-----END PGP SIGNATURE----- +`, hashStr) + if err := os.WriteFile(provFile, []byte(provContent), 0644); err != nil { + t.Fatalf("Failed to create provenance file: %v", err) + } +} + +func createTestKeyring(t *testing.T) string { + t.Helper() + + // Create a temporary keyring file + tmpDir := t.TempDir() + keyringPath := filepath.Join(tmpDir, "pubring.gpg") + + // Create empty keyring for testing + if err := os.WriteFile(keyringPath, []byte{}, 0644); err != nil { + t.Fatalf("Failed to create test keyring: %v", err) + } + + return keyringPath +} diff --git a/pkg/getter/ocigetter.go b/pkg/getter/ocigetter.go index 121e000c8..24fc60c56 100644 --- a/pkg/getter/ocigetter.go +++ b/pkg/getter/ocigetter.go @@ -175,6 +175,12 @@ func (g *OCIGetter) newRegistryClient() (*registry.Client, error) { // getPlugin handles plugin-specific OCI pulls func (g *OCIGetter) getPlugin(client *registry.Client, ref string) (*bytes.Buffer, error) { + // Check if this is a provenance file request + requestingProv := strings.HasSuffix(ref, ".prov") + if requestingProv { + ref = strings.TrimSuffix(ref, ".prov") + } + // Extract plugin name from the reference // e.g., "ghcr.io/user/plugin-name:v1.0.0" -> "plugin-name" parts := strings.Split(ref, "/") @@ -190,10 +196,18 @@ func (g *OCIGetter) getPlugin(client *registry.Client, ref string) (*bytes.Buffe pluginName = lastPart[:idx] } - result, err := client.PullPlugin(ref, pluginName) + var pullOpts []registry.PluginPullOption + if requestingProv { + pullOpts = append(pullOpts, registry.PullPluginOptWithProv(true)) + } + + result, err := client.PullPlugin(ref, pluginName, pullOpts...) if err != nil { return nil, err } + if requestingProv { + return bytes.NewBuffer(result.Prov.Data), nil + } return bytes.NewBuffer(result.PluginData), nil } diff --git a/pkg/provenance/doc.go b/pkg/provenance/doc.go index 883c0e724..dd14568d9 100644 --- a/pkg/provenance/doc.go +++ b/pkg/provenance/doc.go @@ -14,15 +14,15 @@ limitations under the License. */ /* -Package provenance provides tools for establishing the authenticity of a chart. +Package provenance provides tools for establishing the authenticity of packages. In Helm, provenance is established via several factors. The primary factor is the -cryptographic signature of a chart. Chart authors may sign charts, which in turn -provide the necessary metadata to ensure the integrity of the chart file, the -Chart.yaml, and the referenced Docker images. +cryptographic signature of a package. Package authors may sign packages, which in turn +provide the necessary metadata to ensure the integrity of the package file, the +metadata, and the referenced Docker images. A provenance file is clear-signed. This provides cryptographic verification that -a particular block of information (Chart.yaml, archive file, images) have not +a particular block of information (metadata, archive file, images) have not been tampered with or altered. To learn more, read the GnuPG documentation on clear signatures: https://www.gnupg.org/gph/en/manual/x135.html diff --git a/pkg/provenance/sign.go b/pkg/provenance/sign.go index 504bc6aa1..103c81fbb 100644 --- a/pkg/provenance/sign.go +++ b/pkg/provenance/sign.go @@ -30,9 +30,6 @@ import ( "golang.org/x/crypto/openpgp/clearsign" //nolint "golang.org/x/crypto/openpgp/packet" //nolint "sigs.k8s.io/yaml" - - hapi "helm.sh/helm/v4/pkg/chart/v2" - "helm.sh/helm/v4/pkg/chart/v2/loader" ) var defaultPGPConfig = packet.Config{ @@ -58,7 +55,7 @@ type SumCollection struct { // Verification contains information about a verification operation. type Verification struct { - // SignedBy contains the entity that signed a chart. + // SignedBy contains the entity that signed a package. SignedBy *openpgp.Entity // FileHash is the hash, prepended with the scheme, for the file that was verified. FileHash string @@ -68,11 +65,11 @@ type Verification struct { // Signatory signs things. // -// Signatories can be constructed from a PGP private key file using NewFromFiles +// Signatories can be constructed from a PGP private key file using NewFromFiles, // or they can be constructed manually by setting the Entity to a valid // PGP entity. // -// The same Signatory can be used to sign or validate multiple charts. +// The same Signatory can be used to sign or validate multiple packages. type Signatory struct { // The signatory for this instance of Helm. This is used for signing. Entity *openpgp.Entity @@ -197,20 +194,21 @@ func (s *Signatory) DecryptKey(fn PassphraseFetcher) error { return s.Entity.PrivateKey.Decrypt(p) } -// ClearSign signs a chart with the given key. +// ClearSign signs a package with the given key and pre-marshalled metadata. // -// This takes the path to a chart archive file and a key, and it returns a clear signature. +// This takes the path to a package archive file, a key, and marshalled metadata bytes. +// This allows both charts and plugins to use the same signing infrastructure. // // The Signatory must have a valid Entity.PrivateKey for this to work. If it does // not, an error will be returned. -func (s *Signatory) ClearSign(chartpath string) (string, error) { +func (s *Signatory) ClearSign(packagePath string, metadataBytes []byte) (string, error) { if s.Entity == nil { return "", errors.New("private key not found") } else if s.Entity.PrivateKey == nil { return "", errors.New("provided key is not a private key. Try providing a keyring with secret keys") } - if fi, err := os.Stat(chartpath); err != nil { + if fi, err := os.Stat(packagePath); err != nil { return "", err } else if fi.IsDir() { return "", errors.New("cannot sign a directory") @@ -218,7 +216,7 @@ func (s *Signatory) ClearSign(chartpath string) (string, error) { out := bytes.NewBuffer(nil) - b, err := messageBlock(chartpath) + b, err := messageBlock(packagePath, metadataBytes) if err != nil { return "", err } @@ -248,10 +246,10 @@ func (s *Signatory) ClearSign(chartpath string) (string, error) { return out.String(), nil } -// Verify checks a signature and verifies that it is legit for a chart. -func (s *Signatory) Verify(chartpath, sigpath string) (*Verification, error) { +// Verify checks a signature and verifies that it is legit for a package. +func (s *Signatory) Verify(packagePath, sigpath string) (*Verification, error) { ver := &Verification{} - for _, fname := range []string{chartpath, sigpath} { + for _, fname := range []string{packagePath, sigpath} { if fi, err := os.Stat(fname); err != nil { return ver, err } else if fi.IsDir() { @@ -272,17 +270,17 @@ func (s *Signatory) Verify(chartpath, sigpath string) (*Verification, error) { ver.SignedBy = by // Second, verify the hash of the tarball. - sum, err := DigestFile(chartpath) + sum, err := DigestFile(packagePath) if err != nil { return ver, err } - _, sums, err := parseMessageBlock(sig.Plaintext) + sums, err := parseMessageBlock(sig.Plaintext) if err != nil { return ver, err } sum = "sha256:" + sum - basename := filepath.Base(chartpath) + basename := filepath.Base(packagePath) if sha, ok := sums.Files[basename]; !ok { return ver, fmt.Errorf("provenance does not contain a SHA for a file named %q", basename) } else if sha != sum { @@ -320,64 +318,64 @@ func (s *Signatory) verifySignature(block *clearsign.Block) (*openpgp.Entity, er ) } -func messageBlock(chartpath string) (*bytes.Buffer, error) { - var b *bytes.Buffer +// messageBlock creates a message block from a package path and pre-marshalled metadata +func messageBlock(packagePath string, metadataBytes []byte) (*bytes.Buffer, error) { // Checksum the archive - chash, err := DigestFile(chartpath) + chash, err := DigestFile(packagePath) if err != nil { - return b, err + return nil, err } - base := filepath.Base(chartpath) + base := filepath.Base(packagePath) sums := &SumCollection{ Files: map[string]string{ base: "sha256:" + chash, }, } - // Load the archive into memory. - chart, err := loader.LoadFile(chartpath) - if err != nil { - return b, err - } - - // Buffer a hash + checksums YAML file - data, err := yaml.Marshal(chart.Metadata) - if err != nil { - return b, err - } - + // Buffer the metadata + checksums YAML file // FIXME: YAML uses ---\n as a file start indicator, but this is not legal in a PGP // clearsign block. So we use ...\n, which is the YAML document end marker. // http://yaml.org/spec/1.2/spec.html#id2800168 - b = bytes.NewBuffer(data) + b := bytes.NewBuffer(metadataBytes) b.WriteString("\n...\n") - data, err = yaml.Marshal(sums) + data, err := yaml.Marshal(sums) if err != nil { - return b, err + return nil, err } b.Write(data) return b, nil } -// parseMessageBlock -func parseMessageBlock(data []byte) (*hapi.Metadata, *SumCollection, error) { - // This sucks. +// parseMessageBlock parses a message block and returns only checksums (metadata ignored like upstream) +func parseMessageBlock(data []byte) (*SumCollection, error) { + sc := &SumCollection{} + + // We ignore metadata, just like upstream - only need checksums for verification + if err := ParseMessageBlock(data, nil, sc); err != nil { + return sc, err + } + return sc, nil +} + +// ParseMessageBlock parses a message block containing metadata and checksums. +// +// This is the generic version that can work with any metadata type. +// The metadata parameter should be a pointer to a struct that can be unmarshaled from YAML. +func ParseMessageBlock(data []byte, metadata interface{}, sums *SumCollection) error { parts := bytes.Split(data, []byte("\n...\n")) if len(parts) < 2 { - return nil, nil, errors.New("message block must have at least two parts") + return errors.New("message block must have at least two parts") } - md := &hapi.Metadata{} - sc := &SumCollection{} - - if err := yaml.Unmarshal(parts[0], md); err != nil { - return md, sc, err + if metadata != nil { + if err := yaml.Unmarshal(parts[0], metadata); err != nil { + return err + } } - err := yaml.Unmarshal(parts[1], sc) - return md, sc, err + return yaml.Unmarshal(parts[1], sums) } // loadKey loads a GPG key found at a particular path. @@ -406,7 +404,7 @@ func loadKeyRing(ringpath string) (openpgp.EntityList, error) { // It takes the path to the archive file, and returns a string representation of // the SHA256 sum. // -// The intended use of this function is to generate a sum of a chart TGZ file. +// This function can be used to generate a sum of any package archive file. func DigestFile(filename string) (string, error) { f, err := os.Open(filename) if err != nil { diff --git a/pkg/provenance/sign_test.go b/pkg/provenance/sign_test.go index 9a60fd19c..4594fac01 100644 --- a/pkg/provenance/sign_test.go +++ b/pkg/provenance/sign_test.go @@ -25,6 +25,9 @@ import ( "testing" pgperrors "golang.org/x/crypto/openpgp/errors" //nolint + "sigs.k8s.io/yaml" + + "helm.sh/helm/v4/pkg/chart/v2/loader" ) const ( @@ -75,8 +78,27 @@ files: hashtest-1.2.3.tgz: sha256:c6841b3a895f1444a6738b5d04564a57e860ce42f8519c3be807fb6d9bee7888 ` +// loadChartMetadataForSigning is a test helper that loads chart metadata and marshals it to YAML bytes +func loadChartMetadataForSigning(t *testing.T, chartPath string) []byte { + t.Helper() + + chart, err := loader.LoadFile(chartPath) + if err != nil { + t.Fatal(err) + } + + metadataBytes, err := yaml.Marshal(chart.Metadata) + if err != nil { + t.Fatal(err) + } + + return metadataBytes +} + func TestMessageBlock(t *testing.T) { - out, err := messageBlock(testChartfile) + metadataBytes := loadChartMetadataForSigning(t, testChartfile) + + out, err := messageBlock(testChartfile, metadataBytes) if err != nil { t.Fatal(err) } @@ -88,14 +110,12 @@ func TestMessageBlock(t *testing.T) { } func TestParseMessageBlock(t *testing.T) { - md, sc, err := parseMessageBlock([]byte(testMessageBlock)) + sc, err := parseMessageBlock([]byte(testMessageBlock)) if err != nil { t.Fatal(err) } - if md.Name != "hashtest" { - t.Errorf("Expected name %q, got %q", "hashtest", md.Name) - } + // parseMessageBlock only returns checksums, not metadata (like upstream) if lsc := len(sc.Files); lsc != 1 { t.Errorf("Expected 1 file, got %d", lsc) @@ -221,7 +241,9 @@ func TestClearSign(t *testing.T) { t.Fatal(err) } - sig, err := signer.ClearSign(testChartfile) + metadataBytes := loadChartMetadataForSigning(t, testChartfile) + + sig, err := signer.ClearSign(testChartfile, metadataBytes) if err != nil { t.Fatal(err) } @@ -252,7 +274,9 @@ func TestClearSignError(t *testing.T) { // ensure that signing always fails signer.Entity.PrivateKey.PrivateKey = failSigner{} - sig, err := signer.ClearSign(testChartfile) + metadataBytes := loadChartMetadataForSigning(t, testChartfile) + + sig, err := signer.ClearSign(testChartfile, metadataBytes) if err == nil { t.Fatal("didn't get an error from ClearSign but expected one") } @@ -271,7 +295,9 @@ func TestDecodeSignature(t *testing.T) { t.Fatal(err) } - sig, err := signer.ClearSign(testChartfile) + metadataBytes := loadChartMetadataForSigning(t, testChartfile) + + sig, err := signer.ClearSign(testChartfile, metadataBytes) if err != nil { t.Fatal(err) } diff --git a/pkg/registry/plugin.go b/pkg/registry/plugin.go index 5d22a99ee..991bace76 100644 --- a/pkg/registry/plugin.go +++ b/pkg/registry/plugin.go @@ -38,11 +38,13 @@ type PluginPullOptions struct { // PluginPullResult contains the result of a plugin pull operation type PluginPullResult struct { - Manifest ocispec.Descriptor - PluginData []byte - ProvenanceData []byte // Optional provenance data - Ref string - PluginName string + Manifest ocispec.Descriptor + PluginData []byte + Prov struct { + Data []byte + } + Ref string + PluginName string } // PullPlugin downloads a plugin from an OCI registry using artifact type @@ -96,30 +98,31 @@ func (c *Client) processPluginPull(genericResult *GenericPullResult, pluginName return nil, fmt.Errorf("expected config media type %s for legacy compatibility, got %s", PluginArtifactType, manifest.Config.MediaType) } - // Find the required plugin tarball and optional provenance - expectedTarball := pluginName + ".tgz" - expectedProvenance := pluginName + ".tgz.prov" - + // Find the plugin tarball and optional provenance using NAME-VERSION.tgz format var pluginDescriptor *ocispec.Descriptor var provenanceDescriptor *ocispec.Descriptor + var foundProvenanceName string // Look for layers with the expected titles/annotations for _, layer := range manifest.Layers { d := layer - // Check for title annotation (preferred method) + // Check for title annotation if title, exists := d.Annotations[ocispec.AnnotationTitle]; exists { - switch title { - case expectedTarball: + // Check if this looks like a plugin tarball: {pluginName}-{version}.tgz + if pluginDescriptor == nil && strings.HasPrefix(title, pluginName+"-") && strings.HasSuffix(title, ".tgz") { pluginDescriptor = &d - case expectedProvenance: + } + // Check if this looks like a plugin provenance: {pluginName}-{version}.tgz.prov + if provenanceDescriptor == nil && strings.HasPrefix(title, pluginName+"-") && strings.HasSuffix(title, ".tgz.prov") { provenanceDescriptor = &d + foundProvenanceName = title } } } // Plugin tarball is required if pluginDescriptor == nil { - return nil, fmt.Errorf("required layer %s not found in manifest", expectedTarball) + return nil, fmt.Errorf("required layer matching pattern %s-VERSION.tgz not found in manifest", pluginName) } // Build plugin-specific result @@ -138,7 +141,7 @@ func (c *Client) processPluginPull(genericResult *GenericPullResult, pluginName // Fetch provenance data if available if provenanceDescriptor != nil { - result.ProvenanceData, err = genericClient.GetDescriptorData(genericResult.MemoryStore, *provenanceDescriptor) + result.Prov.Data, err = genericClient.GetDescriptorData(genericResult.MemoryStore, *provenanceDescriptor) if err != nil { return nil, fmt.Errorf("unable to retrieve provenance data with digest %s: %w", provenanceDescriptor.Digest, err) } @@ -146,8 +149,8 @@ func (c *Client) processPluginPull(genericResult *GenericPullResult, pluginName fmt.Fprintf(c.out, "Pulled plugin: %s\n", result.Ref) fmt.Fprintf(c.out, "Digest: %s\n", result.Manifest.Digest) - if result.ProvenanceData != nil { - fmt.Fprintf(c.out, "Provenance: %s\n", expectedProvenance) + if result.Prov.Data != nil { + fmt.Fprintf(c.out, "Provenance: %s\n", foundProvenanceName) } if strings.Contains(result.Ref, "_") { @@ -162,6 +165,7 @@ func (c *Client) processPluginPull(genericResult *GenericPullResult, pluginName type ( pluginPullOperation struct { pluginName string + withProv bool } // PluginPullOption allows customizing plugin pull operations @@ -199,3 +203,10 @@ func GetPluginName(source string) (string, error) { return pluginName, nil } + +// PullPluginOptWithProv configures the pull to fetch provenance data +func PullPluginOptWithProv(withProv bool) PluginPullOption { + return func(operation *pluginPullOperation) { + operation.withProv = withProv + } +}