From b482840e83b0c6cf214cd221c1717cb7494c380d Mon Sep 17 00:00:00 2001 From: pxp928 <parth.psu@gmail.com> Date: Tue, 5 Apr 2022 08:26:42 -0400 Subject: [PATCH] [TEP-0089] - Implement Non-falsifiable provenance support Signed-off-by: pxp928 <parth.psu@gmail.com> --- pkg/reconciler/taskrun/taskrun.go | 28 ++++ pkg/reconciler/taskrun/taskrun_test.go | 120 ++++++++++++++ pkg/spire/sign.go | 47 ++++++ pkg/spire/spire.go | 12 ++ pkg/spire/spire_mock.go | 88 ++++++++++ pkg/spire/spire_mock_test.go | 214 +++++++++++++++++++++++++ pkg/spire/verify.go | 100 ++++++++++++ test/embed_test.go | 6 + test/entrypoint_test.go | 6 + test/helm_task_test.go | 1 + test/hermetic_taskrun_test.go | 2 + test/ignore_step_error_test.go | 1 + test/init_test.go | 24 +++ test/kaniko_task_test.go | 1 + test/pipelinefinally_test.go | 21 +++ test/pipelinerun_test.go | 3 + test/status_test.go | 2 + test/taskrun_test.go | 3 + 18 files changed, 679 insertions(+) diff --git a/pkg/reconciler/taskrun/taskrun.go b/pkg/reconciler/taskrun/taskrun.go index f555412e290..fca218e8b96 100644 --- a/pkg/reconciler/taskrun/taskrun.go +++ b/pkg/reconciler/taskrun/taskrun.go @@ -112,6 +112,20 @@ func (c *Reconciler) ReconcileKind(ctx context.Context, tr *v1beta1.TaskRun) pkg // on the event to perform user facing initialisations, such has reset a CI check status afterCondition := tr.Status.GetCondition(apis.ConditionSucceeded) events.Emit(ctx, nil, afterCondition, tr) + } else if config.FromContextOrDefaults(ctx).FeatureFlags.EnableSpire { + var verified = false + if c.SpireClient != nil { + if err := c.SpireClient.VerifyStatusInternalAnnotation(ctx, tr, logger); err == nil { + verified = true + } + if !verified { + if tr.Status.Annotations == nil { + tr.Status.Annotations = map[string]string{} + } + tr.Status.Annotations[spire.NotVerifiedAnnotation] = "yes" + } + logger.Infof("taskrun verification status: %t with hash %v \n", verified, tr.Status.Annotations[spire.TaskRunStatusHashAnnotation]) + } } // If the TaskRun is complete, run some post run fixtures when applicable @@ -260,6 +274,20 @@ func (c *Reconciler) finishReconcileUpdateEmitEvents(ctx context.Context, tr *v1 events.Emit(ctx, beforeCondition, afterCondition, tr) var err error + // Add status internal annotations hash only if it was verified + if config.FromContextOrDefaults(ctx).FeatureFlags.EnableSpire && + c.SpireClient != nil && c.SpireClient.CheckSpireVerifiedFlag(tr) { + if err := spire.CheckStatusInternalAnnotation(tr); err != nil { + err = c.SpireClient.AppendStatusInternalAnnotation(ctx, tr) + if err != nil { + logger.Warn("Failed to sign TaskRun internal status hash", zap.Error(err)) + events.EmitError(controller.GetEventRecorder(ctx), err, tr) + } else { + logger.Infof("Successfully signed TaskRun internal status with hash: %v", + tr.Status.Annotations[spire.TaskRunStatusHashAnnotation]) + } + } + } merr := multierror.Append(previousError, err).ErrorOrNil() diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index aa1f686c114..4a756f06254 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -44,6 +44,7 @@ import ( "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources" ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" "github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim" + "github.com/tektoncd/pipeline/pkg/spire" spireconfig "github.com/tektoncd/pipeline/pkg/spire/config" "github.com/tektoncd/pipeline/test" "github.com/tektoncd/pipeline/test/diff" @@ -63,6 +64,7 @@ import ( ktesting "k8s.io/client-go/testing" "k8s.io/client-go/tools/record" "knative.dev/pkg/apis" + duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" "knative.dev/pkg/changeset" cminformer "knative.dev/pkg/configmap/informer" "knative.dev/pkg/controller" @@ -3800,6 +3802,124 @@ status: } } +func TestReconcileOnTaskRunSign(t *testing.T) { + sc := &spire.MockClient{} + + taskSt := &apis.Condition{ + Type: apis.ConditionSucceeded, + Status: corev1.ConditionTrue, + Reason: "Build succeeded", + Message: "Build succeeded", + } + taskRunStartedUnsigned := &v1beta1.TaskRun{ + ObjectMeta: objectMeta("taskrun-started-unsigned", "foo"), + Spec: v1beta1.TaskRunSpec{ + TaskRef: &v1beta1.TaskRef{ + Name: simpleTask.Name, + }, + }, + Status: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: duckv1beta1.Conditions{ + *taskSt, + }, + }, + TaskRunStatusFields: v1beta1.TaskRunStatusFields{ + StartTime: &metav1.Time{Time: now.Add(-15 * time.Second)}, + }, + }, + } + taskRunUnstarted := &v1beta1.TaskRun{ + ObjectMeta: objectMeta("taskrun-unstarted", "foo"), + Spec: v1beta1.TaskRunSpec{ + TaskRef: &v1beta1.TaskRef{ + Name: simpleTask.Name, + }, + }, + Status: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: duckv1beta1.Conditions{ + *taskSt, + }, + }, + }, + } + taskRunStartedSigned := &v1beta1.TaskRun{ + ObjectMeta: objectMeta("taskrun-started-signed", "foo"), + Spec: v1beta1.TaskRunSpec{ + TaskRef: &v1beta1.TaskRef{ + Name: simpleTask.Name, + }, + }, + Status: v1beta1.TaskRunStatus{ + Status: duckv1beta1.Status{ + Conditions: duckv1beta1.Conditions{ + *taskSt, + }, + }, + }, + } + if err := sc.AppendStatusInternalAnnotation(context.Background(), taskRunStartedSigned); err != nil { + t.Fatal("failed to sign test taskrun") + } + + d := test.Data{ + ConfigMaps: []*corev1.ConfigMap{{ + ObjectMeta: metav1.ObjectMeta{Name: config.GetFeatureFlagsConfigName(), Namespace: system.Namespace()}, + Data: map[string]string{ + "enable-spire": "true", + }, + }}, + + TaskRuns: []*v1beta1.TaskRun{ + taskRunStartedUnsigned, taskRunUnstarted, taskRunStartedSigned, + }, + Tasks: []*v1beta1.Task{simpleTask}, + } + testAssets, cancel := getTaskRunController(t, d) + defer cancel() + c := testAssets.Controller + clients := testAssets.Clients + + testCases := []struct { + name string + tr *v1beta1.TaskRun + verifiable bool + }{ + { + name: "sign/verify unstarted taskrun", + tr: taskRunUnstarted, + verifiable: true, + }, + { + name: "sign/verify signed started taskrun", + tr: taskRunStartedSigned, + verifiable: true, + }, + { + name: "sign/verify unsigned started taskrun should fail", + tr: taskRunStartedUnsigned, + verifiable: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(tc.tr)); err != nil { + t.Fatalf("Unexpected error when reconciling completed TaskRun : %v", err) + } + newTr, err := clients.Pipeline.TektonV1beta1().TaskRuns(tc.tr.Namespace).Get(testAssets.Ctx, tc.tr.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Expected completed TaskRun %s to exist but instead got error when getting it: %v", tc.tr.Name, err) + } + verified := sc.CheckSpireVerifiedFlag(newTr) + if verified != tc.verifiable { + t.Fatalf("expected verifiable: %v, got %v", tc.verifiable, verified) + } + }) + } +} + func Test_validateTaskSpecRequestResources_ValidResources(t *testing.T) { ctx := context.Background() diff --git a/pkg/spire/sign.go b/pkg/spire/sign.go index 4df43ff7cbe..90d9772dee8 100644 --- a/pkg/spire/sign.go +++ b/pkg/spire/sign.go @@ -23,6 +23,7 @@ import ( "crypto/sha256" "encoding/base64" "encoding/pem" + "fmt" "strings" "github.com/spiffe/go-spiffe/v2/svid/x509svid" @@ -101,3 +102,49 @@ func getManifest(results []v1beta1.PipelineResourceResult) string { } return strings.Join(keys, ",") } + +// AppendStatusInternalAnnotation creates the status annotations which are used by the controller to verify the status hash +func (sc *spireControllerAPIClient) AppendStatusInternalAnnotation(ctx context.Context, tr *v1beta1.TaskRun) error { + err := sc.checkClient(ctx) + if err != nil { + return err + } + + // Add status hash + currentHash, err := hashTaskrunStatusInternal(tr) + if err != nil { + return err + } + + // Sign with controller private key + xsvid, err := sc.fetchSVID(ctx) + if err != nil { + return err + } + + sig, err := signWithKey(xsvid, currentHash) + if err != nil { + return err + } + + // Store Controller SVID + p := pem.EncodeToMemory(&pem.Block{ + Bytes: xsvid.Certificates[0].Raw, + Type: "CERTIFICATE", + }) + if tr.Status.Annotations == nil { + tr.Status.Annotations = map[string]string{} + } + tr.Status.Annotations[controllerSvidAnnotation] = string(p) + tr.Status.Annotations[TaskRunStatusHashAnnotation] = currentHash + tr.Status.Annotations[taskRunStatusHashSigAnnotation] = base64.StdEncoding.EncodeToString(sig) + return nil +} + +func (sc *spireControllerAPIClient) fetchSVID(ctx context.Context) (*x509svid.SVID, error) { + xsvid, err := sc.workloadAPI.FetchX509SVID(ctx) + if err != nil { + return nil, fmt.Errorf("failed to fetch controller SVID: %w", err) + } + return xsvid, nil +} diff --git a/pkg/spire/spire.go b/pkg/spire/spire.go index 5f912b99210..76781ed827c 100644 --- a/pkg/spire/spire.go +++ b/pkg/spire/spire.go @@ -20,10 +20,19 @@ import ( "context" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "go.uber.org/zap" corev1 "k8s.io/api/core/v1" ) const ( + // TaskRunStatusHashAnnotation TaskRun status annotation Hash Key + TaskRunStatusHashAnnotation = "tekton.dev/status-hash" + // taskRunStatusHashSigAnnotation TaskRun status annotation hash signature Key + taskRunStatusHashSigAnnotation = "tekton.dev/status-hash-sig" + // controllerSvidAnnotation TaskRun status annotation controller SVID Key + controllerSvidAnnotation = "tekton.dev/controller-svid" + // NotVerifiedAnnotation TaskRun status annotation not verified by spire key that get set when status match fails + NotVerifiedAnnotation = "tekton.dev/not-verified" // KeySVID key used by TaskRun SVID KeySVID = "SVID" // KeySignatureSuffix is the suffix of the keys that contain signatures @@ -34,9 +43,12 @@ const ( // ControllerAPIClient interface maps to the spire controller API to interact with spire type ControllerAPIClient interface { + AppendStatusInternalAnnotation(ctx context.Context, tr *v1beta1.TaskRun) error + CheckSpireVerifiedFlag(tr *v1beta1.TaskRun) bool Close() CreateEntries(ctx context.Context, tr *v1beta1.TaskRun, pod *corev1.Pod, ttl int) error DeleteEntry(ctx context.Context, tr *v1beta1.TaskRun, pod *corev1.Pod) error + VerifyStatusInternalAnnotation(ctx context.Context, tr *v1beta1.TaskRun, logger *zap.SugaredLogger) error VerifyTaskRunResults(ctx context.Context, prs []v1beta1.PipelineResourceResult, tr *v1beta1.TaskRun) error } diff --git a/pkg/spire/spire_mock.go b/pkg/spire/spire_mock.go index fa5c9ebc917..8ff7283473e 100644 --- a/pkg/spire/spire_mock.go +++ b/pkg/spire/spire_mock.go @@ -24,6 +24,7 @@ import ( "github.com/pkg/errors" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" + "go.uber.org/zap" corev1 "k8s.io/api/core/v1" ) @@ -43,16 +44,30 @@ type MockClient struct { // VerifyAlwaysReturns defines whether to always verify successfully or to always fail verification if non-nil. // This only take effect on Verify functions: + // - VerifyStatusInternalAnnotationOverride // - VerifyTaskRunResultsOverride VerifyAlwaysReturns *bool + // VerifyStatusInternalAnnotationOverride contains the function to overwrite a call to VerifyStatusInternalAnnotation + VerifyStatusInternalAnnotationOverride func(ctx context.Context, tr *v1beta1.TaskRun, logger *zap.SugaredLogger) error + // VerifyTaskRunResultsOverride contains the function to overwrite a call to VerifyTaskRunResults VerifyTaskRunResultsOverride func(ctx context.Context, prs []v1beta1.PipelineResourceResult, tr *v1beta1.TaskRun) error + // AppendStatusInternalAnnotationOverride contains the function to overwrite a call to AppendStatusInternalAnnotation + AppendStatusInternalAnnotationOverride func(ctx context.Context, tr *v1beta1.TaskRun) error + + // CheckSpireVerifiedFlagOverride contains the function to overwrite a call to CheckSpireVerifiedFlag + CheckSpireVerifiedFlagOverride func(tr *v1beta1.TaskRun) bool + // SignOverride contains the function to overwrite a call to Sign SignOverride func(ctx context.Context, results []v1beta1.PipelineResourceResult) ([]v1beta1.PipelineResourceResult, error) } +const ( + controllerSvid = "CONTROLLER_SVID_DATA" +) + func (*MockClient) mockSign(content, signedBy string) string { return fmt.Sprintf("signed-by-%s:%x", signedBy, sha256.Sum256([]byte(content))) } @@ -66,6 +81,38 @@ func (*MockClient) GetIdentity(tr *v1beta1.TaskRun) string { return fmt.Sprintf("/ns/%v/taskrun/%v", tr.Namespace, tr.Name) } +// AppendStatusInternalAnnotation creates the status annotations which are used by the controller to verify the status hash +func (sc *MockClient) AppendStatusInternalAnnotation(ctx context.Context, tr *v1beta1.TaskRun) error { + if sc.AppendStatusInternalAnnotationOverride != nil { + return sc.AppendStatusInternalAnnotationOverride(ctx, tr) + } + // Add status hash + currentHash, err := hashTaskrunStatusInternal(tr) + if err != nil { + return err + } + + if tr.Status.Annotations == nil { + tr.Status.Annotations = map[string]string{} + } + tr.Status.Annotations[controllerSvidAnnotation] = controllerSvid + tr.Status.Annotations[TaskRunStatusHashAnnotation] = currentHash + tr.Status.Annotations[taskRunStatusHashSigAnnotation] = sc.mockSign(currentHash, "controller") + return nil +} + +// CheckSpireVerifiedFlag checks if the not-verified status annotation is set which would result in spire verification failed +func (sc *MockClient) CheckSpireVerifiedFlag(tr *v1beta1.TaskRun) bool { + if sc.CheckSpireVerifiedFlagOverride != nil { + return sc.CheckSpireVerifiedFlagOverride(tr) + } + + if _, notVerified := tr.Status.Annotations[NotVerifiedAnnotation]; !notVerified { + return true + } + return false +} + // CreateEntries adds entries to the dictionary of entries that mock the SPIRE server datastore func (sc *MockClient) CreateEntries(ctx context.Context, tr *v1beta1.TaskRun, pod *corev1.Pod, ttl int) error { id := fmt.Sprintf("/ns/%v/taskrun/%v", tr.Namespace, tr.Name) @@ -85,6 +132,47 @@ func (sc *MockClient) DeleteEntry(ctx context.Context, tr *v1beta1.TaskRun, pod return nil } +// VerifyStatusInternalAnnotation checks that the internal status annotations are valid by the mocked spire client +func (sc *MockClient) VerifyStatusInternalAnnotation(ctx context.Context, tr *v1beta1.TaskRun, logger *zap.SugaredLogger) error { + if sc.VerifyStatusInternalAnnotationOverride != nil { + return sc.VerifyStatusInternalAnnotationOverride(ctx, tr, logger) + } + + if sc.VerifyAlwaysReturns != nil { + if *sc.VerifyAlwaysReturns { + return nil + } + return errors.New("failed to verify from mock VerifyAlwaysReturns") + } + + if !sc.CheckSpireVerifiedFlag(tr) { + return errors.New("annotation tekton.dev/not-verified = yes failed spire verification") + } + + annotations := tr.Status.Annotations + + // Verify annotations are there + if annotations[controllerSvidAnnotation] != controllerSvid { + return errors.New("svid annotation missing") + } + + // Check signature + currentHash, err := hashTaskrunStatusInternal(tr) + if err != nil { + return err + } + if !sc.mockVerify(currentHash, annotations[taskRunStatusHashSigAnnotation], "controller") { + return errors.New("signature was not able to be verified") + } + + // check current status hash vs annotation status hash by controller + if err := CheckStatusInternalAnnotation(tr); err != nil { + return err + } + + return nil +} + // VerifyTaskRunResults checks that all the TaskRun results are valid by the mocked spire client func (sc *MockClient) VerifyTaskRunResults(ctx context.Context, prs []v1beta1.PipelineResourceResult, tr *v1beta1.TaskRun) error { if sc.VerifyTaskRunResultsOverride != nil { diff --git a/pkg/spire/spire_mock_test.go b/pkg/spire/spire_mock_test.go index 86cd272c33c..aec909beb45 100644 --- a/pkg/spire/spire_mock_test.go +++ b/pkg/spire/spire_mock_test.go @@ -31,6 +31,216 @@ import ( duckv1beta1 "knative.dev/pkg/apis/duck/v1beta1" ) +// Simple task run sign/verify +func TestSpireMock_TaskRunSign(t *testing.T) { + spireMockClient := &MockClient{} + var ( + cc ControllerAPIClient = spireMockClient + ) + + ctx := context.Background() + var err error + + for _, tr := range testTaskRuns() { + err = cc.AppendStatusInternalAnnotation(ctx, tr) + if err != nil { + t.Fatalf("failed to sign TaskRun: %v", err) + } + + err = cc.VerifyStatusInternalAnnotation(ctx, tr, nil) + if err != nil { + t.Fatalf("failed to verify TaskRun: %v", err) + } + } +} + +// test CheckSpireVerifiedFlag(tr *v1beta1.TaskRun) bool +func TestSpireMock_CheckSpireVerifiedFlag(t *testing.T) { + spireMockClient := &MockClient{} + var ( + cc ControllerAPIClient = spireMockClient + ) + + trs := testTaskRuns() + tr := trs[0] + + if !cc.CheckSpireVerifiedFlag(tr) { + t.Fatalf("verified flag should be unset") + } + + if tr.Status.Status.Annotations == nil { + tr.Status.Status.Annotations = map[string]string{} + } + tr.Status.Status.Annotations[NotVerifiedAnnotation] = "yes" + + if cc.CheckSpireVerifiedFlag(tr) { + t.Fatalf("verified flag should be unset") + } +} + +// Task run check signed status is not the same with two taskruns +func TestSpireMock_CheckHashSimilarities(t *testing.T) { + spireMockClient := &MockClient{} + var ( + cc ControllerAPIClient = spireMockClient + ) + + ctx := context.Background() + trs := testTaskRuns() + tr1, tr2 := trs[0], trs[1] + + trs = testTaskRuns() + tr1c, tr2c := trs[0], trs[1] + + tr2c.Status.Status.Annotations = map[string]string{"new": "value"} + + signTrs := []*v1beta1.TaskRun{tr1, tr1c, tr2, tr2c} + + for _, tr := range signTrs { + err := cc.AppendStatusInternalAnnotation(ctx, tr) + if err != nil { + t.Fatalf("failed to sign TaskRun: %v", err) + } + } + + if getHash(tr1) != getHash(tr1c) { + t.Fatalf("2 hashes of the same status should be same") + } + + if getHash(tr1) == getHash(tr2) { + t.Fatalf("2 hashes of different status should not be the same") + } + + if getHash(tr2) != getHash(tr2c) { + t.Fatalf("2 hashes of the same status should be same (ignoring Status.Status)") + } +} + +// Task run sign, modify signature/hash/svid/content and verify +func TestSpireMock_CheckTamper(t *testing.T) { + + tests := []struct { + // description of test case + desc string + // annotations to set + setAnnotations map[string]string + // modify the status + modifyStatus bool + // modify the hash to match the new status but not the signature + modifyHashToMatch bool + // if test should pass + verify bool + }{ + { + desc: "tamper nothing", + verify: true, + }, + { + desc: "tamper unrelated hash", + setAnnotations: map[string]string{ + "unrelated-hash": "change", + }, + verify: true, + }, + { + desc: "tamper status hash", + setAnnotations: map[string]string{ + TaskRunStatusHashAnnotation: "change-hash", + }, + verify: false, + }, + { + desc: "tamper sig", + setAnnotations: map[string]string{ + taskRunStatusHashSigAnnotation: "change-sig", + }, + verify: false, + }, + { + desc: "tamper SVID", + setAnnotations: map[string]string{ + controllerSvidAnnotation: "change-svid", + }, + verify: false, + }, + { + desc: "delete status hash", + setAnnotations: map[string]string{ + TaskRunStatusHashAnnotation: "", + }, + verify: false, + }, + { + desc: "delete sig", + setAnnotations: map[string]string{ + taskRunStatusHashSigAnnotation: "", + }, + verify: false, + }, + { + desc: "delete SVID", + setAnnotations: map[string]string{ + controllerSvidAnnotation: "", + }, + verify: false, + }, + { + desc: "tamper status", + modifyStatus: true, + verify: false, + }, + { + desc: "tamper status and status hash", + modifyStatus: true, + modifyHashToMatch: true, + verify: false, + }, + } + for _, tt := range tests { + spireMockClient := &MockClient{} + var ( + cc ControllerAPIClient = spireMockClient + ) + + ctx := context.Background() + for _, tr := range testTaskRuns() { + err := cc.AppendStatusInternalAnnotation(ctx, tr) + if err != nil { + t.Fatalf("failed to sign TaskRun: %v", err) + } + + if tr.Status.Status.Annotations == nil { + tr.Status.Status.Annotations = map[string]string{} + } + + if tt.setAnnotations != nil { + for k, v := range tt.setAnnotations { + tr.Status.Status.Annotations[k] = v + } + } + + if tt.modifyStatus { + tr.Status.TaskRunStatusFields.Steps = append(tr.Status.TaskRunStatusFields.Steps, v1beta1.StepState{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ExitCode: int32(54321)}, + }}) + } + + if tt.modifyHashToMatch { + h, _ := hashTaskrunStatusInternal(tr) + tr.Status.Status.Annotations[TaskRunStatusHashAnnotation] = h + } + + verified := cc.VerifyStatusInternalAnnotation(ctx, tr, nil) == nil + if verified != tt.verify { + t.Fatalf("test %v expected verify %v, got %v", tt.desc, tt.verify, verified) + } + } + + } + +} + // Task result sign and verify func TestSpireMock_TaskRunResultsSign(t *testing.T) { spireMockClient := &MockClient{} @@ -480,6 +690,10 @@ func testPipelineResourceResults() [][]v1beta1.PipelineResourceResult { } } +func getHash(tr *v1beta1.TaskRun) string { + return tr.Status.Status.Annotations[TaskRunStatusHashAnnotation] +} + func genPodObj(tr *v1beta1.TaskRun, uid string) *corev1.Pod { if uid == "" { uid = uuid.NewString() diff --git a/pkg/spire/verify.go b/pkg/spire/verify.go index 7a9fd02614b..4c78b3e29e4 100644 --- a/pkg/spire/verify.go +++ b/pkg/spire/verify.go @@ -25,11 +25,13 @@ import ( "crypto/sha256" "crypto/x509" "encoding/base64" + "encoding/json" "encoding/pem" "fmt" "strings" "github.com/pkg/errors" + "go.uber.org/zap" "github.com/spiffe/go-spiffe/v2/workloadapi" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" @@ -86,6 +88,92 @@ func (sc *spireControllerAPIClient) VerifyTaskRunResults(ctx context.Context, pr return nil } +// VerifyStatusInternalAnnotation run multuple verification steps to ensure that the spire status annotations are valid +func (sc *spireControllerAPIClient) VerifyStatusInternalAnnotation(ctx context.Context, tr *v1beta1.TaskRun, logger *zap.SugaredLogger) error { + err := sc.checkClient(ctx) + if err != nil { + return err + } + + if !sc.CheckSpireVerifiedFlag(tr) { + return errors.New("annotation tekton.dev/not-verified = yes failed spire verification") + } + + annotations := tr.Status.Annotations + + // get trust bundle from spire server + trust, err := getTrustBundle(ctx, sc.workloadAPI) + if err != nil { + return err + } + + // verify controller SVID + svid, ok := annotations[controllerSvidAnnotation] + if !ok { + return errors.New("No SVID found") + } + block, _ := pem.Decode([]byte(svid)) + cert, err := x509.ParseCertificate(block.Bytes) + if err != nil { + return fmt.Errorf("invalid SVID: %w", err) + } + + // verify certificate root of trust + if err := verifyCertificateTrust(cert, trust); err != nil { + return err + } + logger.Infof("Successfully verified certificate %s against SPIRE", svid) + + if err := verifyAnnotation(cert.PublicKey, annotations); err != nil { + return err + } + logger.Info("Successfully verified signature") + + // CheckStatusInternalAnnotation check current status hash vs annotation status hash by controller + if err := CheckStatusInternalAnnotation(tr); err != nil { + return err + } + logger.Info("Successfully verified status annotation hash matches the current taskrun status") + + return nil +} + +// CheckSpireVerifiedFlag checks if the not-verified status annotation is set which would result in spire verification failed +func (sc *spireControllerAPIClient) CheckSpireVerifiedFlag(tr *v1beta1.TaskRun) bool { + if _, notVerified := tr.Status.Annotations[NotVerifiedAnnotation]; !notVerified { + return true + } + return false +} + +func hashTaskrunStatusInternal(tr *v1beta1.TaskRun) (string, error) { + s, err := json.Marshal(tr.Status.TaskRunStatusFields) + if err != nil { + return "", err + } + return fmt.Sprintf("%x", sha256.Sum256(s)), nil +} + +// CheckStatusInternalAnnotation ensures that the internal status annotation hash and current status hash match +func CheckStatusInternalAnnotation(tr *v1beta1.TaskRun) error { + // get stored hash of status + annotations := tr.Status.Annotations + hash, ok := annotations[TaskRunStatusHashAnnotation] + if !ok { + return fmt.Errorf("no annotation status hash found for %s", TaskRunStatusHashAnnotation) + } + // get current hash of status + current, err := hashTaskrunStatusInternal(tr) + if err != nil { + return err + } + if hash != current { + return fmt.Errorf("current status hash and stored annotation hash does not match! Annotation Hash: %s, Current Status Hash: %s", hash, current) + } + + return nil +} + func getSVID(resultMap map[string]v1beta1.PipelineResourceResult) (*x509.Certificate, error) { svid, ok := resultMap[KeySVID] if !ok { @@ -162,6 +250,18 @@ func verifyManifest(results map[string]v1beta1.PipelineResourceResult) error { return nil } +func verifyAnnotation(pub interface{}, annotations map[string]string) error { + signature, ok := annotations[taskRunStatusHashSigAnnotation] + if !ok { + return fmt.Errorf("no signature found for %s", taskRunStatusHashSigAnnotation) + } + hash, ok := annotations[TaskRunStatusHashAnnotation] + if !ok { + return fmt.Errorf("no annotation status hash found for %s", TaskRunStatusHashAnnotation) + } + return verifySignature(pub, signature, hash) +} + func verifyResult(pub interface{}, key string, results map[string]v1beta1.PipelineResourceResult) error { signature, ok := results[key+KeySignatureSuffix] if !ok { diff --git a/test/embed_test.go b/test/embed_test.go index ddbfad967a0..7e346855954 100644 --- a/test/embed_test.go +++ b/test/embed_test.go @@ -67,6 +67,11 @@ func embeddedResourceTest(t *testing.T, spireEnabled bool) { defer resetConfigMap(ctx, t, c, systemNamespace, config.GetFeatureFlagsConfigName(), originalConfigMapData) } + if spireEnabled { + originalConfigMapData := enableSpireConfigMap(ctx, c, t) + defer resetConfigMap(ctx, t, c, systemNamespace, config.GetFeatureFlagsConfigName(), originalConfigMapData) + } + t.Logf("Creating Task and TaskRun in namespace %s", namespace) if _, err := c.TaskClient.Create(ctx, getEmbeddedTask(t, embedTaskName, namespace, []string{"/bin/sh", "-c", fmt.Sprintf("echo %s", taskOutput)}), metav1.CreateOptions{}); err != nil { t.Fatalf("Failed to create Task `%s`: %s", embedTaskName, err) @@ -89,6 +94,7 @@ func embeddedResourceTest(t *testing.T, spireEnabled bool) { t.Errorf("Error retrieving taskrun: %s", err) } spireShouldPassTaskRunResultsVerify(tr, t) + spireShouldPassSpireAnnotation(tr, t) } } diff --git a/test/entrypoint_test.go b/test/entrypoint_test.go index 534994717e9..336681cc150 100644 --- a/test/entrypoint_test.go +++ b/test/entrypoint_test.go @@ -63,6 +63,11 @@ func entryPointerTest(t *testing.T, spireEnabled bool) { defer resetConfigMap(ctx, t, c, systemNamespace, config.GetFeatureFlagsConfigName(), originalConfigMapData) } + if spireEnabled { + originalConfigMapData := enableSpireConfigMap(ctx, c, t) + defer resetConfigMap(ctx, t, c, systemNamespace, config.GetFeatureFlagsConfigName(), originalConfigMapData) + } + t.Logf("Creating TaskRun in namespace %s", namespace) if _, err := c.TaskRunClient.Create(ctx, parse.MustParseTaskRun(t, fmt.Sprintf(` metadata: @@ -92,6 +97,7 @@ spec: t.Errorf("Error retrieving taskrun: %s", err) } spireShouldPassTaskRunResultsVerify(tr, t) + spireShouldPassSpireAnnotation(tr, t) } } diff --git a/test/helm_task_test.go b/test/helm_task_test.go index ab912f77494..275ccb21a1a 100644 --- a/test/helm_task_test.go +++ b/test/helm_task_test.go @@ -126,6 +126,7 @@ func helmDeploytest(t *testing.T, spireEnabled bool) { } for _, taskrunItem := range taskrunList.Items { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } } diff --git a/test/hermetic_taskrun_test.go b/test/hermetic_taskrun_test.go index 7c419adb40b..371f07c256e 100644 --- a/test/hermetic_taskrun_test.go +++ b/test/hermetic_taskrun_test.go @@ -89,6 +89,7 @@ func hermeticTest(t *testing.T, spireEnabled bool) { t.Errorf("Error retrieving taskrun: %s", err) } spireShouldPassTaskRunResultsVerify(tr, t) + spireShouldPassSpireAnnotation(tr, t) } // now, run the task mode with hermetic mode @@ -108,6 +109,7 @@ func hermeticTest(t *testing.T, spireEnabled bool) { t.Errorf("Error retrieving taskrun: %s", err) } spireShouldFailTaskRunResultsVerify(tr, t) + spireShouldPassSpireAnnotation(tr, t) } }) } diff --git a/test/ignore_step_error_test.go b/test/ignore_step_error_test.go index e3fb6f07131..9bd174aa641 100644 --- a/test/ignore_step_error_test.go +++ b/test/ignore_step_error_test.go @@ -115,6 +115,7 @@ spec: if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } for _, r := range taskrunItem.Status.TaskRunResults { diff --git a/test/init_test.go b/test/init_test.go index d7c3993d240..02ed9e8e2af 100644 --- a/test/init_test.go +++ b/test/init_test.go @@ -33,6 +33,7 @@ import ( "github.com/tektoncd/pipeline/pkg/apis/config" "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1" "github.com/tektoncd/pipeline/pkg/names" + "github.com/tektoncd/pipeline/pkg/spire" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -319,3 +320,26 @@ func spireShouldPassTaskRunResultsVerify(tr *v1beta1.TaskRun, t *testing.T) { t.Errorf("Taskrun `%s` status condition not verified. Spire taskrun results verification failure", tr.Name) } } + +// Verifies if the taskrun status annotation does not contain "not-verified" +func spireShouldPassSpireAnnotation(tr *v1beta1.TaskRun, t *testing.T) { + if _, notVerified := tr.Status.Annotations[spire.NotVerifiedAnnotation]; notVerified { + t.Errorf("Taskrun `%s` status not verified. Annotation tekton.dev/not-verified = yes. Failed spire verification", tr.Name) + } +} + +// Verifies if the taskrun status annotation does contain "not-verified" +func spireShouldFailSpireAnnotation(tr *v1beta1.TaskRun, t *testing.T) { + _, notVerified := tr.Status.Annotations[spire.NotVerifiedAnnotation] + _, hash := tr.Status.Annotations[spire.TaskRunStatusHashAnnotation] + if !notVerified && hash { + t.Errorf("Taskrun `%s` status should be not verified missing Annotation tekton.dev/not-verified = yes", tr.Name) + } +} + +// Verifies the taskrun status annotation should not exist +func spireZeroSpireAnnotation(tr *v1beta1.TaskRun, t *testing.T) { + if len(tr.Status.Annotations) > 0 { + t.Errorf("Taskrun `%s` status should not have any spire annotations", tr.Name) + } +} diff --git a/test/kaniko_task_test.go b/test/kaniko_task_test.go index c0fdf29fe61..6a7dae7b08d 100644 --- a/test/kaniko_task_test.go +++ b/test/kaniko_task_test.go @@ -140,6 +140,7 @@ func kanikoTest(t *testing.T, spireEnabled bool) { if spireEnabled { spireShouldPassTaskRunResultsVerify(tr, t) + spireShouldPassSpireAnnotation(tr, t) } // match the local digest, which is first capture group against the remote image diff --git a/test/pipelinefinally_test.go b/test/pipelinefinally_test.go index 1a402444539..b0b73e0f35f 100644 --- a/test/pipelinefinally_test.go +++ b/test/pipelinefinally_test.go @@ -283,6 +283,7 @@ spec: } if spireEnabled { spireShouldFailTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } dagTask1EndTime = taskrunItem.Status.CompletionTime case n == "dagtask2": @@ -291,6 +292,7 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } dagTask2EndTime = taskrunItem.Status.CompletionTime case n == "dagtask3": @@ -300,11 +302,13 @@ spec: if spireEnabled { // Skipped due to condition so status annotations should be there. Results should not be verified as not run spireShouldFailTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } case n == "dagtask4": if spireEnabled { // Skipped so status annotations should not be there. Results should not be verified as not run spireShouldFailTaskRunResultsVerify(&taskrunItem, t) + spireZeroSpireAnnotation(&taskrunItem, t) } t.Fatalf("task %s should have skipped due to when expression", n) case n == "dagtask5": @@ -313,6 +317,7 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } case n == "finaltask1": if err := WaitForTaskRunState(ctx, c, taskrunItem.Name, TaskRunSucceed(taskrunItem.Name), "TaskRunSuccess"); err != nil { @@ -320,6 +325,7 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } finalTaskStartTime = taskrunItem.Status.StartTime case n == "finaltask2": @@ -328,6 +334,7 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } for _, p := range taskrunItem.Spec.Params { switch param := p.Name; param { @@ -356,6 +363,7 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } for _, p := range taskrunItem.Spec.Params { if p.Name == "dagtask-result" && p.Value.StringVal != "Hello" { @@ -368,6 +376,7 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } case n == "guardedfinaltaskusingdagtask5status1": if !isSuccessful(t, n, taskrunItem.Status.Conditions) { @@ -375,17 +384,20 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } case n == "guardedfinaltaskusingdagtask5result2": if spireEnabled { // Skipped so status annotations should not be there. Results should not be verified as not run spireShouldFailTaskRunResultsVerify(&taskrunItem, t) + spireZeroSpireAnnotation(&taskrunItem, t) } t.Fatalf("final task %s should have skipped due to when expression evaluating to false", n) case n == "finaltaskconsumingdagtask1" || n == "finaltaskconsumingdagtask4" || n == "guardedfinaltaskconsumingdagtask4": if spireEnabled { // Skipped so status annotations should not be there. Results should not be verified as not run spireShouldFailTaskRunResultsVerify(&taskrunItem, t) + spireZeroSpireAnnotation(&taskrunItem, t) } t.Fatalf("final task %s should have skipped due to missing task result reference", n) default: @@ -519,6 +531,7 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } case n == "finaltask1": if !isFailed(t, n, taskrunItem.Status.Conditions) { @@ -526,6 +539,7 @@ spec: } if spireEnabled { spireShouldFailTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } default: t.Fatalf("TaskRuns were not found for both final and dag tasks") @@ -649,10 +663,12 @@ spec: } if spireEnabled { spireShouldFailTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } case "dagtask2": if spireEnabled { spireShouldFailTaskRunResultsVerify(&taskrunItem, t) + spireZeroSpireAnnotation(&taskrunItem, t) } t.Fatalf("second dag task %s should be skipped as it depends on the result from cancelled 'dagtask1'", n) case "finaltask1": @@ -661,10 +677,12 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } case "finaltask2": if spireEnabled { spireShouldFailTaskRunResultsVerify(&taskrunItem, t) + spireZeroSpireAnnotation(&taskrunItem, t) } t.Fatalf("second final task %s should be skipped as it depends on the result from cancelled 'dagtask1'", n) default: @@ -789,6 +807,7 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } case "finaltask1": if !isSuccessful(t, n, taskrunItem.Status.Conditions) { @@ -796,6 +815,7 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } case "finaltask2": if !isSuccessful(t, n, taskrunItem.Status.Conditions) { @@ -803,6 +823,7 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } default: t.Fatalf("TaskRuns were not found for both final and dag tasks") diff --git a/test/pipelinerun_test.go b/test/pipelinerun_test.go index 597da8ee9f0..fd7d513bc53 100644 --- a/test/pipelinerun_test.go +++ b/test/pipelinerun_test.go @@ -273,6 +273,7 @@ spec: } if spireEnabled { spireShouldPassTaskRunResultsVerify(&actualTaskRunItem, t) + spireShouldPassSpireAnnotation(&actualTaskRunItem, t) } } expectedTaskRunNames = append(expectedTaskRunNames, taskRunName) @@ -445,6 +446,7 @@ spec: } for _, taskrunItem := range taskrunList.Items { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } } @@ -557,6 +559,7 @@ spec: } for _, taskrunItem := range taskrunList.Items { spireShouldPassTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } } } diff --git a/test/status_test.go b/test/status_test.go index 623a51f005a..2b6411a55cd 100644 --- a/test/status_test.go +++ b/test/status_test.go @@ -94,6 +94,7 @@ spec: t.Errorf("Error retrieving taskrun: %s", err) } spireShouldFailTaskRunResultsVerify(tr, t) + spireShouldPassSpireAnnotation(tr, t) } pipeline := parse.MustParsePipeline(t, fmt.Sprintf(` @@ -130,6 +131,7 @@ spec: } for _, taskrunItem := range taskrunList.Items { spireShouldFailTaskRunResultsVerify(&taskrunItem, t) + spireShouldPassSpireAnnotation(&taskrunItem, t) } } diff --git a/test/taskrun_test.go b/test/taskrun_test.go index ce45d520d70..a75e2b0707b 100644 --- a/test/taskrun_test.go +++ b/test/taskrun_test.go @@ -111,6 +111,7 @@ spec: if spireEnabled { spireShouldFailTaskRunResultsVerify(taskrun, t) + spireShouldPassSpireAnnotation(taskrun, t) } expectedStepState := []v1beta1.StepState{{ @@ -218,6 +219,7 @@ spec: if spireEnabled { spireShouldPassTaskRunResultsVerify(taskrun, t) + spireShouldPassSpireAnnotation(taskrun, t) } expectedStepState := []v1beta1.StepState{{ @@ -333,6 +335,7 @@ spec: if spireEnabled { spireShouldFailTaskRunResultsVerify(taskrun, t) + spireShouldFailSpireAnnotation(taskrun, t) } expectedStepState := []v1beta1.StepState{{