Skip to content

Commit 00bdd4d

Browse files
author
Bohan Chen
committed
refactor slsa.go and fix and bug fixes
rename slsa.go as the name wasn't really descriptive of what its doing bugs: - client-go strips the TypeMeta from the object so we have to pass it in manually. - granting rbac for the controller to get namespaces - and using version instead of identifier so we don't get the git sha when generating the build type link - ecdsa signing requires digest not full msg - 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 <bohanc@vmware.com>
1 parent 1d1f3ec commit 00bdd4d

11 files changed

Lines changed: 118 additions & 130 deletions

File tree

‎cmd/controller/main.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ func main() {
210210
}
211211

212212
slsaAttester := slsa.Attester{
213-
Version: cmd.Identifer,
213+
Version: cmd.Version,
214214

215215
LifecycleProvider: lifecycleProvider,
216216
ImageReader: slsa.NewImageReader(&registry.Client{}),

‎config/controllerrole.yaml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ rules:
4646
resources:
4747
- secrets
4848
- pods/log
49+
- namespaces
4950
verbs:
5051
- get
5152
- apiGroups:

‎pkg/reconciler/build/build.go‎

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"github.com/google/go-containerregistry/pkg/authn"
99
ggcrv1 "github.com/google/go-containerregistry/pkg/v1"
1010
intoto "github.com/in-toto/in-toto-golang/in_toto"
11-
"github.com/pkg/errors"
1211
"go.uber.org/zap"
1312
corev1 "k8s.io/api/core/v1"
1413
"k8s.io/apimachinery/pkg/api/equality"
@@ -59,7 +58,7 @@ type PodProgressLogger interface {
5958

6059
//go:generate counterfeiter . SLSAAttester
6160
type SLSAAttester interface {
62-
GenerateStatement(build *buildapi.Build, buildMetadata *cnb.BuildMetadata, pod *corev1.Pod, builderAndAppKeychain authn.Keychain, builderID slsa.BuilderID, depFns ...slsa.BuilderDependencyFn) (intoto.Statement, error)
61+
AttestBuild(build *buildapi.Build, buildMetadata *cnb.BuildMetadata, pod *corev1.Pod, builderAndAppKeychain authn.Keychain, builderID slsa.BuilderID, depFns ...slsa.BuilderDependencyFn) (intoto.Statement, error)
6362
Sign(ctx context.Context, stmt intoto.Statement, signers ...slsa.Signer) ([]byte, error)
6463
Write(ctx context.Context, digestStr string, payload []byte, keychain authn.Keychain) (ggcrv1.Image, string, error)
6564
}
@@ -206,7 +205,7 @@ func (c *Reconciler) reconcile(ctx context.Context, build *buildapi.Build) error
206205
if c.FeatureFlags.GenerateSlsaAttestation {
207206
attestDigest, err = c.attestBuild(ctx, build, buildMetadata, pod)
208207
if err != nil {
209-
return fmt.Errorf("attesting build: %v", err)
208+
return fmt.Errorf("failed to attest build: %v", err)
210209
}
211210
}
212211

@@ -399,7 +398,7 @@ func (c *Reconciler) buildMetadataFromBuildPod(pod *corev1.Pod) (*cnb.BuildMetad
399398
return cnb.DecompressBuildMetadata(status.State.Terminated.Message)
400399
}
401400
}
402-
return nil, errors.New(buildapi.CompletionContainerName + " container not found")
401+
return nil, fmt.Errorf("%v container not found", buildapi.CompletionContainerName)
403402
}
404403

405404
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
414413

415414
controllerSecrets, err := c.SecretFetcher.SecretsForSystemServiceAccount(ctx)
416415
if err != nil {
417-
return "", fmt.Errorf("getting controller secrets: %v", err)
416+
return "", fmt.Errorf("failed to get controller secrets: %v", err)
418417
}
419418

420419
buildSecrets, err := c.SecretFetcher.SecretsForServiceAccount(ctx, build.ServiceAccount(), build.Namespace)
421420
if err != nil {
422-
return "", fmt.Errorf("getting service account secrets: %v", err)
421+
return "", fmt.Errorf("failed to get service account secrets: %v", err)
423422
}
424423

425424
secrets := append(controllerSecrets, buildSecrets...)
426425
signingKeys, err := secret.FilterAndExtractSLSASecrets(secrets)
427426
if err != nil {
428-
return "", fmt.Errorf("parsing slsa secrets: %v", err)
427+
return "", fmt.Errorf("failed to parse slsa secrets: %v", err)
429428
}
430429

