Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TEP-0091] Add Verification at reconciler #5581

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions config/config-feature-flags.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -81,3 +81,7 @@ data:
# Setting this flag to "true" enables CloudEvents for Runs, as long as a
# CloudEvents sink is configured in the config-defaults config map
send-cloudevents-for-runs: "false"
# Setting this flag to "enforce" will enforce verification of tasks/pipeline. Failing to verify
# will fail the taskrun/pipelinerun. "warn" will only log the err message and "skip"
# will skip the whole verification
resource-verification-mode: "skip"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might make sense to add some docs to the install instructions as well on the new config option(s) - maybe linking to the separate doc you've written?

Copy link
Member Author

@Yongxuanzhang Yongxuanzhang Oct 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure!! added

41 changes: 41 additions & 0 deletions config/config-trusted-resources.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
# Copyright 2022 The Tekton Authors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# https://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

apiVersion: v1
kind: ConfigMap
metadata:
name: config-trusted-resources
namespace: tekton-pipelines
labels:
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-pipelines
data:
_example: |
################################
# #
# EXAMPLE CONFIGURATION #
# #
################################
# This block is not actually functional configuration,
# but serves to illustrate the available configuration
# options and document them in a way that is accessible
# to users that `kubectl edit` this config map.
#
# These sample configuration options may be copied out of
# this example block and unindented to be in the data block
# to actually change the configuration.

