Fixing incorrect error message on Verify (#350)

* fixed incorrect error message
* adding functions to mark required flags
---------

Signed-off-by: chaosinthecrd <tom@tmlabs.co.uk>
Co-authored-by: John Kjell <john@testifysec.com>

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)
 	}

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()

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

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)
 

options/sign.go

@@ -25,11 +25,18 @@ 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)
 	cmd.Flags().StringVarP(&so.DataType, "datatype", "t", "https://witness.testifysec.com/policy/v0.1", "The URI reference to the type of data being signed. Defaults to the Witness policy type")
 	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...)
 }

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...)
 }

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