update

Signed-off-by: laurentsimon <laurentsimon@google.com>

cli/slsa-verifier/verify/options.go

@@ -76,19 +76,11 @@ func (o *VerifyOptions) AddFlags(cmd *cobra.Command) {
 
 // VerifyNpmOptions is the top-level options for the `verifyNpmPackage` command.
 type VerifyNpmOptions struct {
-	/* Source requirements */
-	SourceURI        string
-	SourceBranch     string
-	SourceTag        string
-	SourceVersionTag string
-	/* Builder Requirements */
-	BuildWorkflowInputs workflowInputs
-	BuilderID           string
+	VerifyOptions
 	/* Other */
 	AttestationsPath string
 	PackageName      string
 	PackageVersion   string
-	PrintProvenance  bool
 }
 
 var _ Interface = (*VerifyNpmOptions)(nil)

cli/slsa-verifier/verify/verify_npm_package.go

@@ -88,13 +88,7 @@ func (c *VerifyNpmPackageCommand) Exec(ctx context.Context, tarballs []string) (
 			fmt.Fprintf(os.Stdout, "%s\n", string(verifiedProvenance))
 		}
 
-		if builderID == nil {
-			builderID = outBuilderID
-		} else if *builderID != *outBuilderID {
-			err := fmt.Errorf("encountered different builderIDs %v %v", builderID, outBuilderID)
-			fmt.Fprintf(os.Stderr, "Verifying npm package %s: FAILED: %v\n\n", tarball, err)
-			return nil, err
-		}
+		builderID = outBuilderID
 		fmt.Fprintf(os.Stderr, "Verifying npm package %s: PASSED\n\n", tarball)
 	}
 

errors/errors.go

@@ -36,4 +36,5 @@ var (
 	ErrorInvalidRekorEntry         = errors.New("invalid Rekor entry")
 	ErrorRekorPubKey               = errors.New("error retrieving Rekor public keys")
 	ErrorInvalidPackageName        = errors.New("invalid package name")
+	ErrorInvalidSubject            = errors.New("invalid subject")
 )

verifiers/internal/gha/builder.go

@@ -36,100 +36,54 @@ var defaultBYOBReusableWorkflows = map[string]bool{
 	trustedBuilderRepository + "/.github/workflows/delegator_generic_slsa3.yml": true,
 }
 
