From 74013111d5bef2512cf893e6accef3afb5b60add Mon Sep 17 00:00:00 2001 From: Bohan Chen Date: Mon, 8 Jan 2024 17:18:04 -0500 Subject: [PATCH] stick to consistent tense for err msgs and make the start/stop time parsing best effort - the slsa spec says its optional so we shouldn't fail the whole attestation if we can't find it Signed-off-by: Bohan Chen --- pkg/reconciler/build/build.go | 21 ++++++++++----------- pkg/slsa/attest.go | 31 ++++++++++--------------------- pkg/slsa/image.go | 8 ++++---- pkg/slsa/pkcs8_signer.go | 2 +- pkg/slsa/sign.go | 20 ++++++++++---------- 5 files changed, 35 insertions(+), 47 deletions(-) diff --git a/pkg/reconciler/build/build.go b/pkg/reconciler/build/build.go index 8d62bcbd5..033fa8dd7 100644 --- a/pkg/reconciler/build/build.go +++ b/pkg/reconciler/build/build.go @@ -8,7 +8,6 @@ import ( "github.com/google/go-containerregistry/pkg/authn" ggcrv1 "github.com/google/go-containerregistry/pkg/v1" intoto "github.com/in-toto/in-toto-golang/in_toto" - "github.com/pkg/errors" "go.uber.org/zap" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" @@ -206,7 +205,7 @@ func (c *Reconciler) reconcile(ctx context.Context, build *buildapi.Build) error if c.FeatureFlags.GenerateSlsaAttestation { attestDigest, err = c.attestBuild(ctx, build, buildMetadata, pod) if err != nil { - return fmt.Errorf("attesting build: %v", err) + return fmt.Errorf("failed to attest build: %v", err) } } @@ -399,7 +398,7 @@ func (c *Reconciler) buildMetadataFromBuildPod(pod *corev1.Pod) (*cnb.BuildMetad return cnb.DecompressBuildMetadata(status.State.Terminated.Message) } } - return nil, errors.New(buildapi.CompletionContainerName + " container not found") + return nil, fmt.Errorf("%v container not found", buildapi.CompletionContainerName) } func (c *Reconciler) attestBuild(ctx context.Context, build *buildapi.Build, buildMetadata *cnb.BuildMetadata, pod *corev1.Pod) (string, error) { @@ -414,18 +413,18 @@ func (c *Reconciler) attestBuild(ctx context.Context, build *buildapi.Build, bui controllerSecrets, err := c.SecretFetcher.SecretsForSystemServiceAccount(ctx) if err != nil { - return "", fmt.Errorf("getting controller secrets: %v", err) + return "", fmt.Errorf("failed to get controller secrets: %v", err) } buildSecrets, err := c.SecretFetcher.SecretsForServiceAccount(ctx, build.ServiceAccount(), build.Namespace) if err != nil { - return "", fmt.Errorf("getting service account secrets: %v", err) + return "", fmt.Errorf("failed to get service account secrets: %v", err) } secrets := append(controllerSecrets, buildSecrets...) signingKeys, err := secret.FilterAndExtractSLSASecrets(secrets) if err != nil { - return "", fmt.Errorf("parsing slsa secrets: %v", err) + return "", fmt.Errorf("failed to parse slsa secrets: %v", err) } signers := make([]slsa.Signer, len(signingKeys)) @@ -438,7 +437,7 @@ func (c *Reconciler) attestBuild(ctx context.Context, build *buildapi.Build, bui s, err = slsa.NewPKCS8Signer(key.Key, key.SecretName) } if err != nil { - return "", fmt.Errorf("creating signer: %v", err) + return "", fmt.Errorf("failed to create signer: %v", err) } signers[i] = s } @@ -450,22 +449,22 @@ func (c *Reconciler) attestBuild(ctx context.Context, build *buildapi.Build, bui deps, err := c.attestBuildDeps(ctx, build, pod, secrets) if err != nil { - return "", fmt.Errorf("gathering build deps: %v", err) + return "", fmt.Errorf("failed to gather build deps: %v", err) } statement, err := c.Attester.AttestBuild(build, buildMetadata, pod, keychain, buildId, deps...) if err != nil { - return "", fmt.Errorf("generating statement: %v", err) + return "", fmt.Errorf("failed to generate statement: %v", err) } payload, err := c.Attester.Sign(ctx, statement, signers...) if err != nil { - return "", fmt.Errorf("signing statement: %v", err) + return "", fmt.Errorf("failed to sign statement: %v", err) } _, digest, err := c.Attester.Write(ctx, buildMetadata.LatestImage, payload, keychain) if err != nil { - return "", fmt.Errorf("writting attestation: %v", err) + return "", fmt.Errorf("failed to write attestation: %v", err) } return digest, nil diff --git a/pkg/slsa/attest.go b/pkg/slsa/attest.go index 071fde3f7..86242e9bd 100644 --- a/pkg/slsa/attest.go +++ b/pkg/slsa/attest.go @@ -46,34 +46,31 @@ type Attester struct { func (a *Attester) AttestBuild(build *buildv1alpha2.Build, buildMetadata *cnb.BuildMetadata, pod *corev1.Pod, builderAndAppKeychain authn.Keychain, builderId BuilderID, depFns ...BuilderDependencyFn) (intoto.Statement, error) { builderRepo, builderSha, builderLabels, err := a.ImageReader.Read(builderAndAppKeychain, build.Spec.Builder.Image) if err != nil { - return intoto.Statement{}, fmt.Errorf("reading builder image: %v", err) + return intoto.Statement{}, fmt.Errorf("failed to read builder image: %v", err) } appRepo, appSha, appLabels, err := a.ImageReader.Read(builderAndAppKeychain, buildMetadata.LatestImage) if err != nil { - return intoto.Statement{}, fmt.Errorf("reading app image: %v", err) + return intoto.Statement{}, fmt.Errorf("failed to read app image: %v", err) } source, sourceDigest, err := extractSourceFromLabel(appLabels) if err != nil { - return intoto.Statement{}, fmt.Errorf("extracting source from label: %v", err) + return intoto.Statement{}, fmt.Errorf("failed to extract source from label: %v", err) } - start, stop, err := getStartStopTime(pod) - if err != nil { - return intoto.Statement{}, fmt.Errorf("parsing start/stop time: %v", err) - } + start, stop := getStartStopTime(pod) lifecycle, err := a.LifecycleProvider.Metadata() if err != nil { - return intoto.Statement{}, fmt.Errorf("reading lifecycle metadata: %v", err) + return intoto.Statement{}, fmt.Errorf("failed to read lifecycle metadata: %v", err) } builderDeps := make([]slsav1.ResourceDescriptor, 0) for i, fn := range depFns { dep, err := fn() if err != nil { - return intoto.Statement{}, fmt.Errorf("fetching builder dependency #%v: %v", i, err) + return intoto.Statement{}, fmt.Errorf("failed to fetch builder dependency #%v: %v", i, err) } builderDeps = append(builderDeps, dep) @@ -161,7 +158,7 @@ func getBuildType(version string) string { return fmt.Sprintf("https://github.com/buildpacks-community/kpack/blob/v%v/docs/slsa.md", version) } -func getStartStopTime(pod *corev1.Pod) (*time.Time, *time.Time, error) { +func getStartStopTime(pod *corev1.Pod) (*time.Time, *time.Time) { var ( start *time.Time stop *time.Time @@ -171,25 +168,17 @@ func getStartStopTime(pod *corev1.Pod) (*time.Time, *time.Time, error) { if c.Name == buildv1alpha2.PrepareContainerName { if c.State.Terminated != nil && c.State.Terminated.ExitCode == 0 { start = &c.State.Terminated.StartedAt.Time - } else { - return nil, nil, fmt.Errorf("prepare not finished yet") } } if c.Name == buildv1alpha2.CompletionContainerName { if c.State.Terminated != nil && c.State.Terminated.ExitCode == 0 { stop = &c.State.Terminated.FinishedAt.Time - } else { - return nil, nil, fmt.Errorf("completion not finished yet") } } } - if start == nil || stop == nil { - return nil, nil, fmt.Errorf("failed to extract time") - } - - return start, stop, nil + return start, stop } func convertMap(orig map[string]string) map[string]interface{} { @@ -222,7 +211,7 @@ func WithVersionedObject(kind string, obj K8sObject) BuilderDependencyFn { } bytes, err := json.Marshal(versioned) if err != nil { - return slsav1.ResourceDescriptor{}, fmt.Errorf("marshalling json: %v", err) + return slsav1.ResourceDescriptor{}, fmt.Errorf("failed to marshal json: %v", err) } return slsav1.ResourceDescriptor{ @@ -246,7 +235,7 @@ func WithVersionedObjects(kind string, objs []K8sObject) BuilderDependencyFn { } bytes, err := json.Marshal(versioned) if err != nil { - return slsav1.ResourceDescriptor{}, fmt.Errorf("marshalling json: %v", err) + return slsav1.ResourceDescriptor{}, fmt.Errorf("failed to marshal json: %v", err) } return slsav1.ResourceDescriptor{ diff --git a/pkg/slsa/image.go b/pkg/slsa/image.go index 1c3ce2f8b..2eff537af 100644 --- a/pkg/slsa/image.go +++ b/pkg/slsa/image.go @@ -32,17 +32,17 @@ func NewImageReader(fetcher ImageFetcher) *reader { func (r *reader) Read(keychain authn.Keychain, repoName string) (string, string, map[string]string, error) { img, id, err := r.fetcher.Fetch(keychain, repoName) if err != nil { - return "", "", nil, fmt.Errorf("fetching image: %v", err) + return "", "", nil, fmt.Errorf("failed to fetch image: %v", err) } ref, err := name.NewDigest(id) if err != nil { - return "", "", nil, fmt.Errorf("parsing digest: %v", err) + return "", "", nil, fmt.Errorf("failed to parse digest: %v", err) } configFile, err := img.ConfigFile() if err != nil { - return "", "", nil, fmt.Errorf("getting image config: %v", err) + return "", "", nil, fmt.Errorf("failed to get image config: %v", err) } sha, found := strings.CutPrefix(ref.DigestStr(), "sha256:") @@ -62,7 +62,7 @@ func extractSourceFromLabel(labels map[string]string) (string, slsacommon.Digest var p project err := json.Unmarshal([]byte(metadata), &p) if err != nil { - return "", nil, fmt.Errorf("unmarshalling json: %v", err) + return "", nil, fmt.Errorf("failed to unmarshal json: %v", err) } switch p.Source.Type { diff --git a/pkg/slsa/pkcs8_signer.go b/pkg/slsa/pkcs8_signer.go index c53b55935..b67c91b0b 100644 --- a/pkg/slsa/pkcs8_signer.go +++ b/pkg/slsa/pkcs8_signer.go @@ -70,7 +70,7 @@ func (s *rsaSigner) Sign(ctx context.Context, data []byte) ([]byte, error) { hasher := hf.New() _, err := hasher.Write(data) if err != nil { - return nil, fmt.Errorf("hashing data: %v", err) + return nil, fmt.Errorf("failed to hash data: %v", err) } return rsa.SignPKCS1v15(nil, s.key, crypto.SHA256, hasher.Sum(nil)) diff --git a/pkg/slsa/sign.go b/pkg/slsa/sign.go index f5560ef9a..fe33d45c1 100644 --- a/pkg/slsa/sign.go +++ b/pkg/slsa/sign.go @@ -30,7 +30,7 @@ const ( func (*Attester) Sign(ctx context.Context, stmt intoto.Statement, signers ...Signer) ([]byte, error) { payload, err := json.Marshal(stmt) if err != nil { - return nil, fmt.Errorf("marshalling statement: %v", err) + return nil, fmt.Errorf("failed to marshal statement: %v", err) } pae := dsse.PAE(IntotoPayloadType, payload) @@ -38,12 +38,12 @@ func (*Attester) Sign(ctx context.Context, stmt intoto.Statement, signers ...Sig for i, signer := range signers { keyId, err := signer.KeyID() if err != nil { - return nil, fmt.Errorf("retrieving keyid: %v", err) + return nil, fmt.Errorf("failed to retreive keyid: %v", err) } sig, err := signer.Sign(ctx, pae) if err != nil { - return nil, fmt.Errorf("signing payload using '%v': %v", keyId, err) + return nil, fmt.Errorf("failed to sign payload using '%v': %v", keyId, err) } sigs[i] = dsse.Signature{ @@ -60,7 +60,7 @@ func (*Attester) Sign(ctx context.Context, stmt intoto.Statement, signers ...Sig b, err := json.Marshal(envelope) if err != nil { - return nil, fmt.Errorf("marshalling envelope: %v", err) + return nil, fmt.Errorf("failed to marshal envelope: %v", err) } return b, nil @@ -76,22 +76,22 @@ func (*Attester) Write(ctx context.Context, digestStr string, payload []byte, ke attestation, err := cosignstatic.NewAttestation(payload, opts...) if err != nil { - return nil, "", fmt.Errorf(":%v", err) + return nil, "", fmt.Errorf("failed to create attestation: %v", err) } ref, err := name.ParseReference(digestStr) if err != nil { - return nil, "", fmt.Errorf(":%v", err) + return nil, "", fmt.Errorf("failed to parse reference: %v", err) } attestationTag, err := cosignremote.AttestationTag(ref) if err != nil { - return nil, "", fmt.Errorf(":%v", err) + return nil, "", fmt.Errorf("failed to get attestation tag:%v", err) } annots, err := attestation.Annotations() if err != nil { - return nil, "", fmt.Errorf("getting attestation annotations: %v", err) + return nil, "", fmt.Errorf("failed to get attestation annotations: %v", err) } // Overwrite any existing attestations with a new one. The only time this is @@ -115,12 +115,12 @@ func (*Attester) Write(ctx context.Context, digestStr string, payload []byte, ke err = remote.Write(attestationTag, img, remoteOpts...) if err != nil { - return nil, "", fmt.Errorf(":%v", err) + return nil, "", fmt.Errorf("failed to write attestation: %v", err) } signatureDigest, err := img.Digest() if err != nil { - return nil, "", fmt.Errorf(":%v", err) + return nil, "", fmt.Errorf("failed to retrieve digest: %v", err) } return img, fmt.Sprintf("%v@%v", attestationTag.Context().Name(), signatureDigest.String()), nil