# publickeys specifies the list of public keys, the paths are separated by comma
# publickeys: "/etc/verification-secrets/cosign.pub,
# gcpkms://projects/tekton/locations/us/keyRings/trusted-resources/cryptoKeys/trusted-resources"
11 changes: 11 additions & 0 deletions config/controller.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,10 @@ spec:
mountPath: /etc/config-logging
- name: config-registry-cert
mountPath: /etc/config-registry-cert
# Mount secret for trusted resources
Yongxuanzhang marked this conversation as resolved.
Show resolved Hide resolved
- name: verification-secrets
mountPath: /etc/verification-secrets
readOnly: true
env:
- name: SYSTEM_NAMESPACE
valueFrom:
Expand All @@ -112,6 +116,8 @@ spec:
value: feature-flags
- name: CONFIG_LEADERELECTION_NAME
value: config-leader-election
- name: CONFIG_TRUSTED_RESOURCES_NAME
value: config-trusted-resources
- name: SSL_CERT_FILE
value: /etc/config-registry-cert/cert
- name: SSL_CERT_DIR
Expand Down Expand Up @@ -159,6 +165,11 @@ spec:
- name: config-registry-cert
configMap:
name: config-registry-cert
# Mount secret for trusted resources
- name: verification-secrets
secret:
secretName: verification-secrets
optional: true
---
apiVersion: v1
kind: Service
Expand Down
1 change: 1 addition & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ See the following topics to learn how to use Tekton Pipelines in your project:
- [Variable Substitutions](tasks.md#using-variable-substitution)
- [Running a Custom Task (alpha)](runs.md)
- [Remote resolution of Pipelines and Tasks](resolution.md)
- [Trusted Resources](trusted-resources.md)

## Contributing to Tekton Pipelines

Expand Down
3 changes: 3 additions & 0 deletions docs/install.md
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,8 @@ features](#alpha-features) to be used.
name, kind, and API version information for each `TaskRun` and `Run` in the `PipelineRun` instead. Set it to "both" to
do both. For more information, see [Configuring usage of `TaskRun` and `Run` embedded statuses](pipelineruns.md#configuring-usage-of-taskrun-and-run-embedded-statuses).

- `resource-verification-mode`: Setting this flag to "enforce" will enforce verification of tasks/pipeline. Failing to verify will fail the taskrun/pipelinerun. "warn" will only log the err message and "skip" will skip the whole verification.

For example:

```yaml
Expand Down Expand Up @@ -457,6 +459,7 @@ Features currently in "alpha" are:
| [Task-level Resource Requirements](compute-resources.md#task-level-compute-resources-configuration) | [TEP-0104](https://github.com/tektoncd/community/blob/main/teps/0104-tasklevel-resource-requirements.md) | [v0.39.0](https://github.com/tektoncd/pipeline/releases/tag/v0.39.0) | |
| [Object Params and Results](pipelineruns.md#specifying-parameters) | [TEP-0075](https://github.com/tektoncd/community/blob/main/teps/0075-object-param-and-result-types.md) | [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0) | |
| [Array Results](pipelineruns.md#specifying-parameters) | [TEP-0076](https://github.com/tektoncd/community/blob/main/teps/0076-array-result-types.md) | [v0.38.0](https://github.com/tektoncd/pipeline/releases/tag/v0.38.0) | |
| [Trusted Resources](./trusted-resources.md) | [TEP-0091](https://github.com/tektoncd/community/blob/main/teps/0091-trusted-resources.md) | N/A | resource-verification-mode |

### Beta Features

Expand Down
93 changes: 93 additions & 0 deletions docs/trusted-resources.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
# Trusted Resources

- [Overview](#overview)
- [Instructions](#Instructions)
- [Sign Resources](#sign-resources)
- [Enable Trusted Resources](#enable-trusted-resources)

## Overview

Trusted Resources is a feature which can be used to sign Tekton Resources and verify them. Details of design can be found at [TEP--0091](https://github.com/tektoncd/community/blob/main/teps/0091-trusted-resources.md). This feature is under `alpha` version and support `v1beta1` version of `Task` and `Pipeline`.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add some details around what behavior is expected if a resource fails verification? The PR also mentioned that there are some details around what is supported and what isn't, e.g. KMS isn't supported, only PEM files - can you explain about that in this docs as well?

Copy link
Member Author

@Yongxuanzhang Yongxuanzhang Oct 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! The expected behaviour is mentioned in the feature flag section. I will also update in the overview section.

Verification failure will mark corresponding taskrun/pipelinerun as Failed status and stop the execution.

**Note:** KMS is not currently supported and will be supported in the following work.


## Instructions

### Sign Resources
For `Sign` cli you may refer to [experimental repo](https://github.com/tektoncd/experimental/tree/main/pipeline/trusted-resources) to sign the resources. We're working to add `sign` and `verify` into [Tekton Cli](https://github.com/tektoncd/cli) as a subcommand.

A signed task example:
```yaml
apiVersion: tekton.dev/v1beta1
kind: Task
metadata:
annotations:
tekton.dev/signature: MEYCIQDM8WHQAn/yKJ6psTsa0BMjbI9IdguR+Zi6sPTVynxv6wIhAMy8JSETHP7A2Ncw7MyA7qp9eLsu/1cCKOjRL1mFXIKV
creationTimestamp: null
name: example-task
namespace: tekton-trusted-resources
spec:
steps:
- image: ubuntu
name: echo
```

### Enable Trusted Resources

#### Enable feature flag

Update the config map:
```yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: feature-flags
namespace: tekton-pipelines
labels:
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-pipelines
data:
resource-verification-mode: "enforce"
```

**Note:** `resource-verification-mode` needs to be set as `enforce` or `warn` to enable resource verification.

`resource-verification-mode` configurations:
* `enforce`: Failing verification will mark the taskruns/pipelineruns as failed.
* `warn`: Log warning but don't fail the taskruns/pipelineruns.
* `skip`: Directly skip the verification.

Or patch the new values:
```bash
kubectl patch configmap feature-flags -n tekton-pipelines -p='{"data":{"resource-verification-mode":"enforce"}}
```

#### Config key at configmap
Note that multiple keys reference should be separated by comma. If the resource can pass any key in the list, it will pass the verification.

We currently hardcode SHA256 as hashfunc for loading public keys as verifiers.

Public key files should be added into secret and mounted into controller volumes. To add keys into secret you may execute:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to volume mount the secrets, or should we just reference them via the sigstore k8s://namespace/name syntax? https://docs.sigstore.dev/cosign/kubernetes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting we use cosign generate-key-pair k8s://default/testsecret in docs? It's another way to save keys in secret, but we still need to mount this secret to controller container right? Like what we do in Chains:
https://github.com/tektoncd/chains/blob/780521656ab0ea2a72d7a969a13040848ac6b969/config/100-deployment.yaml#L91-L92

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking more specifying these values in the config map - publickeys: k8s://namespace/name, vault://key, gcpkms://key. This way users don't need to redeploy Tekton each time they want to add a key.

I'm fine with punting on this as a TODO if you want, wanted to ask what the long term plan is since there's a few places where we assume local keys and my guess is that probably won't be the case forever.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, that seems better!
If we use publickeys: k8s://namespace/name, do we need to fetch secret to get the key files?
In the long term I think we will still keep local keys as an option for users

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add a todo for this

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Yongxuanzhang is the updated plan for this reflected in the TEP?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Yongxuanzhang is the updated plan for this reflected in the TEP?

Not yet, I will change this in the tep pr


```shell
kubectl create secret generic verification-secrets \
--from-file=cosign.pub=./cosign.pub \
--from-file=cosign.pub=./cosign2.pub \
-n tekton-pipelines
Yongxuanzhang marked this conversation as resolved.
Show resolved Hide resolved
```

```yaml
apiVersion: v1
kind: ConfigMap
metadata:
name: config-trusted-resources
namespace: tekton-pipelines
labels:
app.kubernetes.io/instance: default
app.kubernetes.io/part-of: tekton-pipelines
data:
publickeys: "/etc/verification-secrets/cosign.pub, /etc/verification-secrets/cosign2.pub"
```
31 changes: 31 additions & 0 deletions pkg/apis/config/feature_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ const (
// MinimalEmbeddedStatus is the value used for "embedded-status" when only ChildReferences should be used in
// PipelineRunStatusFields.
MinimalEmbeddedStatus = "minimal"
// EnforceResourceVerificationMode is the value used for "resource-verification-mode" when verification is applied and fail the
// TaskRun or PipelineRun when verification fails
EnforceResourceVerificationMode = "enforce"
// WarnResourceVerificationMode is the value used for "resource-verification-mode" when verification is applied but only log
// the warning when verification fails
WarnResourceVerificationMode = "warn"
// SkipResourceVerificationMode is the value used for "resource-verification-mode" when verification is skipped
SkipResourceVerificationMode = "skip"
// DefaultDisableAffinityAssistant is the default value for "disable-affinity-assistant".
DefaultDisableAffinityAssistant = false
// DefaultDisableCredsInit is the default value for "disable-creds-init".
Expand All @@ -64,6 +72,8 @@ const (
DefaultEmbeddedStatus = FullEmbeddedStatus
// DefaultEnableSpire is the default value for "enable-spire".
DefaultEnableSpire = false
// DefaultResourceVerificationMode is the default value for "resource-verification-mode".
DefaultResourceVerificationMode = SkipResourceVerificationMode

disableAffinityAssistantKey = "disable-affinity-assistant"
disableCredsInitKey = "disable-creds-init"
Expand All @@ -76,6 +86,7 @@ const (
sendCloudEventsForRuns = "send-cloudevents-for-runs"
embeddedStatus = "embedded-status"
enableSpire = "enable-spire"
verificationMode = "resource-verification-mode"
)

// FeatureFlags holds the features configurations
Expand All @@ -93,6 +104,7 @@ type FeatureFlags struct {
AwaitSidecarReadiness bool
EmbeddedStatus string
EnableSpire bool
ResourceVerificationMode string
}

// GetFeatureFlagsConfigName returns the name of the configmap containing all
Expand Down Expand Up @@ -144,6 +156,9 @@ func NewFeatureFlagsFromMap(cfgMap map[string]string) (*FeatureFlags, error) {
if err := setEmbeddedStatus(cfgMap, DefaultEmbeddedStatus, &tc.EmbeddedStatus); err != nil {
return nil, err
}
if err := setResourceVerificationMode(cfgMap, DefaultResourceVerificationMode, &tc.ResourceVerificationMode); err != nil {
return nil, err
}

// Given that they are alpha features, Tekton Bundles and Custom Tasks should be switched on if
// enable-api-fields is "alpha". If enable-api-fields is not "alpha" then fall back to the value of
Expand Down Expand Up @@ -201,6 +216,22 @@ func setEmbeddedStatus(cfgMap map[string]string, defaultValue string, feature *s
return nil
}

// setResourceVerificationMode sets the "resource-verification-mode" flag based on the content of a given map.
// If the value is invalid or missing then an error is returned.
func setResourceVerificationMode(cfgMap map[string]string, defaultValue string, feature *string) error {
value := defaultValue
if cfg, ok := cfgMap[verificationMode]; ok {
value = strings.ToLower(cfg)
}
switch value {
case EnforceResourceVerificationMode, WarnResourceVerificationMode, SkipResourceVerificationMode:
*feature = value
default:
return fmt.Errorf("invalid value for feature flag %q: %q", verificationMode, value)
}
return nil
}

// NewFeatureFlagsFromConfigMap returns a Config for the given configmap
func NewFeatureFlagsFromConfigMap(config *corev1.ConfigMap) (*FeatureFlags, error) {
return NewFeatureFlagsFromMap(config.Data)
Expand Down
30 changes: 19 additions & 11 deletions pkg/apis/config/feature_flags_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
RunningInEnvWithInjectedSidecars: true,
RequireGitSSHSecretKnownHosts: false,

DisableCredsInit: config.DefaultDisableCredsInit,
AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness,
EnableTektonOCIBundles: config.DefaultEnableTektonOciBundles,
EnableCustomTasks: config.DefaultEnableCustomTasks,
EnableAPIFields: config.DefaultEnableAPIFields,
SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns,
EmbeddedStatus: config.DefaultEmbeddedStatus,
DisableCredsInit: config.DefaultDisableCredsInit,
AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness,
EnableTektonOCIBundles: config.DefaultEnableTektonOciBundles,
EnableCustomTasks: config.DefaultEnableCustomTasks,
EnableAPIFields: config.DefaultEnableAPIFields,
SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns,
EmbeddedStatus: config.DefaultEmbeddedStatus,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
},
fileName: config.GetFeatureFlagsConfigName(),
},
Expand All @@ -60,6 +61,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
SendCloudEventsForRuns: true,
EmbeddedStatus: "both",
EnableSpire: true,
ResourceVerificationMode: "enforce",
},
fileName: "feature-flags-all-flags-set",
},
Expand All @@ -79,6 +81,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
RequireGitSSHSecretKnownHosts: config.DefaultRequireGitSSHSecretKnownHosts,
SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns,
EmbeddedStatus: config.DefaultEmbeddedStatus,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
},
fileName: "feature-flags-enable-api-fields-overrides-bundles-and-custom-tasks",
},
Expand All @@ -95,6 +98,7 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
RequireGitSSHSecretKnownHosts: config.DefaultRequireGitSSHSecretKnownHosts,
SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns,
EmbeddedStatus: config.DefaultEmbeddedStatus,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
},
fileName: "feature-flags-bundles-and-custom-tasks",
},
Expand All @@ -111,15 +115,16 @@ func TestNewFeatureFlagsFromConfigMap(t *testing.T) {
RequireGitSSHSecretKnownHosts: config.DefaultRequireGitSSHSecretKnownHosts,
SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns,
EmbeddedStatus: config.DefaultEmbeddedStatus,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
},
fileName: "feature-flags-beta-api-fields",
},
{
expectedConfig: &config.FeatureFlags{
EnableAPIFields: "stable",
EmbeddedStatus: "full",
EnableSpire: true,

EnableAPIFields: "stable",
EmbeddedStatus: "full",
EnableSpire: true,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
RunningInEnvWithInjectedSidecars: config.DefaultRunningInEnvWithInjectedSidecars,
AwaitSidecarReadiness: config.DefaultAwaitSidecarReadiness,
},
Expand Down Expand Up @@ -150,6 +155,7 @@ func TestNewFeatureFlagsFromEmptyConfigMap(t *testing.T) {
SendCloudEventsForRuns: config.DefaultSendCloudEventsForRuns,
EmbeddedStatus: config.DefaultEmbeddedStatus,
EnableSpire: config.DefaultEnableSpire,
ResourceVerificationMode: config.DefaultResourceVerificationMode,
}
verifyConfigFileWithExpectedFeatureFlagsConfig(t, FeatureFlagsConfigEmptyName, expectedConfig)
}
Expand Down Expand Up @@ -190,6 +196,8 @@ func TestNewFeatureFlagsConfigMapErrors(t *testing.T) {
fileName: "feature-flags-invalid-enable-api-fields",
}, {
fileName: "feature-flags-invalid-embedded-status",
}, {
fileName: "feature-flags-invalid-resource-verification-mode",
}} {
t.Run(tc.fileName, func(t *testing.T) {
cm := test.ConfigMapFromTestFile(t, tc.fileName)
Expand Down
Loading