Skip to content

Commit

Permalink
refactor: break --insecure into --http-only and --tls-skip-verify
Browse files Browse the repository at this point in the history
Fixes #2860

Signed-off-by: Joonas Bergius <joonas@bergi.us>
  • Loading branch information
joonas committed Sep 1, 2024
1 parent 19cdbba commit 0896cb9
Show file tree
Hide file tree
Showing 21 changed files with 117 additions and 84 deletions.
18 changes: 10 additions & 8 deletions src/cmd/common/viper.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,16 @@ const (

// Root config keys

VLogLevel = "log_level"
VArchitecture = "architecture"
VNoLogFile = "no_log_file"
VNoProgress = "no_progress"
VNoColor = "no_color"
VZarfCache = "zarf_cache"
VTmpDir = "tmp_dir"
VInsecure = "insecure"
VLogLevel = "log_level"
VArchitecture = "architecture"
VNoLogFile = "no_log_file"
VNoProgress = "no_progress"
VNoColor = "no_color"
VZarfCache = "zarf_cache"
VTmpDir = "tmp_dir"
VInsecure = "insecure"
VPlainHTTP = "plain_http"
VInsecureSkipTLSVerify = "insecure_skip_tls_verify"

// Init config keys

Expand Down
1 change: 1 addition & 0 deletions src/cmd/initialize.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ func init() {

initCmd.Flags().IntVar(&pkgConfig.PkgOpts.Retries, "retries", v.GetInt(common.VPkgRetries), lang.CmdPackageFlagRetries)
initCmd.Flags().StringVarP(&pkgConfig.PkgOpts.PublicKeyPath, "key", "k", v.GetString(common.VPkgPublicKey), lang.CmdPackageFlagFlagPublicKey)
initCmd.Flags().BoolVar(&pkgConfig.PkgOpts.SkipSignatureValidation, "skip-signature-validation", false, lang.CmdPackageFlagSkipSignatureValidation)

initCmd.Flags().SortFlags = true
}
7 changes: 7 additions & 0 deletions src/cmd/package.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ var packageCmd = &cobra.Command{
Use: "package",
Aliases: []string{"p"},
Short: lang.CmdPackageShort,
PersistentPreRun: func(_ *cobra.Command, _ []string) {
// If --insecure was provided, set --skip-signature-validation to match
if config.CommonOptions.Insecure {
pkgConfig.PkgOpts.SkipSignatureValidation = true
}
},
}

var packageCreateCmd = &cobra.Command{
Expand Down Expand Up @@ -370,6 +376,7 @@ func bindPackageFlags(v *viper.Viper) {
packageFlags := packageCmd.PersistentFlags()
packageFlags.IntVar(&config.CommonOptions.OCIConcurrency, "oci-concurrency", v.GetInt(common.VPkgOCIConcurrency), lang.CmdPackageFlagConcurrency)
packageFlags.StringVarP(&pkgConfig.PkgOpts.PublicKeyPath, "key", "k", v.GetString(common.VPkgPublicKey), lang.CmdPackageFlagFlagPublicKey)
packageFlags.BoolVar(&pkgConfig.PkgOpts.SkipSignatureValidation, "skip-signature-validation", false, lang.CmdPackageFlagSkipSignatureValidation)
}

func bindCreateFlags(v *viper.Viper) {
Expand Down
9 changes: 9 additions & 0 deletions src/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ var (
var rootCmd = &cobra.Command{
Use: "zarf COMMAND",
PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
// If --insecure was provided, set --insecure-skip-tls-verify and --plain-http to match
if config.CommonOptions.Insecure {
config.CommonOptions.InsecureSkipTLSVerify = true
config.CommonOptions.PlainHTTP = true
}

// Skip for vendor only commands
if common.CheckVendorOnlyFromPath(cmd) {
return nil
Expand Down Expand Up @@ -121,4 +127,7 @@ func init() {
rootCmd.PersistentFlags().StringVar(&config.CommonOptions.CachePath, "zarf-cache", v.GetString(common.VZarfCache), lang.RootCmdFlagCachePath)
rootCmd.PersistentFlags().StringVar(&config.CommonOptions.TempDirectory, "tmpdir", v.GetString(common.VTmpDir), lang.RootCmdFlagTempDir)
rootCmd.PersistentFlags().BoolVar(&config.CommonOptions.Insecure, "insecure", v.GetBool(common.VInsecure), lang.RootCmdFlagInsecure)
rootCmd.PersistentFlags().MarkDeprecated("insecure", "please use --plain-http, --insecure-skip-tls-verify, or --skip-signature-validation instead.")
rootCmd.PersistentFlags().BoolVar(&config.CommonOptions.PlainHTTP, "plain-http", v.GetBool(common.VPlainHTTP), lang.RootCmdFlagPlainHTTP)
rootCmd.PersistentFlags().BoolVar(&config.CommonOptions.InsecureSkipTLSVerify, "insecure-skip-tls-verify", v.GetBool(common.VInsecureSkipTLSVerify), lang.RootCmdFlagInsecureSkipTLSVerify)
}
29 changes: 16 additions & 13 deletions src/config/lang/english.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,16 @@ const (
RootCmdLong = "Zarf eliminates the complexity of air gap software delivery for Kubernetes clusters and cloud native workloads\n" +
"using a declarative packaging strategy to support DevSecOps in offline and semi-connected environments."

RootCmdFlagLogLevel = "Log level when running Zarf. Valid options are: warn, info, debug, trace"
RootCmdFlagArch = "Architecture for OCI images and Zarf packages"
RootCmdFlagSkipLogFile = "Disable log file creation"
RootCmdFlagNoProgress = "Disable fancy UI progress bars, spinners, logos, etc"
RootCmdFlagNoColor = "Disable colors in output"
RootCmdFlagCachePath = "Specify the location of the Zarf cache directory"
RootCmdFlagTempDir = "Specify the temporary directory to use for intermediate files"
RootCmdFlagInsecure = "Allow access to insecure registries and disable other recommended security enforcements such as package checksum and signature validation. This flag should only be used if you have a specific reason and accept the reduced security posture."
RootCmdFlagLogLevel = "Log level when running Zarf. Valid options are: warn, info, debug, trace"
RootCmdFlagArch = "Architecture for OCI images and Zarf packages"
RootCmdFlagSkipLogFile = "Disable log file creation"
RootCmdFlagNoProgress = "Disable fancy UI progress bars, spinners, logos, etc"
RootCmdFlagNoColor = "Disable colors in output"
RootCmdFlagCachePath = "Specify the location of the Zarf cache directory"
RootCmdFlagTempDir = "Specify the temporary directory to use for intermediate files"
RootCmdFlagInsecure = "Allow access to insecure registries and disable other recommended security enforcements such as package checksum and signature validation. This flag should only be used if you have a specific reason and accept the reduced security posture."
RootCmdFlagPlainHTTP = "Force the connections over HTTP instead of HTTPS. This flag should only be used if you have a specific reason and accept the reduced security posture."
RootCmdFlagInsecureSkipTLSVerify = "Skip checking server's certificate for validity. This flag should only be used if you have a specific reason and accept the reduced security posture."

RootCmdDeprecatedDeploy = "Deprecated: Please use \"zarf package deploy %s\" to deploy this package. This warning will be removed in Zarf v1.0.0."
RootCmdDeprecatedCreate = "Deprecated: Please use \"zarf package create\" to create this package. This warning will be removed in Zarf v1.0.0."
Expand Down Expand Up @@ -210,10 +212,11 @@ $ zarf init --artifact-push-password={PASSWORD} --artifact-push-username={USERNA
CmdInternalCrc32Short = "Generates a decimal CRC32 for the given text"

// zarf package
CmdPackageShort = "Zarf package commands for creating, deploying, and inspecting packages"
CmdPackageFlagConcurrency = "Number of concurrent layer operations to perform when interacting with a remote package."
CmdPackageFlagFlagPublicKey = "Path to public key file for validating signed packages"
CmdPackageFlagRetries = "Number of retries to perform for Zarf deploy operations like git/image pushes or Helm installs"
CmdPackageShort = "Zarf package commands for creating, deploying, and inspecting packages"
CmdPackageFlagConcurrency = "Number of concurrent layer operations to perform when interacting with a remote package."
CmdPackageFlagFlagPublicKey = "Path to public key file for validating signed packages"
CmdPackageFlagSkipSignatureValidation = "Skip validating the signature of the Zarf package"
CmdPackageFlagRetries = "Number of retries to perform for Zarf deploy operations like git/image pushes or Helm installs"

CmdPackageCreateShort = "Creates a Zarf package from a given directory or the current directory"
CmdPackageCreateLong = "Builds an archive of resources and dependencies defined by the 'zarf.yaml' in the specified directory.\n" +
Expand Down Expand Up @@ -273,7 +276,7 @@ $ zarf package mirror-resources <your-package.tar.zst> \
CmdPackageDeployFlagAdoptExistingResources = "Adopts any pre-existing K8s resources into the Helm charts managed by Zarf. ONLY use when you have existing deployments you want Zarf to takeover."
CmdPackageDeployFlagSet = "Specify deployment variables to set on the command line (KEY=value)"
CmdPackageDeployFlagComponents = "Comma-separated list of components to deploy. Adding this flag will skip the prompts for selected components. Globbing component names with '*' and deselecting 'default' components with a leading '-' are also supported."
CmdPackageDeployFlagShasum = "Shasum of the package to deploy. Required if deploying a remote package and \"--insecure\" is not provided"
CmdPackageDeployFlagShasum = "Shasum of the package to deploy. Required if deploying a remote package."
CmdPackageDeployFlagSget = "[Deprecated] Path to public sget key file for remote packages signed via cosign. This flag will be removed in v1.0.0 please use the --key flag instead."
CmdPackageDeployFlagSkipWebhooks = "[alpha] Skip waiting for external webhooks to execute as each package component is deployed"
CmdPackageDeployFlagTimeout = "Timeout for Helm operations such as installs and rollbacks"
Expand Down
2 changes: 1 addition & 1 deletion src/internal/packager/helm/chart.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ func (h *Helm) TemplateChart(ctx context.Context) (manifest string, chartValues
client.IncludeCRDs = true
// TODO: Further research this with regular/OCI charts
client.Verify = false
client.InsecureSkipTLSverify = config.CommonOptions.Insecure
client.InsecureSkipTLSverify = config.CommonOptions.InsecureSkipTLSVerify
if h.kubeVersion != "" {
parsedKubeVersion, err := chartutil.ParseKubeVersion(h.kubeVersion)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion src/internal/packager/helm/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ func (h *Helm) DownloadPublishedChart(ctx context.Context, cosignKeyPath string)
Verify: downloader.VerifyNever,
Getters: getter.All(pull.Settings),
Options: []getter.Option{
getter.WithInsecureSkipVerifyTLS(config.CommonOptions.Insecure),
getter.WithInsecureSkipVerifyTLS(config.CommonOptions.InsecureSkipTLSVerify),
getter.WithBasicAuth(username, password),
},
}
Expand Down
6 changes: 3 additions & 3 deletions src/internal/packager/images/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ type PushConfig struct {
func NoopOpt(*crane.Options) {}

// WithGlobalInsecureFlag returns an option for crane that configures insecure
// based upon Zarf's global --insecure flag.
// based upon Zarf's global --insecure-skip-tls-verify (and --insecure) flags.
func WithGlobalInsecureFlag() []crane.Option {
if config.CommonOptions.Insecure {
if config.CommonOptions.InsecureSkipTLSVerify {
return []crane.Option{crane.Insecure}
}
// passing a nil option will cause panic
Expand Down Expand Up @@ -103,7 +103,7 @@ func createPushOpts(cfg PushConfig, pb *message.ProgressBar) []crane.Option {
opts = append(opts, WithPushAuth(cfg.RegInfo))

transport := http.DefaultTransport.(*http.Transport).Clone()
transport.TLSClientConfig.InsecureSkipVerify = config.CommonOptions.Insecure
transport.TLSClientConfig.InsecureSkipVerify = config.CommonOptions.InsecureSkipTLSVerify
// TODO (@WSTARR) This is set to match the TLSHandshakeTimeout to potentially mitigate effects of https://github.com/zarf-dev/zarf/issues/1444
transport.ResponseHeaderTimeout = 10 * time.Second

Expand Down
15 changes: 9 additions & 6 deletions src/pkg/packager/creator/normal.go
Original file line number Diff line number Diff line change
Expand Up @@ -281,14 +281,17 @@ func (pc *PackageCreator) Output(ctx context.Context, dst *layout.PackagePaths,
return fmt.Errorf("unable to publish package: %w", err)
}
message.HorizontalRule()
flags := ""
if config.CommonOptions.Insecure {
flags = "--insecure"
flags := []string{}
if config.CommonOptions.PlainHTTP {
flags = append(flags, "--plain-http")
}
if config.CommonOptions.InsecureSkipTLSVerify {
flags = append(flags, "--insecure-skip-tls-verify")
}
message.Title("To inspect/deploy/pull:", "")
message.ZarfCommand("package inspect %s %s", helpers.OCIURLPrefix+remote.Repo().Reference.String(), flags)
message.ZarfCommand("package deploy %s %s", helpers.OCIURLPrefix+remote.Repo().Reference.String(), flags)
message.ZarfCommand("package pull %s %s", helpers.OCIURLPrefix+remote.Repo().Reference.String(), flags)
message.ZarfCommand("package inspect %s %s", helpers.OCIURLPrefix+remote.Repo().Reference.String(), strings.Join(flags, " "))
message.ZarfCommand("package deploy %s %s", helpers.OCIURLPrefix+remote.Repo().Reference.String(), strings.Join(flags, " "))
message.ZarfCommand("package pull %s %s", helpers.OCIURLPrefix+remote.Repo().Reference.String(), strings.Join(flags, " "))
} else {
// Use the output path if the user specified it.
packageName := fmt.Sprintf("%s%s", sources.NameFromMetadata(pkg, pc.createOpts.IsSkeleton), sources.PkgSuffix(pkg.Metadata.Uncompressed))
Expand Down
2 changes: 1 addition & 1 deletion src/pkg/packager/sources/new_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ func TestPackageSource(t *testing.T) {
{
name: "http-insecure",
src: fmt.Sprintf("%s/zarf-package-wordpress-amd64-16.0.4.tar.zst", ts.URL),
expectedErr: "remote package provided without a shasum, use --insecure to ignore, or provide one w/ --shasum",
expectedErr: "remote package provided without a shasum, please provide one with --shasum",
},
}
for _, tt := range tests {
Expand Down
18 changes: 11 additions & 7 deletions src/pkg/packager/sources/oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,10 @@ func (s *OCISource) LoadPackage(ctx context.Context, dst *layout.PackagePaths, f

spinner.Success()

if err := ValidatePackageSignature(ctx, dst, s.PublicKeyPath); err != nil {
return pkg, nil, err
if !s.SkipSignatureValidation {
if err := ValidatePackageSignature(ctx, dst, s.PublicKeyPath); err != nil {
return pkg, nil, err
}
}
}

Expand Down Expand Up @@ -141,11 +143,13 @@ func (s *OCISource) LoadPackageMetadata(ctx context.Context, dst *layout.Package
spinner.Success()
}

if err := ValidatePackageSignature(ctx, dst, s.PublicKeyPath); err != nil {
if errors.Is(err, ErrPkgSigButNoKey) && skipValidation {
message.Warn("The package was signed but no public key was provided, skipping signature validation")
} else {
return pkg, nil, err
if !s.SkipSignatureValidation {
if err := ValidatePackageSignature(ctx, dst, s.PublicKeyPath); err != nil {
if errors.Is(err, ErrPkgSigButNoKey) && skipValidation {
message.Warn("The package was signed but no public key was provided, skipping signature validation")
} else {
return pkg, nil, err
}
}
}
}
Expand Down
18 changes: 11 additions & 7 deletions src/pkg/packager/sources/tarball.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,10 @@ func (s *TarballSource) LoadPackage(ctx context.Context, dst *layout.PackagePath

spinner.Success()

if err := ValidatePackageSignature(ctx, dst, s.PublicKeyPath); err != nil {
return pkg, nil, err
if !s.SkipSignatureValidation {
if err := ValidatePackageSignature(ctx, dst, s.PublicKeyPath); err != nil {
return pkg, nil, err
}
}
}

Expand Down Expand Up @@ -185,11 +187,13 @@ func (s *TarballSource) LoadPackageMetadata(ctx context.Context, dst *layout.Pac
spinner.Success()
}

if err := ValidatePackageSignature(ctx, dst, s.PublicKeyPath); err != nil {
if errors.Is(err, ErrPkgSigButNoKey) && skipValidation {
message.Warn("The package was signed but no public key was provided, skipping signature validation")
} else {
return pkg, nil, err
if !s.SkipSignatureValidation {
if err := ValidatePackageSignature(ctx, dst, s.PublicKeyPath); err != nil {
if errors.Is(err, ErrPkgSigButNoKey) && skipValidation {
message.Warn("The package was signed but no public key was provided, skipping signature validation")
} else {
return pkg, nil, err
}
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/pkg/packager/sources/url.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ type URLSource struct {

// Collect downloads a package from the source URL.
func (s *URLSource) Collect(ctx context.Context, dir string) (string, error) {
if !config.CommonOptions.Insecure && s.Shasum == "" && !strings.HasPrefix(s.PackageSource, helpers.SGETURLPrefix) {
return "", fmt.Errorf("remote package provided without a shasum, use --insecure to ignore, or provide one w/ --shasum")
if s.Shasum == "" && !strings.HasPrefix(s.PackageSource, helpers.SGETURLPrefix) {
return "", fmt.Errorf("remote package provided without a shasum, please provide one with --shasum")
}
var packageURL string
if s.Shasum != "" {
Expand Down
8 changes: 1 addition & 7 deletions src/pkg/packager/sources/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"strings"

"github.com/defenseunicorns/pkg/helpers/v2"
"github.com/zarf-dev/zarf/src/config"
"github.com/zarf-dev/zarf/src/pkg/layout"
"github.com/zarf-dev/zarf/src/pkg/message"
"github.com/zarf-dev/zarf/src/pkg/utils"
Expand All @@ -25,16 +24,11 @@ var (
// ErrPkgKeyButNoSig is returned when a key was provided but the package is not signed
ErrPkgKeyButNoSig = errors.New("a key was provided but the package is not signed - the package may be corrupted or the --key flag was erroneously specified")
// ErrPkgSigButNoKey is returned when a package is signed but no key was provided
ErrPkgSigButNoKey = errors.New("package is signed but no key was provided - add a key with the --key flag or use the --insecure flag and run the command again")
ErrPkgSigButNoKey = errors.New("package is signed but no key was provided - add a key with the --key flag or use the --skip-signature-validation flag and run the command again")
)

// ValidatePackageSignature validates the signature of a package
func ValidatePackageSignature(ctx context.Context, paths *layout.PackagePaths, publicKeyPath string) error {
// If the insecure flag was provided ignore the signature validation
if config.CommonOptions.Insecure {
return nil
}

if publicKeyPath != "" {
message.Debugf("Using public key %q for signature validation", publicKeyPath)
}
Expand Down
4 changes: 2 additions & 2 deletions src/pkg/zoci/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ type Remote struct {
func NewRemote(url string, platform ocispec.Platform, mods ...oci.Modifier) (*Remote, error) {
logger := slog.New(message.ZarfHandler{})
modifiers := append([]oci.Modifier{
oci.WithPlainHTTP(config.CommonOptions.Insecure),
oci.WithInsecureSkipVerify(config.CommonOptions.Insecure),
oci.WithPlainHTTP(config.CommonOptions.PlainHTTP),
oci.WithInsecureSkipVerify(config.CommonOptions.InsecureSkipTLSVerify),
oci.WithLogger(logger),
oci.WithUserAgent("zarf/" + config.CLIVersion),
}, mods...)
Expand Down
4 changes: 2 additions & 2 deletions src/test/e2e/11_oci_pull_inspect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,15 +61,15 @@ func (suite *PullInspectTestSuite) Test_0_Pull() {
suite.Contains(stdErr, "Package signature validated!")

// Test pull w/ bad ref.
stdOut, stdErr, err = e2e.Zarf(suite.T(), "package", "pull", "oci://"+badPullInspectRef.String(), "--insecure")
stdOut, stdErr, err = e2e.Zarf(suite.T(), "package", "pull", "oci://"+badPullInspectRef.String(), "--plain-http", "--insecure-skip-tls-verify")
suite.Error(err, stdOut, stdErr)
}

func (suite *PullInspectTestSuite) Test_1_Remote_Inspect() {
suite.T().Log("E2E: Package Inspect oci://")

// Test inspect w/ bad ref.
_, stdErr, err := e2e.Zarf(suite.T(), "package", "inspect", "oci://"+badPullInspectRef.String(), "--insecure")
_, stdErr, err := e2e.Zarf(suite.T(), "package", "inspect", "oci://"+badPullInspectRef.String(), "--plain-http", "--insecure-skip-tls-verify", "--skip-signature-validation")
suite.Error(err, stdErr)

// Test inspect on a public package.
Expand Down
Loading

0 comments on commit 0896cb9

Please sign in to comment.