431430
signers := make([]slsa.Signer, len(signingKeys))
@@ -438,7 +437,7 @@ func (c *Reconciler) attestBuild(ctx context.Context, build *buildapi.Build, bui
438437
s, err = slsa.NewPKCS8Signer(key.Key, key.SecretName)
439438
}
440439
if err != nil {
441-
return "", fmt.Errorf("creating signer: %v", err)
440+
return "", fmt.Errorf("failed to create signer: %v", err)
442441
}
443442
signers[i] = s
444443
}
@@ -450,22 +449,22 @@ func (c *Reconciler) attestBuild(ctx context.Context, build *buildapi.Build, bui
450449

451450
deps, err := c.attestBuildDeps(ctx, build, pod, secrets)
452451
if err != nil {
453-
return "", fmt.Errorf("gathering build deps: %v", err)
452+
return "", fmt.Errorf("failed to gather build deps: %v", err)
454453
}
455454

456-
statement, err := c.Attester.GenerateStatement(build, buildMetadata, pod, keychain, buildId, deps...)
455+
statement, err := c.Attester.AttestBuild(build, buildMetadata, pod, keychain, buildId, deps...)
457456
if err != nil {
458-
return "", fmt.Errorf("generating statement: %v", err)
457+
return "", fmt.Errorf("failed to generate statement: %v", err)
459458
}
460459

461460
payload, err := c.Attester.Sign(ctx, statement, signers...)
462461
if err != nil {
463-
return "", fmt.Errorf("signing statement: %v", err)
462+
return "", fmt.Errorf("failed to sign statement: %v", err)
464463
}
465464

466465
_, digest, err := c.Attester.Write(ctx, buildMetadata.LatestImage, payload, keychain)
467466
if err != nil {
468-
return "", fmt.Errorf("writting attestation: %v", err)
467+
return "", fmt.Errorf("failed to write attestation: %v", err)
469468
}
470469

471470
return digest, nil
@@ -483,10 +482,10 @@ func (c *Reconciler) attestBuildDeps(ctx context.Context, build *buildapi.Build,
483482
}
484483

485484
deps := []slsa.BuilderDependencyFn{
486-
slsa.WithVersionedObject(ns),
487-
slsa.WithVersionedObject(build),
488-
slsa.WithVersionedObject(pod),
489-
slsa.WithVersionedObject(sa),
485+
slsa.WithVersionedObject("Namespace", ns),
486+
slsa.WithVersionedObject("Build", build),
487+
slsa.WithVersionedObject("Pod", pod),
488+
slsa.WithVersionedObject("ServiceAccount", sa),
490489
}
491490

492491
attestSecrets := make([]slsa.K8sObject, len(secrets))
@@ -495,7 +494,7 @@ func (c *Reconciler) attestBuildDeps(ctx context.Context, build *buildapi.Build,
495494
}
496495

497496
if len(attestSecrets) != 0 {
498-
deps = append(deps, slsa.WithVersionedObjects(attestSecrets))
497+
deps = append(deps, slsa.WithVersionedObjects("Secrets", attestSecrets))
499498
}
500499

501500
return deps, nil

‎pkg/reconciler/build/build_test.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1546,7 +1546,7 @@ func testBuildReconciler(t *testing.T, when spec.G, it spec.S) {
15461546
bld.ResourceVersion = "0"
15471547

15481548
it.Before(func() {
1549-
fakeAttester.GenerateStatementReturns(in_toto.Statement{}, nil)
1549+
fakeAttester.AttestBuildReturns(in_toto.Statement{}, nil)
15501550
fakeAttester.WriteReturns(nil, "some-attestation-image", nil)
15511551
fakeSecretFetcher.SecretsForServiceAccountReturns([]*corev1.Secret{}, nil)
15521552
fakeSecretFetcher.SecretsForSystemServiceAccountReturns([]*corev1.Secret{}, nil)
@@ -1584,8 +1584,8 @@ func testBuildReconciler(t *testing.T, when spec.G, it spec.S) {
15841584
},
15851585
})
15861586

1587-
require.Equal(t, 1, fakeAttester.GenerateStatementCallCount())
1588-
_, _, _, _, id, deps := fakeAttester.GenerateStatementArgsForCall(0)
1587+
require.Equal(t, 1, fakeAttester.AttestBuildCallCount())
1588+
_, _, _, _, id, deps := fakeAttester.AttestBuildArgsForCall(0)
15891589
require.Equal(t, slsa.BuilderID("https://kpack.io/slsa/unsigned-build"), id)
15901590
require.Len(t, deps, 4)
15911591

@@ -1622,8 +1622,8 @@ func testBuildReconciler(t *testing.T, when spec.G, it spec.S) {
16221622
},
16231623
})
16241624

1625-
require.Equal(t, 1, fakeAttester.GenerateStatementCallCount())
1626-
_, _, _, _, id, deps := fakeAttester.GenerateStatementArgsForCall(0)
1625+
require.Equal(t, 1, fakeAttester.AttestBuildCallCount())
1626+
_, _, _, _, id, deps := fakeAttester.AttestBuildArgsForCall(0)
16271627
require.Equal(t, slsa.BuilderID("https://kpack.io/slsa/signed-build"), id)
16281628
require.Len(t, deps, 5)
16291629

@@ -1659,8 +1659,8 @@ func testBuildReconciler(t *testing.T, when spec.G, it spec.S) {
16591659
},
16601660
})
16611661

1662-
require.Equal(t, 1, fakeAttester.GenerateStatementCallCount())
1663-
_, _, _, _, id, deps := fakeAttester.GenerateStatementArgsForCall(0)
1662+
require.Equal(t, 1, fakeAttester.AttestBuildCallCount())
1663+
_, _, _, _, id, deps := fakeAttester.AttestBuildArgsForCall(0)
16641664
require.Equal(t, slsa.BuilderID("https://kpack.io/slsa/signed-build"), id)
16651665
require.Len(t, deps, 5)
16661666

‎pkg/reconciler/build/buildfakes/fake_slsaattester.go‎

Lines changed: 39 additions & 39 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)