From 503813d8908353433fe9974f8c690207b395ba54 Mon Sep 17 00:00:00 2001 From: chaosinthecrd Date: Fri, 1 Dec 2023 15:16:48 +0000 Subject: [PATCH 1/2] refactored to handle error wrapping inside the log package Signed-off-by: chaosinthecrd --- log/log.go | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/log/log.go b/log/log.go index 3949f272..14d54256 100644 --- a/log/log.go +++ b/log/log.go @@ -14,6 +14,10 @@ package log +import ( + "fmt" +) + var log Logger = SilentLogger{} // Logger is used by witness library code to print out relevant information at runtime. @@ -40,7 +44,8 @@ func GetLogger() Logger { } func Errorf(format string, args ...interface{}) { - log.Errorf(format, args...) + err := fmt.Errorf(format, args...) + log.Error(err) } func Error(args ...interface{}) { @@ -48,7 +53,13 @@ func Error(args ...interface{}) { } func Warnf(format string, args ...interface{}) { - log.Warnf(format, args...) + // We want to wrap the error if there is one. + for _, a := range args { + if _, ok := a.(error); ok { + err := fmt.Errorf(format, args...) + log.Warn(err) + } + } } func Warn(args ...interface{}) { @@ -56,7 +67,12 @@ func Warn(args ...interface{}) { } func Debugf(format string, args ...interface{}) { - log.Debugf(format, args...) + for _, a := range args { + if _, ok := a.(error); ok { + err := fmt.Errorf(format, args...) + log.Debug(err) + } + } } func Debug(args ...interface{}) { From 20b8f9dc85873a94cb8fd68c5acd437d3281bd00 Mon Sep 17 00:00:00 2001 From: chaosinthecrd Date: Fri, 1 Dec 2023 16:29:28 +0000 Subject: [PATCH 2/2] using %w directive wherever possible Signed-off-by: chaosinthecrd --- attestation/aws-iid/aws-iid.go | 26 ++++++++++++------------- attestation/commandrun/tracing_linux.go | 4 ++-- attestation/context.go | 2 +- attestation/factory.go | 2 +- attestation/gcp-iit/gcp-iit.go | 15 +++++++------- attestation/github/github.go | 4 ++-- attestation/gitlab/gitlab.go | 6 +++--- attestation/maven/maven.go | 4 ++-- attestation/oci/oci.go | 10 +++++----- attestation/sarif/sarif.go | 4 ++-- policy/policy.go | 2 +- signer/file/file.go | 10 +++++----- source/verified.go | 2 +- verify.go | 2 +- 14 files changed, 47 insertions(+), 46 deletions(-) diff --git a/attestation/aws-iid/aws-iid.go b/attestation/aws-iid/aws-iid.go index fd9ad36b..874fbd6d 100644 --- a/attestation/aws-iid/aws-iid.go +++ b/attestation/aws-iid/aws-iid.go @@ -134,12 +134,12 @@ func (a *Attestor) getIID() error { svc := ec2metadata.New(&a.session, a.conf) iid, err := svc.GetDynamicData(docPath) if err != nil { - return fmt.Errorf("failed to get instance identity document: %v", err) + return fmt.Errorf("failed to get instance identity document: %w", err) } sig, err := svc.GetDynamicData(sigPath) if err != nil { - return fmt.Errorf("failed to get signature: %v", err) + return fmt.Errorf("failed to get signature: %w", err) } a.RawIID = iid @@ -147,7 +147,7 @@ func (a *Attestor) getIID() error { err = json.Unmarshal([]byte(a.RawIID), &a.EC2InstanceIdentityDocument) if err != nil { - return fmt.Errorf("failed to unmarshal iid: %v", err) + return fmt.Errorf("failed to unmarshal iid: %w", err) } return nil @@ -161,17 +161,17 @@ func (a *Attestor) Verify() error { docHash := sha256.Sum256([]byte(a.RawIID)) sigBytes, err := base64.StdEncoding.DecodeString(a.RawSig) if err != nil { - return fmt.Errorf("failed to decode signature: %v", err) + return fmt.Errorf("failed to decode signature: %w", err) } pubKey, err := getAWSCAPublicKey() if err != nil { - return fmt.Errorf("failed to get AWS public key: %v", err) + return fmt.Errorf("failed to get AWS public key: %w", err) } pubKeyBytes, err := x509.MarshalPKIXPublicKey(pubKey) if err != nil { - return fmt.Errorf("failed to marshal public key: %v", err) + return fmt.Errorf("failed to marshal public key: %w", err) } pem := pem.EncodeToMemory(&pem.Block{ @@ -182,12 +182,12 @@ func (a *Attestor) Verify() error { a.PublicKey = string(pem) if err != nil { - return fmt.Errorf("failed to encode public key: %v", err) + return fmt.Errorf("failed to encode public key: %w", err) } err = rsa.VerifyPKCS1v15(pubKey, crypto.SHA256, docHash[:], sigBytes) if err != nil { - log.Debugf("(attestation/aws-iid) failed to verify signature: %v", err) + log.Debugf("(attestation/aws-iid) failed to verify signature: %w", err) return nil } @@ -200,25 +200,25 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet { if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.EC2InstanceIdentityDocument.InstanceID), hashes); err == nil { subjects[fmt.Sprintf("instanceid:%s", a.EC2InstanceIdentityDocument.InstanceID)] = ds } else { - log.Debugf("(attestation/aws) failed to record aws instanceid subject: %v", err) + log.Debugf("(attestation/aws) failed to record aws instanceid subject: %w", err) } if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.EC2InstanceIdentityDocument.AccountID), hashes); err == nil { subjects[fmt.Sprintf("accountid:%s", a.EC2InstanceIdentityDocument.AccountID)] = ds } else { - log.Debugf("(attestation/aws) failed to record aws accountid subject: %v", err) + log.Debugf("(attestation/aws) failed to record aws accountid subject: %w", err) } if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.EC2InstanceIdentityDocument.ImageID), hashes); err == nil { subjects[fmt.Sprintf("imageid:%s", a.EC2InstanceIdentityDocument.ImageID)] = ds } else { - log.Debugf("(attestation/aws) failed to record aws imageid subject: %v", err) + log.Debugf("(attestation/aws) failed to record aws imageid subject: %w", err) } if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.EC2InstanceIdentityDocument.PrivateIP), hashes); err == nil { subjects[fmt.Sprintf("privateip:%s", a.EC2InstanceIdentityDocument.PrivateIP)] = ds } else { - log.Debugf("(attestation/aws) failed to record aws privateip subject: %v", err) + log.Debugf("(attestation/aws) failed to record aws privateip subject: %w", err) } return subjects @@ -232,7 +232,7 @@ func getAWSCAPublicKey() (*rsa.PublicKey, error) { cert, err := x509.ParseCertificate(block.Bytes) if err != nil { - return nil, fmt.Errorf("failed to parse certificate: %v", err) + return nil, fmt.Errorf("failed to parse certificate: %w", err) } return cert.PublicKey.(*rsa.PublicKey), nil diff --git a/attestation/commandrun/tracing_linux.go b/attestation/commandrun/tracing_linux.go index c9e63dc2..e9cce1bd 100644 --- a/attestation/commandrun/tracing_linux.go +++ b/attestation/commandrun/tracing_linux.go @@ -112,12 +112,12 @@ func (p *ptraceContext) runTrace() error { if status.Stopped() && isPtraceTrap { injectedSig = 0 if err := p.nextSyscall(pid); err != nil { - log.Debugf("(tracing) got error while processing syscall: %v", err) + log.Debugf("(tracing) got error while processing syscall: %w", err) } } if err := unix.PtraceSyscall(pid, injectedSig); err != nil { - log.Debugf("(tracing) got error from ptrace syscall: %v", err) + log.Debugf("(tracing) got error from ptrace syscall: %w", err) } } } diff --git a/attestation/context.go b/attestation/context.go index 80bab148..d0e1a455 100644 --- a/attestation/context.go +++ b/attestation/context.go @@ -185,7 +185,7 @@ func (ctx *AttestationContext) runAttestor(attestor Attestor) error { log.Infof("Starting %v attestor...", attestor.Name()) startTime := time.Now() if err := attestor.Attest(ctx); err != nil { - log.Errorf("Error running %v attestor: %v", attestor.Name(), err) + log.Errorf("Error running %v attestor: %w", attestor.Name(), err) ctx.completedAttestors = append(ctx.completedAttestors, CompletedAttestor{ Attestor: attestor, StartTime: startTime, diff --git a/attestation/factory.go b/attestation/factory.go index cfcf2cc6..ef50a60c 100644 --- a/attestation/factory.go +++ b/attestation/factory.go @@ -35,7 +35,7 @@ type Attestor interface { } // Subjecter allows attestors to expose bits of information that will be added to -// the in-toto statement as subjects. External services such as Rekor and Archivist +// the in-toto statement as subjects. External services such as Rekor and Archivista // use in-toto subjects as indexes back to attestations. type Subjecter interface { Subjects() map[string]cryptoutil.DigestSet diff --git a/attestation/gcp-iit/gcp-iit.go b/attestation/gcp-iit/gcp-iit.go index d970acc8..c2cdfdbd 100644 --- a/attestation/gcp-iit/gcp-iit.go +++ b/attestation/gcp-iit/gcp-iit.go @@ -102,6 +102,7 @@ func (a *Attestor) Attest(ctx *attestation.AttestationContext) error { tokenURL := identityTokenURL(defaultIdentityTokenHost, defaultServiceAccount) identityToken, err := getMetadata(tokenURL) if err != nil { + // status.Errorf does not support %w directive return status.Errorf(codes.Internal, "unable to retrieve valid identity token: %v", err) } @@ -150,7 +151,7 @@ func (a *Attestor) getInstanceData() { for k, v := range endpoints { data, err := getMetadata(v) if err != nil { - log.Warnf("failed to retrieve gcp metadata from %v: %v", v, err) + log.Warnf("failed to retrieve gcp metadata from %v: %w", v, err) continue } metadata[k] = string(data) @@ -165,7 +166,7 @@ func (a *Attestor) getInstanceData() { projID, projNum, err := parseJWTProjectInfo(a.JWT) if err != nil { - log.Warnf("unable to parse gcp project info from JWT: %v\n", err) + log.Warnf("unable to parse gcp project info from JWT: %w\n", err) } a.ProjectID = projID @@ -179,31 +180,31 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet { if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.InstanceID), hashes); err == nil { subjects[fmt.Sprintf("instanceid:%v", a.InstanceID)] = ds } else { - log.Debugf("(attestation/gcp) failed to record gcp instanceid subject: %v", err) + log.Debugf("(attestation/gcp) failed to record gcp instanceid subject: %w", err) } if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.InstanceHostname), hashes); err == nil { subjects[fmt.Sprintf("instancename:%v", a.InstanceHostname)] = ds } else { - log.Debugf("(attestation/gcp) failed to record gcp instancename subject: %v", err) + log.Debugf("(attestation/gcp) failed to record gcp instancename subject: %w", err) } if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectID), hashes); err == nil { subjects[fmt.Sprintf("projectid:%v", a.ProjectID)] = ds } else { - log.Debugf("(attestation/gcp) failed to record gcp projectid subject: %v", err) + log.Debugf("(attestation/gcp) failed to record gcp projectid subject: %w", err) } if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectNumber), hashes); err == nil { subjects[fmt.Sprintf("projectnumber:%v", a.ProjectNumber)] = ds } else { - log.Debugf("(attestation/gcp) failed to record gcp projectnumber subject: %v", err) + log.Debugf("(attestation/gcp) failed to record gcp projectnumber subject: %w", err) } if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ClusterUID), hashes); err == nil { subjects[fmt.Sprintf("clusteruid:%v", a.ClusterUID)] = ds } else { - log.Debugf("(attestation/gcp) failed to record gcp clusteruid subject: %v", err) + log.Debugf("(attestation/gcp) failed to record gcp clusteruid subject: %w", err) } return subjects diff --git a/attestation/github/github.go b/attestation/github/github.go index ef394210..cece9c0c 100644 --- a/attestation/github/github.go +++ b/attestation/github/github.go @@ -140,13 +140,13 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet { if pipelineSubj, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.PipelineUrl), hashes); err == nil { subjects[fmt.Sprintf("pipelineurl:%v", a.PipelineUrl)] = pipelineSubj } else { - log.Debugf("(attestation/github) failed to record github pipelineurl subject: %v", err) + log.Debugf("(attestation/github) failed to record github pipelineurl subject: %w", err) } if projectSubj, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectUrl), hashes); err == nil { subjects[fmt.Sprintf("projecturl:%v", a.ProjectUrl)] = projectSubj } else { - log.Debugf("(attestation/github) failed to record github projecturl subject: %v", err) + log.Debugf("(attestation/github) failed to record github projecturl subject: %w", err) } return subjects diff --git a/attestation/gitlab/gitlab.go b/attestation/gitlab/gitlab.go index ba0b9517..5d6c1a80 100644 --- a/attestation/gitlab/gitlab.go +++ b/attestation/gitlab/gitlab.go @@ -122,19 +122,19 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet { if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.PipelineUrl), hashes); err == nil { subjects[fmt.Sprintf("pipelineurl:%v", a.PipelineUrl)] = ds } else { - log.Debugf("(attestation/gitlab) failed to record gitlab pipelineurl subject: %v", err) + log.Debugf("(attestation/gitlab) failed to record gitlab pipelineurl subject: %w", err) } if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.JobUrl), hashes); err == nil { subjects[fmt.Sprintf("joburl:%v", a.JobUrl)] = ds } else { - log.Debugf("(attestation/gitlab) failed to record gitlab joburl subject: %v", err) + log.Debugf("(attestation/gitlab) failed to record gitlab joburl subject: %w", err) } if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(a.ProjectUrl), hashes); err == nil { subjects[fmt.Sprintf("projecturl:%v", a.ProjectUrl)] = ds } else { - log.Debugf("(attestation/gitlab) failed to record gitlab projecturl subject: %v", err) + log.Debugf("(attestation/gitlab) failed to record gitlab projecturl subject: %w", err) } return subjects diff --git a/attestation/maven/maven.go b/attestation/maven/maven.go index 801852a8..985b4ab1 100644 --- a/attestation/maven/maven.go +++ b/attestation/maven/maven.go @@ -121,14 +121,14 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet { if ds, err := cryptoutil.CalculateDigestSetFromBytes([]byte(projectSubject), hashes); err == nil { subjects[projectSubject] = ds } else { - log.Debugf("(attestation/maven) failed to record %v subject: %v", projectSubject, err) + log.Debugf("(attestation/maven) failed to record %v subject: %w", projectSubject, err) } for _, dep := range a.Dependencies { depSubject := fmt.Sprintf("dependency:%v/%v@%v", dep.GroupId, dep.ArtifactId, dep.Version) depDigest, err := cryptoutil.CalculateDigestSetFromBytes([]byte(depSubject), hashes) if err != nil { - log.Debugf("(attestation/maven) failed to record %v subject: %v", depSubject, err) + log.Debugf("(attestation/maven) failed to record %v subject: %w", depSubject, err) } subjects[depSubject] = depDigest diff --git a/attestation/oci/oci.go b/attestation/oci/oci.go index de8f3386..f16569af 100644 --- a/attestation/oci/oci.go +++ b/attestation/oci/oci.go @@ -99,7 +99,7 @@ func (m *Manifest) getImageID(ctx *attestation.AttestationContext, tarFilePath s imageID, err := cryptoutil.CalculateDigestSetFromBytes(b, ctx.Hashes()) if err != nil { - log.Debugf("(attestation/oci) error calculating image id: %v", err) + log.Debugf("(attestation/oci) error calculating image id: %w", err) return nil, err } @@ -127,18 +127,18 @@ func (a *Attestor) RunType() attestation.RunType { func (a *Attestor) Attest(ctx *attestation.AttestationContext) error { if err := a.getCandidate(ctx); err != nil { - log.Debugf("(attestation/oci) error getting candidate: %v", err) + log.Debugf("(attestation/oci) error getting candidate: %w", err) return err } if err := a.parseMaifest(ctx); err != nil { - log.Debugf("(attestation/oci) error parsing manifest: %v", err) + log.Debugf("(attestation/oci) error parsing manifest: %w", err) return err } imageID, err := a.Manifest[0].getImageID(ctx, a.tarFilePath) if err != nil { - log.Debugf("(attestation/oci) error getting image id: %v", err) + log.Debugf("(attestation/oci) error getting image id: %w", err) return err } @@ -241,7 +241,7 @@ func (a *Attestor) Subjects() map[string]cryptoutil.DigestSet { for _, tag := range a.ImageTags { hash, err := cryptoutil.CalculateDigestSetFromBytes([]byte(tag), hashes) if err != nil { - log.Debugf("(attestation/oci) error calculating image tag: %v", err) + log.Debugf("(attestation/oci) error calculating image tag: %w", err) continue } subj[fmt.Sprintf("imagetag:%s", tag)] = hash diff --git a/attestation/sarif/sarif.go b/attestation/sarif/sarif.go index c5e09972..8c21d8ca 100644 --- a/attestation/sarif/sarif.go +++ b/attestation/sarif/sarif.go @@ -71,7 +71,7 @@ func (a *Attestor) RunType() attestation.RunType { func (a *Attestor) Attest(ctx *attestation.AttestationContext) error { if err := a.getCandidate(ctx); err != nil { - log.Debugf("(attestation/sarif) error getting candidate: %v", err) + log.Debugf("(attestation/sarif) error getting candidate: %w", err) return err } @@ -113,7 +113,7 @@ func (a *Attestor) getCandidate(ctx *attestation.AttestationContext) error { //check to see if we can unmarshal into sarif type if err := json.Unmarshal(reportBytes, &a.Report); err != nil { - log.Debugf("(attestation/sarif) error unmarshaling report: %v", err) + log.Debugf("(attestation/sarif) error unmarshaling report: %w", err) continue } diff --git a/policy/policy.go b/policy/policy.go index c4f57deb..b6456b1f 100644 --- a/policy/policy.go +++ b/policy/policy.go @@ -238,7 +238,7 @@ func (step Step) checkFunctionaries(verifiedStatements []source.VerifiedCollecti for _, verifier := range verifiedStatement.Verifiers { verifierID, err := verifier.KeyID() if err != nil { - log.Debugf("(policy) skipping verifier: could not get key id: %v", err) + log.Debugf("(policy) skipping verifier: could not get key id: %w", err) continue } diff --git a/signer/file/file.go b/signer/file/file.go index a52247f8..95256daa 100644 --- a/signer/file/file.go +++ b/signer/file/file.go @@ -110,20 +110,20 @@ func New(opts ...Option) FileSignerProvider { func (fsp FileSignerProvider) Signer(ctx context.Context) (cryptoutil.Signer, error) { keyFile, err := os.Open(fsp.KeyPath) if err != nil { - return nil, fmt.Errorf("failed to open key file: %v", err) + return nil, fmt.Errorf("failed to open key file: %w", err) } defer keyFile.Close() key, err := cryptoutil.TryParseKeyFromReader(keyFile) if err != nil { - return nil, fmt.Errorf("failed to load key: %v", err) + return nil, fmt.Errorf("failed to load key: %w", err) } signerOpts := []cryptoutil.SignerOption{} if fsp.CertPath != "" { leaf, err := loadCert(fsp.CertPath) if err != nil { - return nil, fmt.Errorf("failed to load certificate: %v", err) + return nil, fmt.Errorf("failed to load certificate: %w", err) } signerOpts = append(signerOpts, cryptoutil.SignWithCertificate(leaf)) @@ -134,7 +134,7 @@ func (fsp FileSignerProvider) Signer(ctx context.Context) (cryptoutil.Signer, er for _, path := range fsp.IntermediatePaths { cert, err := loadCert(path) if err != nil { - return nil, fmt.Errorf("failed to load intermediate: %v", err) + return nil, fmt.Errorf("failed to load intermediate: %w", err) } intermediates = append(intermediates, cert) @@ -149,7 +149,7 @@ func (fsp FileSignerProvider) Signer(ctx context.Context) (cryptoutil.Signer, er func loadCert(path string) (*x509.Certificate, error) { certFile, err := os.Open(path) if err != nil { - return nil, fmt.Errorf("failed to load certificate: %v", err) + return nil, fmt.Errorf("failed to load certificate: %w", err) } defer certFile.Close() diff --git a/source/verified.go b/source/verified.go index 7b9c92bb..0bbc0058 100644 --- a/source/verified.go +++ b/source/verified.go @@ -50,7 +50,7 @@ func (s *VerifiedSource) Search(ctx context.Context, collectionName string, subj for _, toVerify := range unverified { envelopeVerifiers, err := toVerify.Envelope.Verify(s.verifyOpts...) if err != nil { - log.Debugf("(verified source) skipping envelope: couldn't verify enveloper's signature with the policy's verifiers: %+v", err) + log.Debugf("(verified source) skipping envelope: couldn't verify enveloper's signature with the policy's verifiers: %w", err) continue } diff --git a/verify.go b/verify.go index daa91902..857aa09b 100644 --- a/verify.go +++ b/verify.go @@ -32,7 +32,7 @@ func VerifySignature(r io.Reader, verifiers ...cryptoutil.Verifier) (dsse.Envelo decoder := json.NewDecoder(r) envelope := dsse.Envelope{} if err := decoder.Decode(&envelope); err != nil { - return envelope, fmt.Errorf("failed to parse dsse envelope: %v", err) + return envelope, fmt.Errorf("failed to parse dsse envelope: %w", err) } _, err := envelope.Verify(dsse.VerifyWithVerifiers(verifiers...))