-// VerifyWorkflowIdentity verifies the signing certificate information
-// Builder IDs are verified against an expected builder ID provided in the
-// builerOpts, or against the set of defaultBuilders provided.
-func VerifyWorkflowIdentity(id *WorkflowIdentity,
-	builderOpts *options.BuilderOpts, source string,
-	defaultBuilders map[string]bool,
-) (*utils.TrustedBuilderID, error) {
-	// cert URI path is /org/repo/path/to/workflow@ref
-	workflowPath := strings.SplitN(id.JobWobWorkflowRef, "@", 2)
-	if len(workflowPath) < 2 {
-		return nil, fmt.Errorf("%w: workflow uri: %s", serrors.ErrorMalformedURI, id.JobWobWorkflowRef)
-	}
-
-	// Verify trusted workflow.
-	reusableWorkflowPath := strings.Trim(workflowPath[0], "/")
-	reusableWorkflowTag := strings.Trim(workflowPath[1], "/")
-	builderID, err := verifyTrustedBuilderID(reusableWorkflowPath, reusableWorkflowTag,
-		builderOpts.ExpectedID, defaultBuilders)
-	if err != nil {
-		return nil, err
-	}
-
-	// Verify the ref is a full semantic version tag.
-	if err := verifyTrustedBuilderRef(id, reusableWorkflowTag); err != nil {
-		return nil, err
-	}
-
-	// Issuer verification.
-	if id.Issuer != certOidcIssuer {
-		return nil, fmt.Errorf("%w: %s", serrors.ErrorInvalidOIDCIssuer, id.Issuer)
-	}
-
+// VerifyCertficateSourceRepository verifies the source repository.
+func VerifyCertficateSourceRepository(id *WorkflowIdentity,
+	sourceRepo string,
+) error {
 	// The caller repository in the x509 extension is not fully qualified. It only contains
 	// {org}/{repository}.
-	expectedSource := strings.TrimPrefix(source, "git+https://")
+	expectedSource := strings.TrimPrefix(sourceRepo, "git+https://")
 	expectedSource = strings.TrimPrefix(expectedSource, "github.com/")
 	if id.CallerRepository != expectedSource {
-		return nil, fmt.Errorf("%w: expected source '%s', got '%s'", serrors.ErrorMismatchSource,
+		return fmt.Errorf("%w: expected source '%s', got '%s'", serrors.ErrorMismatchSource,
 			expectedSource, id.CallerRepository)
 	}
-
-	// Return the builder and its tag.
-	// Note: the tag has the format `refs/tags/v1.2.3`.
-	return builderID, nil
+	return nil
 }
 
-// VerifyNpmWorkflowIdentity verifies the signing certificate information
-// Any builder ID is allowed, but it must match the source repo as well
-// in the provenance.
-func VerifyNpmWorkflowIdentity(id *WorkflowIdentity, source string,
+// VerifyBuilderIdentity verifies the signing certificate information
+// Builder IDs are verified against an expected builder ID provided in the
+// builerOpts, or against the set of defaultBuilders provided.
+func VerifyBuilderIdentity(id *WorkflowIdentity,
+	builderOpts *options.BuilderOpts,
 	defaultBuilders map[string]bool,
 ) (*utils.TrustedBuilderID, error) {
-	// cert URI path is /org/repo/path/to/workflow@ref
-	workflowPath := strings.SplitN(id.JobWobWorkflowRef, "@", 2)
-	if len(workflowPath) < 2 {
-		return nil, fmt.Errorf("%w: workflow uri: %s", serrors.ErrorMalformedURI, id.JobWobWorkflowRef)
-	}
-
 	// Issuer verification.
-	if !strings.EqualFold(id.Issuer, certOidcIssuer) {
+	// NOTE: this is necessary before we do any further verification.
+	if id.Issuer != certOidcIssuer {
 		return nil, fmt.Errorf("%w: %s", serrors.ErrorInvalidOIDCIssuer, id.Issuer)
 	}
 
-	// The caller repository in the x509 extension is not fully qualified. It only contains
-	// {org}/{repository}.
-	expectedSource := strings.TrimPrefix(source, "git+https://")
-	expectedSource = strings.TrimPrefix(expectedSource, "github.com/")
-	if !strings.EqualFold(id.CallerRepository, expectedSource) {
-		return nil, fmt.Errorf("%w: expected source '%s', got '%s'", serrors.ErrorMismatchSource,
-			expectedSource, id.CallerRepository)
+	// cert URI path is /org/repo/path/to/workflow@ref
+	workflowPath := strings.SplitN(id.JobWobWorkflowRef, "@", 2)
+	if len(workflowPath) < 2 {
+		return nil, fmt.Errorf("%w: workflow uri: %s", serrors.ErrorMalformedURI, id.JobWobWorkflowRef)
 	}
 
 	// Verify trusted workflow.
 	reusableWorkflowPath := strings.Trim(workflowPath[0], "/")
 	reusableWorkflowTag := strings.Trim(workflowPath[1], "/")
-	// No expected builder ID verification in the certificate:
-	//  1. It's either one of our trusted builders.
-	//	2. Or it's any workflow in a repo.
 	builderID, err := verifyTrustedBuilderID(reusableWorkflowPath, reusableWorkflowTag,
-		nil, defaultBuilders)
+		builderOpts.ExpectedID, defaultBuilders)
 	if err != nil {
-		// WARNING: the default npm builder is *not* one of our trusted builders.
-		// We return success but no trusted builder.
-		return nil, nil
+		return nil, err
 	}
 
-	// This is one of our trusted reusable workflows.
 	// Verify the ref is a full semantic version tag.
 	if err := verifyTrustedBuilderRef(id, reusableWorkflowTag); err != nil {
 		return nil, err
 	}
 
-	// Return the builder and its tag.
-	// Note: the tag has the format `refs/tags/v1.2.3`.
 	return builderID, nil
 }
 

verifiers/internal/gha/npm.go

@@ -447,7 +447,7 @@ func getSubject(att *SignedAttestation) (string, error) {
 		return "", fmt.Errorf("%w", err)
 	}
 	if len(subjects) != 1 {
-		return "", fmt.Errorf("TODO")
+		return "", fmt.Errorf("%w: subject length: %v", serrors.ErrorInvalidSubject, len(subjects))
 	}
 
 	// Package name starts with a prefix.

verifiers/internal/gha/verifier.go

@@ -54,15 +54,19 @@ func verifyEnvAndCert(env *dsse.Envelope,
 		return nil, nil, err
 	}
 
-	// Verify the workflow identity.
-	builderID, err := VerifyWorkflowIdentity(workflowInfo, builderOpts,
-		provenanceOpts.ExpectedSourceURI, defaultBuilders)
+	// Verify the builder identity.
+	builderID, err := VerifyBuilderIdentity(workflowInfo, builderOpts, defaultBuilders)
 	if err != nil {
 		return nil, nil, err
 	}
 
+	// Verify the source repository from the certificate.
+	if err := VerifyCertficateSourceRepository(workflowInfo, provenanceOpts.ExpectedSourceURI); err != nil {
+		return nil, nil, err
+	}
+
 	// Verify properties of the SLSA provenance.
-	// Unpack and verify info in the provenance, including the Subject Digest.
+	// Unpack and verify info in the provenance, including the subject Digest.
 	provenanceOpts.ExpectedBuilderID = builderID.String()
 	if err := VerifyProvenance(env, provenanceOpts); err != nil {
 		return nil, nil, err
@@ -94,27 +98,32 @@ func verifyNpmEnvAndCert(env *dsse.Envelope,
 	}
 
 	// Verify the workflow identity.
-	trustedBuilderID, err := VerifyNpmWorkflowIdentity(workflowInfo,
+	trustedBuilderID, err := VerifyBuilderIdentity(workflowInfo,
 		provenanceOpts.ExpectedSourceURI, defaultBuilders)
-	if err != nil {
+	// We accept a non-trusted builder for the default npm builder
+	// that uses npm CLI.
+	if err != nil && !errors.Is(err, serrors.ErrorUntrustedReusableWorkflow) {
 		return nil, err
 	}
 
-	// TODO(#493): verify certificate information matches
-	// the provenance if possible in the future.
+	// TODO(#493): retrieve certificate information to match
+	// with the provenance.
 	// Today it's not possible due to lack of information in the cert.
+	// Verify the source repository frmo the certificate.
+	if err := VerifyCertficateSourceRepository(workflowInfo, provenanceOpts.ExpectedSourceURI); err != nil {
+		return nil, nil, err
+	}
 
 	// Verify properties of the SLSA provenance.
 	// Unpack and verify info in the provenance, including the Subject Digest.
 	// WARNING: builderID may be empty if it's not a reusable builder workflow.
 	if trustedBuilderID != nil {
 		provenanceOpts.ExpectedBuilderID = trustedBuilderID.String()
 	} else {
-		// TODO(#494): update the builder ID name.
 		if builderOpts == nil || builderOpts.ExpectedID == nil {
 			return nil, fmt.Errorf("builder mistmatch. No builder ID provided by user , got '%v'", builderGitHubRunnerID)
 		}
-
+		// TODO(#494): update the builder ID name.
 		trustedBuilderID, err = utils.TrustedBuilderIDNew(builderGitHubRunnerID)
 		if err != nil {
 			return nil, err
@@ -129,7 +138,7 @@ func verifyNpmEnvAndCert(env *dsse.Envelope,
 	}
 
 	fmt.Fprintf(os.Stderr, "Verified build using builder %s at commit %s\n",
-		provenanceOpts.ExpectedBuilderID,
+		trustedBuilderID.String(),
 		workflowInfo.CallerHash)
 
 	return trustedBuilderID, nil