From 7ba97fc2cdf668642353206b6d2bf333c99c9335 Mon Sep 17 00:00:00 2001 From: Tom Meadows Date: Fri, 3 May 2024 18:05:24 +0100 Subject: [PATCH] Fixing incorrect error message on Verify (#350) * fixed incorrect error message * adding functions to mark required flags --------- Signed-off-by: chaosinthecrd Co-authored-by: John Kjell --- cmd/run.go | 1 + cmd/verify.go | 6 +----- docs/commands.md | 2 +- options/run.go | 14 +++++++++++++- options/sign.go | 7 +++++++ options/verify.go | 23 ++++++++++++++++++++++- test/test.sh | 20 ++++++++++++++------ 7 files changed, 59 insertions(+), 14 deletions(-) diff --git a/cmd/run.go b/cmd/run.go index 0a51073f..c953c90a 100644 --- a/cmd/run.go +++ b/cmd/run.go @@ -149,6 +149,7 @@ func runRun(ctx context.Context, ro options.RunOptions, args []string, signers . return fmt.Errorf("failed to marshal envelope: %w", err) } + log.Infof("Writing signed envelope to %s\n", ro.OutFilePath) if _, err := out.Write(signedBytes); err != nil { return fmt.Errorf("failed to write envelope to out file: %w", err) } diff --git a/cmd/verify.go b/cmd/verify.go index 00a73bcf..9873cfe0 100644 --- a/cmd/verify.go +++ b/cmd/verify.go @@ -64,10 +64,6 @@ const ( // todo: this logic should be broken out and moved to pkg/ // we need to abstract where keys are coming from, etc func runVerify(ctx context.Context, vo options.VerifyOptions, verifiers ...cryptoutil.Verifier) error { - if vo.KeyPath == "" && len(vo.CAPaths) == 0 && len(verifiers) == 0 { - return fmt.Errorf("must supply either a public key, CA certificates or a verifier") - } - if vo.KeyPath != "" { keyFile, err := os.Open(vo.KeyPath) if err != nil { @@ -85,7 +81,7 @@ func runVerify(ctx context.Context, vo options.VerifyOptions, verifiers ...crypt inFile, err := os.Open(vo.PolicyFilePath) if err != nil { - return fmt.Errorf("failed to open file to sign: %w", err) + return fmt.Errorf("failed to open policy file: %w", err) } defer inFile.Close() diff --git a/docs/commands.md b/docs/commands.md index cd1552e2..9964e5b6 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -50,7 +50,7 @@ witness run [cmd] [flags] --enable-archivista Use Archivista to store or retrieve attestations --hashes strings Hashes selected for digest calculation. Defaults to SHA256 (default [sha256]) -h, --help help for run - -o, --outfile string File to which to write signed data. Defaults to stdout + -o, --outfile string File to write signed data to --signer-file-cert-path string Path to the file containing the certificate for the private key --signer-file-intermediate-paths strings Paths to files containing intermediates required to establish trust of the signer's certificate to a root -k, --signer-file-key-path string Path to the file containing the private key diff --git a/options/run.go b/options/run.go index 00f38ace..472d1336 100644 --- a/options/run.go +++ b/options/run.go @@ -36,17 +36,29 @@ type RunOptions struct { AttestorOptSetters map[string][]func(attestation.Attestor) (attestation.Attestor, error) } +var RequiredRunFlags = []string{ + "step", +} + +var OneRequiredPKSignFlags = []string{ + "signer-file-key-path", + "policy-ca", + "signer-kms-ref", +} + func (ro *RunOptions) AddFlags(cmd *cobra.Command) { ro.SignerOptions.AddFlags(cmd) ro.ArchivistaOptions.AddFlags(cmd) cmd.Flags().StringVarP(&ro.WorkingDir, "workingdir", "d", "", "Directory from which commands will run") cmd.Flags().StringSliceVarP(&ro.Attestations, "attestations", "a", DefaultAttestors, "Attestations to record ('product' and 'material' are always recorded)") cmd.Flags().StringSliceVar(&ro.Hashes, "hashes", []string{"sha256"}, "Hashes selected for digest calculation. Defaults to SHA256") - cmd.Flags().StringVarP(&ro.OutFilePath, "outfile", "o", "", "File to which to write signed data. Defaults to stdout") + cmd.Flags().StringVarP(&ro.OutFilePath, "outfile", "o", "", "File to write signed data to") cmd.Flags().StringVarP(&ro.StepName, "step", "s", "", "Name of the step being run") cmd.Flags().BoolVar(&ro.Tracing, "trace", false, "Enable tracing for the command") cmd.Flags().StringSliceVar(&ro.TimestampServers, "timestamp-servers", []string{}, "Timestamp Authority Servers to use when signing envelope") + cmd.MarkFlagsRequiredTogether(RequiredRunFlags...) + attestationRegistrations := attestation.RegistrationEntries() ro.AttestorOptSetters = addFlagsFromRegistry("attestor", attestationRegistrations, cmd) diff --git a/options/sign.go b/options/sign.go index 603d7ecb..112993c3 100644 --- a/options/sign.go +++ b/options/sign.go @@ -25,6 +25,11 @@ type SignOptions struct { TimestampServers []string } +var RequiredSignFlags = []string{ + "infile", + "outfile", +} + func (so *SignOptions) AddFlags(cmd *cobra.Command) { so.SignerOptions.AddFlags(cmd) so.KMSSignerProviderOptions.AddFlags(cmd) @@ -32,4 +37,6 @@ func (so *SignOptions) AddFlags(cmd *cobra.Command) { cmd.Flags().StringVarP(&so.OutFilePath, "outfile", "o", "", "File to write signed data. Defaults to stdout") cmd.Flags().StringVarP(&so.InFilePath, "infile", "f", "", "Witness policy file to sign") cmd.Flags().StringSliceVar(&so.TimestampServers, "timestamp-servers", []string{}, "Timestamp Authority Servers to use when signing envelope") + + cmd.MarkFlagsRequiredTogether(RequiredSignFlags...) } diff --git a/options/verify.go b/options/verify.go index 9bf32d5b..8b9813d0 100644 --- a/options/verify.go +++ b/options/verify.go @@ -14,7 +14,9 @@ package options -import "github.com/spf13/cobra" +import ( + "github.com/spf13/cobra" +) type VerifyOptions struct { VerifierOptions VerifierOptions @@ -28,6 +30,21 @@ type VerifyOptions struct { CAPaths []string } +var RequiredVerifyFlags = []string{ + "policy", +} + +var OneRequiredPKVerifyFlags = []string{ + "publickey", + "policy-ca", + "verifier-kms-ref", +} + +var OneRequiredSubjectFlags = []string{ + "artifactfile", + "subjects", +} + func (vo *VerifyOptions) AddFlags(cmd *cobra.Command) { vo.VerifierOptions.AddFlags(cmd) vo.ArchivistaOptions.AddFlags(cmd) @@ -38,4 +55,8 @@ func (vo *VerifyOptions) AddFlags(cmd *cobra.Command) { cmd.Flags().StringVarP(&vo.ArtifactFilePath, "artifactfile", "f", "", "Path to the artifact to verify") cmd.Flags().StringSliceVarP(&vo.AdditionalSubjects, "subjects", "s", []string{}, "Additional subjects to lookup attestations") cmd.Flags().StringSliceVarP(&vo.CAPaths, "policy-ca", "", []string{}, "Paths to CA certificates to use for verifying the policy") + + cmd.MarkFlagsRequiredTogether(RequiredVerifyFlags...) + cmd.MarkFlagsOneRequired(OneRequiredPKVerifyFlags...) + cmd.MarkFlagsOneRequired(OneRequiredSubjectFlags...) } diff --git a/test/test.sh b/test/test.sh index 7f1730a3..7c801b9e 100755 --- a/test/test.sh +++ b/test/test.sh @@ -16,27 +16,35 @@ set -e -DIR="$( cd -- "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P )" +DIR="$( + cd -- "$(dirname "$0")" >/dev/null 2>&1 + pwd -P +)" . "$DIR/common.sh" -if ! checkprograms make tar ; then - exit 1 +if ! checkprograms make tar; then + exit 1 fi make -C ../ build rm -f ./policy-signed.json ./build.attestation.json ./package.attestation.json ./fail.attestation.json ./testapp ./testapp.tar.tgz +echo "testing signing policy" ../bin/witness -c test.yaml -l debug sign -f policy.json # successful test +echo "testing witness run on build step" ../bin/witness -c test.yaml run -o build.attestation.json -- go build -o=testapp . +echo "testing witness run on packaging step" ../bin/witness -c test.yaml run -s package -k ./testkey2.pem -o package.attestation.json -- tar czf ./testapp.tar.tgz ./testapp +echo "testing witness verify" ../bin/witness -c test.yaml verify # make sure we fail if we run with a key not in the policy -../bin/witness -c test.yaml run -k failkey.pem -o ./fail.attestation.json -- go build -o=testapp . +echo "testing that witness verify fails with a key not in the policy" +../bin/witness -c test.yaml run -k failkey.pem -o ./fail.attestation.json -- go build -o=testapp . ../bin/witness -c test.yaml run -s package -k ./testkey2.pem -o package.attestation.json -- tar czf ./testapp.tar.tgz ./testapp set +e if ../bin/witness -c test.yaml verify -a ./fail.attestation.json -a ./package.attestation.json; then - echo "expected verify to fail" - exit 1 + echo "expected verify to fail" + exit 1 fi