Change format of subjects to base64 (#174)

ianlewis · web-flow · commit 096c709e54ea · 2022-06-07T08:07:07.000+09:00
* Use base64 for subjects.

* Update docs

* Update tests for parseSubject

* Update docs for subjects input

* Change input to base64-subjects

* Add comment on sha256sum
diff --git a/.github/workflows/pre-submit.e2e.generic.workflow.yml b/.github/workflows/pre-submit.e2e.generic.workflow.yml
@@ -14,7 +14,8 @@ jobs:
       actions: read # For reading workflow info.
     uses: ./.github/workflows/slsa2_provenance.yml
     with:
-      subjects: "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2    binary-name"
+      # echo "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2    binary-name" | base64 -w0
+      subjects: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiAgICBiaW5hcnktbmFtZQo="
 
   verify:
     runs-on: ubuntu-latest
diff --git a/.github/workflows/slsa2_provenance.yml b/.github/workflows/slsa2_provenance.yml
@@ -25,8 +25,8 @@ permissions:
 on:
   workflow_call:
     inputs:
-      subjects:
-        description: "Artifacts for which to generate provenance, formatted the same as the output of sha256sum (SHA256 NAME\\n[...])"
+      base64-subjects:
+        description: "Artifacts for which to generate provenance, formatted the same as the output of sha256sum (SHA256 NAME\\n[...]) and base64 encoded."
         required: true
         type: string
     outputs:
@@ -104,7 +104,7 @@ jobs:
         # order to avoid script injection.
         # See: https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#understanding-the-risk-of-script-injections
         env:
-          SUBJECTS: "${{ inputs.subjects }}"
+          SUBJECTS: "${{ inputs.base64-subjects }}"
           GITHUB_CONTEXT: "${{ toJSON(github) }}"
         run: |
           set -euo pipefail
diff --git a/internal/builders/generic/README.md b/internal/builders/generic/README.md
@@ -30,10 +30,20 @@ To get started, you will need to add some steps to your current workflow. We
 will assume you have an existing Github Actions workflow to build your project.
 
 Add a step to your workflow after you have built your project to generate a
-sha256 hash of your artifacts. The following assumes you have a binary called
-`binary-linux-amd64`.
+sha256 hash of your artifacts and base64 encode it.
 
-After that, add a new job to call the `slsa-github-generator` reusable workflow.
+Assuming you have a binary called `binary-linux-amd64` you can use the
+`sha256sum` and `base64` commands to create the digest. Here we use the `-w0` to
+output the encoded data on one line and make it easier to use as a Github Actions
+output:
+
+```shell
+$ sha256sum artifact1 artifact2 ... | base64 -w0
+```
+
+After you have encoded your digest, add a new job to call the
+`slsa-github-generator` reusable workflow. Here's an example of what it might
+look like all together.
 
 ```yaml
 jobs:
@@ -42,25 +52,26 @@ jobs:
       digest: ${{ steps.hash.outputs.digest }}
     runs-on: ubuntu-latest
     steps:
-      # Your build steps are here.
+      - name: "build artifacts"
+        run: |
+          # Build build artifacts here.
       - name: "generate hash"
         shell: bash
         id: hash
         run: |
           set -euo pipefail
-          DIGEST=$(sha256sum binary-linux-amd64)
-          DIGEST="${DIGEST//'%'/'%25'}"
-          DIGEST="${DIGEST//$'\n'/'%0A'}"
-          DIGEST="${DIGEST//$'\r'/'%0D'}"
-          echo "::set-output name=digest::$DIGEST"
+          # sha256sum generates sha256 hash for all artifacts.
+          # base64 -w0 encodes to base64 and outputs on a single line.
+          # sha256sum artifact1 artifact2 ... | base64 -w0
+          echo "::set-output name=digest::$(sha256sum artifact1 artifact2 | base64 -w0)"
   provenance:
     needs: [build]
     permissions:
       id-token: write
       contents: read
     uses: slsa-framework/slsa-github-generator/.github/workflows/slsa2_provenance.yml@main
     with:
-      subjects: "${{ needs.build.outputs.digest }}"
+      base64-subjects: "${{ needs.build.outputs.digest }}"
 ```
 
 ### Workflow Inputs
@@ -69,9 +80,11 @@ The builder workflow
 [.github/workflows/slsa2_provenance.yml](.github/workflows/slsa2_provenance.yml) accepts
 the following inputs:
 
-| Name       | Required | Description                                                                                                    |
-| ---------- | -------- | -------------------------------------------------------------------------------------------------------------- |
-| `subjects` | yes      | Artifacts for which to generate provenance, formatted the same as the output of sha256sum (SHA256 NAME\n[...]) |
+| Name              | Required | Description                                           |
+| ----------------- | -------- | ----------------------------------------------------- |
+| `base64-subjects` | yes      | Artifacts for which to generate provenance, formatted |
+|                   |          | the same as the output of sha256sum                   |
+|                   |          | (SHA256 NAME\n[...]) and base64 encoded.              |
 
 ### Workflow Outputs
 
diff --git a/internal/builders/generic/attest.go b/internal/builders/generic/attest.go
@@ -16,10 +16,10 @@ package main
 
 import (
 	"bufio"
+	"bytes"
 	"context"
+	"encoding/base64"
 	"encoding/json"
-	"errors"
-	"fmt"
 	"io"
 	"os"
 	"regexp"
@@ -30,6 +30,7 @@ import (
 	"github.com/spf13/cobra"
 
 	"github.com/slsa-framework/slsa-github-generator/github"
+	"github.com/slsa-framework/slsa-github-generator/internal/errors"
 	"github.com/slsa-framework/slsa-github-generator/internal/utils"
 	"github.com/slsa-framework/slsa-github-generator/signing/sigstore"
 	"github.com/slsa-framework/slsa-github-generator/slsa"
@@ -47,11 +48,41 @@ var (
 	provenanceOnlyBuildType = "https://github.com/slsa-framework/slsa-github-generator@v1"
 )
 
+// errBase64 indicates a base64 error in the subject.
+type errBase64 struct {
+	errors.WrappableError
+}
+
+// errSha indicates a error in the hash format.
+type errSha struct {
+	errors.WrappableError
+}
+
+// errNoName indicates a missing subject name.
+type errNoName struct {
+	errors.WrappableError
+}
+
+// errDuplicateSubject indicates a duplicate subject name.
+type errDuplicateSubject struct {
+	errors.WrappableError
+}
+
+// errScan is an error scanning the SHA digest data.
+type errScan struct {
+	errors.WrappableError
+}
+
 // parseSubjects parses the value given to the subjects option.
-func parseSubjects(subjectsStr string) ([]intoto.Subject, error) {
+func parseSubjects(b64str string) ([]intoto.Subject, error) {
 	var parsed []intoto.Subject
 
-	scanner := bufio.NewScanner(strings.NewReader(subjectsStr))
+	subjects, err := base64.StdEncoding.DecodeString(b64str)
+	if err != nil {
+		return nil, errors.Errorf(&errBase64{}, "error decoding subjects (is it base64 encoded?): %w", err)
+	}
+
+	scanner := bufio.NewScanner(bytes.NewReader(subjects))
 	for scanner.Scan() {
 		// Split by whitespace, and get values.
 		parts := wsSplit.Split(strings.TrimSpace(scanner.Text()), 2)
@@ -64,18 +95,18 @@ func parseSubjects(subjectsStr string) ([]intoto.Subject, error) {
 		}
 		// Do a sanity check on the SHA to make sure it's a proper hex digest.
 		if !shaCheck.MatchString(shaDigest) {
-			return nil, fmt.Errorf("unexpected sha256 hash %q", shaDigest)
+			return nil, errors.Errorf(&errSha{}, "unexpected sha256 hash format for %q", shaDigest)
 		}
 
 		// Check for the subject name.
 		if len(parts) == 1 {
-			return nil, fmt.Errorf("expected subject name for hash %q", shaDigest)
+			return nil, errors.Errorf(&errNoName{}, "expected subject name for hash %q", shaDigest)
 		}
 		name := strings.TrimSpace(parts[1])
 
 		for _, p := range parsed {
 			if p.Name == name {
-				return nil, fmt.Errorf("duplicate subject: %q", name)
+				return nil, errors.Errorf(&errDuplicateSubject{}, "duplicate subject %q", name)
 			}
 		}
 
@@ -87,7 +118,7 @@ func parseSubjects(subjectsStr string) ([]intoto.Subject, error) {
 		})
 	}
 	if err := scanner.Err(); err != nil {
-		return nil, err
+		return nil, errors.Errorf(&errScan{}, "reading digest: %w", err)
 	}
 
 	return parsed, nil
@@ -194,7 +225,7 @@ run in the context of a Github Actions workflow.`,
 
 	c.Flags().StringVarP(&predicatePath, "predicate", "p", "", "Path to write the unsigned provenance predicate.")
 	c.Flags().StringVarP(&attPath, "signature", "g", "attestation.intoto.jsonl", "Path to write the signed attestation.")
-	c.Flags().StringVarP(&subjects, "subjects", "s", "", "Formatted list of subjects in the same format as sha256sum.")
+	c.Flags().StringVarP(&subjects, "subjects", "s", "", "Formatted list of subjects in the same format as sha256sum (base64 encoded).")
 
 	return c
 }
diff --git a/internal/builders/generic/attest_test.go b/internal/builders/generic/attest_test.go
@@ -1,11 +1,13 @@
 package main
 
 import (
-	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
 	intoto "github.com/in-toto/in-toto-golang/in_toto"
 	slsav02 "github.com/in-toto/in-toto-golang/in_toto/slsa_provenance/v0.2"
+	"github.com/slsa-framework/slsa-github-generator/internal/errors"
 )
 
 // TestParseSubjects tests the parseSubjects function.
@@ -14,11 +16,12 @@ func TestParseSubjects(t *testing.T) {
 		name     string
 		str      string
 		expected []intoto.Subject
-		err      bool
+		err      error
 	}{
 		{
 			name: "single",
-			str:  "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge",
+			// echo "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge" | base64 -w0
+			str: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiBob2dlCg==",
 			expected: []intoto.Subject{
 				{
 					Name: "hoge",
@@ -30,7 +33,8 @@ func TestParseSubjects(t *testing.T) {
 		},
 		{
 			name: "name has spaces",
-			str:  "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge fuga",
+			// echo "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge fuga" | base64 -w0
+			str: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiBob2dlIGZ1Z2EK",
 			expected: []intoto.Subject{
 				{
 					Name: "hoge fuga",
@@ -42,7 +46,8 @@ func TestParseSubjects(t *testing.T) {
 		},
 		{
 			name: "extra whitespace",
-			str:  "\t  2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 \t hoge fuga  \t  ",
+			// echo -e "\t  2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 \t hoge fuga  \t  " | base64 -w0
+			str: "CSAgMmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiAJIGhvZ2UgZnVnYSAgCSAgCg==",
 			expected: []intoto.Subject{
 				{
 					Name: "hoge fuga",
@@ -55,8 +60,8 @@ func TestParseSubjects(t *testing.T) {
 
 		{
 			name: "multiple",
-			str: `2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge
-e712aff3705ac314b9a890e0ec208faa20054eee514d86ab913d768f94e01279 fuga`,
+			// echo -e "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge\ne712aff3705ac314b9a890e0ec208faa20054eee514d86ab913d768f94e01279 fuga" | base64 -w0
+			str: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiBob2dlCmU3MTJhZmYzNzA1YWMzMTRiOWE4OTBlMGVjMjA4ZmFhMjAwNTRlZWU1MTRkODZhYjkxM2Q3NjhmOTRlMDEyNzkgZnVnYQo=",
 			expected: []intoto.Subject{
 				{
 					Name: "hoge",
@@ -79,9 +84,8 @@ e712aff3705ac314b9a890e0ec208faa20054eee514d86ab913d768f94e01279 fuga`,
 		},
 		{
 			name: "blank lines",
-			str: `2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge
-
-e712aff3705ac314b9a890e0ec208faa20054eee514d86ab913d768f94e01279 fuga`,
+			// echo -e "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge\n\ne712aff3705ac314b9a890e0ec208faa20054eee514d86ab913d768f94e01279 fuga" | base64 -w0
+			str: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiBob2dlCgplNzEyYWZmMzcwNWFjMzE0YjlhODkwZTBlYzIwOGZhYTIwMDU0ZWVlNTE0ZDg2YWI5MTNkNzY4Zjk0ZTAxMjc5IGZ1Z2EK",
 			expected: []intoto.Subject{
 				{
 					Name: "hoge",
@@ -99,36 +103,41 @@ e712aff3705ac314b9a890e0ec208faa20054eee514d86ab913d768f94e01279 fuga`,
 		},
 		{
 			name: "sha only",
-			str:  "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2",
-			err:  true,
+			// echo "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2" | base64 -w0
+			str: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMgo=",
+			err: &errNoName{},
 		},
 		{
 			name: "invalid hash",
-			str:  "abcdef hoge",
-			err:  true,
+			// echo "abcdef hoge" | base64 -w0
+			str: "YWJjZGVmIGhvZ2UK",
+			err: &errSha{},
 		},
 		{
 			name: "duplicate name",
-			str: `2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge
-2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge`,
-			err: true,
+			// echo -e "2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge\n2e0390eb024a52963db7b95e84a9c2b12c004054a7bad9a97ec0c7c89d4681d2 hoge" | base64 -w0
+			str: "MmUwMzkwZWIwMjRhNTI5NjNkYjdiOTVlODRhOWMyYjEyYzAwNDA1NGE3YmFkOWE5N2VjMGM3Yzg5ZDQ2ODFkMiBob2dlCjJlMDM5MGViMDI0YTUyOTYzZGI3Yjk1ZTg0YTljMmIxMmMwMDQwNTRhN2JhZDlhOTdlYzBjN2M4OWQ0NjgxZDIgaG9nZQo=",
+			err: &errDuplicateSubject{},
+		},
+		{
+			name: "not base64",
+			str:  "this is not base64",
+			err:  &errBase64{},
 		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			if s, err := parseSubjects(tc.str); err != nil {
-				if tc.err {
-					// Error was expected.
-					return
+				if tc.err != nil && !errors.As(err, &tc.err) {
+					t.Fatalf("unexpected error: %v", cmp.Diff(err, tc.err, cmpopts.EquateErrors()))
 				}
-				t.Fatalf("unexpected error: %v", err)
 			} else {
-				if tc.err {
-					t.Fatalf("expected error but received %#v", s)
+				if tc.err != nil {
+					t.Fatalf("expected %#v but received %#v", tc.err, s)
 				}
 
-				if want, got := tc.expected, s; !reflect.DeepEqual(want, got) {
+				if want, got := tc.expected, s; !cmp.Equal(want, got) {
 					t.Errorf("unexpected subjects, want: %#v, got: %#v", want, got)
 				}
 			}
