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

Cosign Signing Integration #817

Merged
merged 2 commits into from Sep 27, 2021
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
29 changes: 29 additions & 0 deletions api/openapi-spec/swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -5520,6 +5520,9 @@
"cache": {
"$ref": "#/definitions/kpack.build.v1alpha2.BuildCacheConfig"
},
"cosign": {
"$ref": "#/definitions/kpack.build.v1alpha2.CosignConfig"
},
"defaultProcess": {
"type": "string"
},
Expand Down Expand Up @@ -6059,6 +6062,29 @@
}
}
},
"kpack.build.v1alpha2.CosignAnnotation": {
"type": "object",
"properties": {
"name": {
"type": "string"
},
"value": {
"type": "string"
}
}
},
"kpack.build.v1alpha2.CosignConfig": {
"type": "object",
"properties": {
"annotations": {
"type": "array",
"items": {
"$ref": "#/definitions/kpack.build.v1alpha2.CosignAnnotation"
},
"x-kubernetes-list-type": ""
}
}
},
"kpack.build.v1alpha2.Image": {
"type": "object",
"required": [
Expand Down Expand Up @@ -6215,6 +6241,9 @@
"cache": {
"$ref": "#/definitions/kpack.build.v1alpha2.ImageCacheConfig"
},
"cosign": {
"$ref": "#/definitions/kpack.build.v1alpha2.CosignConfig"
},
"defaultProcess": {
"type": "string"
},
Expand Down
106 changes: 86 additions & 20 deletions cmd/completion/main.go
Original file line number Diff line number Diff line change
@@ -1,28 +1,39 @@
package main

import (
"context"
"flag"
"log"
"os"
"path/filepath"
"strings"

"github.com/BurntSushi/toml"
"github.com/buildpacks/lifecycle"
"github.com/sigstore/cosign/cmd/cosign/cli"

"github.com/pivotal/kpack/pkg/cosign"
"github.com/pivotal/kpack/pkg/dockercreds"
"github.com/pivotal/kpack/pkg/flaghelpers"
"github.com/pivotal/kpack/pkg/notary"
"github.com/pivotal/kpack/pkg/registry"
)

const (
registrySecretsDir = "/var/build-secrets"
reportFilePath = "/var/report/report.toml"
notarySecretDir = "/var/notary/v1"
registrySecretsDir = "/var/build-secrets"
reportFilePath = "/var/report/report.toml"
notarySecretDir = "/var/notary/v1"
cosignSecretLocation = "/var/build-secrets/cosign"
)

var (
notaryV1URL string
dockerCredentials flaghelpers.CredentialsFlags
dockerCfgCredentials flaghelpers.CredentialsFlags
dockerConfigCredentials flaghelpers.CredentialsFlags
cosignAnnotations flaghelpers.CredentialsFlags
cosignRepositories flaghelpers.CredentialsFlags
cosignDockerMediaTypes flaghelpers.CredentialsFlags
logger *log.Logger
)

Expand All @@ -31,46 +42,101 @@ func init() {
flag.Var(&dockerCredentials, "basic-docker", "Basic authentication for docker of the form 'secretname=git.domain.com'")
flag.Var(&dockerCfgCredentials, "dockercfg", "Docker Cfg credentials in the form of the path to the credential")
flag.Var(&dockerConfigCredentials, "dockerconfig", "Docker Config JSON credentials in the form of the path to the credential")

flag.Var(&cosignAnnotations, "cosign-annotations", "Cosign custom signing annotations")
flag.Var(&cosignRepositories, "cosign-repositories", "Cosign signing repository of the form 'secretname=registry.example.com/project'")
flag.Var(&cosignDockerMediaTypes, "cosign-docker-media-types", "Cosign signing with legacy docker media types of the form 'secretname=1'")
logger = log.New(os.Stdout, "", 0)
}

func main() {
flag.Parse()

if notaryV1URL != "" {
creds, err := dockercreds.ParseMountedAnnotatedSecrets(registrySecretsDir, dockerCredentials)
var report lifecycle.ExportReport
_, err := toml.DecodeFile(reportFilePath, &report)
if err != nil {
logger.Fatalf("toml decode: %v", err)
}

creds, err := dockercreds.ParseMountedAnnotatedSecrets(registrySecretsDir, dockerCredentials)
if err != nil {
logger.Fatal(err)
}

for _, c := range append(dockerCfgCredentials, dockerConfigCredentials...) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I realize this was the existing behavior in completion but, it seems unnecessary to rebuild the keychain to just write to docker/config.json again. Can we just mount the same credentials from export into completion?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Did you mean that I remount the homeVolume and set the Env HOME to /builder/home?

I noticed that build-init would save the config to /builder/home/docker/config.json and could therefore be reused by completion during the regular buildPod flow. However, during the rebasePod flow, as the only initContainer is the "rebase" container. It does load some docker credentials but it does not save it. I could introduce the "Save" portion to the rebase image and therefore the completion container could leverage a shared volume for both buildPod and rebasePod scenarios. Is it ideal to introduce this logic into "rebase" when it itself doesn't need the file to be saved just so we can leverage the byproduct of that outside of that container in completion?

The keychain still needed to be built or loaded into a variable to be passed into Notary while cosign only requires the config to be writing to a file named config.json. The keychain could be built from the config.json though instead of from each secret individually if mounting a shared volume is the way to go

Copy link
Contributor Author

@DennyHoang DennyHoang Sep 8, 2021

Choose a reason for hiding this comment

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

@matthewmcnew If I'm understanding this wrong, please lemme know what the best approach is :D

Copy link
Collaborator

Choose a reason for hiding this comment

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

I forgot about the complexities introduced by the rebase container. I think it would eventually be a good idea for the rebase container to "save" the credentials to a volume that can be loaded in completion. I don't think you need to introduce this in your PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since that is the case, I will leave the current credential generation and saving inside the completion image. When we introduce and find a more uniform way of saving credentials, we can adjust the completion container then. For cosign portion in the completion container, with the shared volume during non rebase case, it seemed to work from my quick testing when mounting the shared home volume. For the notary portion, it is still likely that the keychain would need to be loaded to be passed into the notary sign function. Is there a place I can put stories or track this concern? Do I just make a Github issue saying to standardize docker credential saving using shared volumes to be more consistent between rebasePod/buildPod/initContainers/containers?

credPath := filepath.Join(registrySecretsDir, c)

dockerCfgCreds, err := dockercreds.ParseDockerPullSecrets(credPath)
if err != nil {
logger.Fatal(err)
}

for _, c := range append(dockerCfgCredentials, dockerConfigCredentials...) {
credPath := filepath.Join(registrySecretsDir, c)
for domain := range dockerCfgCreds {
logger.Printf("Loading secret for %q from secret %q at location %q", domain, c, credPath)
}

creds, err = creds.Append(dockerCfgCreds)
if err != nil {
logger.Fatal(err)
}

homeDir, err := os.UserHomeDir()
if err != nil {
logger.Fatalf("error obtaining home directory: %v", err)
}

err = creds.Save(filepath.Join(homeDir, ".docker", "config.json"))
if err != nil {
logger.Fatalf("error writing docker creds: %v", err)
}
}

dockerCfgCreds, err := dockercreds.ParseDockerPullSecrets(credPath)
if err != nil {
logger.Fatal(err)
}
cosignSigner := cosign.NewImageSigner(logger, cli.SignCmd)

for domain := range dockerCfgCreds {
logger.Printf("Loading secret for %q from secret %q at location %q", domain, c, credPath)
}
annotations := mapKeyValueArgs(cosignAnnotations)
cosignRepositoryOverrides := mapKeyValueArgs(cosignRepositories)
cosignDockerMediaTypesOverrides := mapKeyValueArgs(cosignDockerMediaTypes)

creds, err = creds.Append(dockerCfgCreds)
if err != nil {
logger.Fatal(err)
}
ctx := context.Background()
if hasCosign() {
if err := cosignSigner.Sign(ctx, report, cosignSecretLocation, annotations, cosignRepositoryOverrides, cosignDockerMediaTypesOverrides); err != nil {
logger.Fatalf("cosignSigner sign: %v\n", err)
}
}

if notaryV1URL != "" {
signer := notary.ImageSigner{
Logger: logger,
Client: &registry.Client{},
Factory: &notary.RemoteRepositoryFactory{},
}
if err := signer.Sign(notaryV1URL, notarySecretDir, reportFilePath, creds); err != nil {
if err := signer.Sign(notaryV1URL, notarySecretDir, report, creds); err != nil {
logger.Fatal(err)
}
}

logger.Println("Build successful")
}

func mapKeyValueArgs(args flaghelpers.CredentialsFlags) map[string]interface{} {
DennyHoang marked this conversation as resolved.
Show resolved Hide resolved
overrides := make(map[string]interface{})

for _, arg := range args {
splitArg := strings.Split(arg, "=")

if len(splitArg) != 2 {
logger.Fatalf("argument not formatted as -arg=key=value: %s", arg)
}

key := splitArg[0]
value := splitArg[1]

overrides[key] = value
}

return overrides
}

func hasCosign() bool {
_, err := os.Stat(cosignSecretLocation)
return !os.IsNotExist(err)
}
81 changes: 81 additions & 0 deletions docs/image.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ The following defines the relevant fields of the `image` resource spec in more d
- `build`: Configuration that is passed to every image build. See [Build Configuration](#build-config) section below.
- `projectDescriptorPath`: Path to the [project descriptor file](https://buildpacks.io/docs/reference/config/project-descriptor/) relative to source root dir or `subPath` if set. If unset, kpack will look for `project.toml` at the root dir or `subPath` if set.
- `notary`: Configuration for Notary image signing. See [Notary Configuration](#notary-config) section below.
- `cosign`: Configuration for additional cosign image signing. See [Cosign Configuration](#cosign-config) section below.

### <a id='builder-config'></a>Builder Configuration

Expand Down Expand Up @@ -158,6 +159,86 @@ To create the notary secret used by kpack for image signing, run the following c
- `<password>`: The password provided to encrypt the private key.
- `<hash>.key`: The private key file.

### <a id='cosign-config'></a>Cosign Configuration
Copy link
Contributor Author

@DennyHoang DennyHoang Aug 31, 2021

Choose a reason for hiding this comment

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

Would it be alright to have Signing/Verifying builder/clusterBuilder images as a separate PR such that the next version of kpack could have the initial image signing first? Then, we will work on implementing the Signing/Verifying builder/clusterBuilder images portion of the RFC as the following kpack version since upcoming feature cutoff is near for the next release candidate for kpack.


#### Cosign Signing Secret
Images can be signed with cosign when a cosign formatted secret is added to the service account used to build the image.
The secret can be added using the cosign CLI or manually.

To create a cosign signing secret through the cosign CLI, when targetted to the Kubernetes cluster, use:
`cosign generate-key-pair k8s://[NAMESPACE]/[NAME]`

Alternatively, create the cosign secret and provide your own cosign key files manually to Kubernetes by running the following command:
```shell script
% kubectl create secret generic <secret-name> --from-literal=cosign.password=<password> --from-file=</path/to/cosign.key>
```
- `<secret-name>`: The name of the secret. Ensure that the secret is created in the same namespace as the eventual image config.
- `<password>`: The password provided to encrypt the private key. If not present, an empty password will be used.
- `</path/to/cosign.key>`: The cosign private key file generated with `cosign generate-key-pair`.

After adding the cosign secret, the secret must be added to the list of `secrets` attached to the service account resource that is building the image.

#### Adding Cosign Annotations
By default, the build number and build timestamp information will be added to the cosign signing annotations. Users can specify additional cosign annotations under the spec key.
```yaml
cosign:
annotations:
- name: "annotationName"
value: "annotationValue"
```

One way these annotations can be viewed is through verifying cosign signatures. The annotations will be under the `optional` key in the verified JSON response. For example, this can be done with:
```bash
% cosign verify -key /path/to/cosign.pub registry.example.com/project/image@sha256:<DIGEST>
```

Which provides a JSON response similar to:
```json
{
"critical": {
"identity": {
"docker-reference": "registry.example.com/project/image"
}, "image": {
"docker-manifest-digest": "sha256:<DIGEST>"
}, "type": "cosign container image signature"
}, "optional": {
"buildNumber": "1",
"buildTimestamp": "20210827.175240",
"annotationName": "annotationValue"
}
}
```

#### Push Cosign Signature to a Different Location
Cosign signatures can be pushed to a different registry from where the image is located. To enable this, add the corresponding annotation to the cosign secret resource.
```
metadata:
name: ...
namespace: ...
annotations:
kpack.io/cosign.repository: other.registry.com/project/image
data:
cosign.key: ...
cosign.password: ...
```
This will be equivalent to setting `COSIGN_REPOSITORY` as specified in cosign [Specifying Registry](https://github.com/sigstore/cosign#specifying-registry)

The same service account that has that cosign secret attached, and would be used for signing and building the image, would require that the registry credentials for this other repository be placed under the listed `secrets` and is not required to be listed in `imagePullSecrets`. It should be noted that if you wish to push the signatures to the same registry but a different path from the image, the credential used must have access to both paths. You cannot use two separate credentials for the same registry with different paths.

#### Cosign Legacy Docker Media Types
To sign images in a registry that does not fully support OCI media types, legacy equivalents can be used by adding the corresponding annotation to the cosign secret resource:
```
metadata:
name: ...
namespace: ...
annotations:
kpack.io/cosign.docker-media-types: "1"
data:
cosign.key: ...
cosign.password: ...
```
This will be equivalent to setting `COSIGN_DOCKER_MEDIA_TYPES=1` as specified in the cosign [registry-support](https://github.com/sigstore/cosign#registry-support)

### Sample Image with a Git Source

```yaml
Expand Down
21 changes: 13 additions & 8 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,39 @@ require (
github.com/BurntSushi/toml v0.3.1
github.com/Masterminds/semver/v3 v3.1.1
github.com/aryann/difflib v0.0.0-20170710044230-e206f873d14a
github.com/aws/aws-sdk-go v1.31.12 // indirect
github.com/buildpacks/imgutil v0.0.0-20210315155240-52098da06639
github.com/buildpacks/lifecycle v0.10.2
github.com/ghodss/yaml v1.0.0
github.com/go-openapi/spec v0.19.9
github.com/go-openapi/spec v0.20.3
github.com/google/go-cmp v0.5.6
github.com/google/go-containerregistry v0.6.0
github.com/google/go-containerregistry/pkg/authn/k8schain v0.0.0-20210322164748-a11b12f378b5
github.com/jinzhu/gorm v1.9.12 // indirect
github.com/libgit2/git2go/v31 v31.4.14
github.com/matthewmcnew/archtest v0.0.0-20191014222827-a111193b50ad
github.com/mattn/go-colorable v0.1.8 // indirect
github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b
github.com/moby/term v0.0.0-20201216013528-df9cb8a40635 // indirect
github.com/pkg/errors v0.9.1
github.com/sabhiram/go-gitignore v0.0.0-20201211210132-54b8a0bf510f
github.com/sclevine/spec v1.4.0
github.com/sigstore/cosign v1.0.0
github.com/spf13/pflag v1.0.5
github.com/stretchr/testify v1.7.0
github.com/theupdateframework/notary v0.6.2-0.20200804143915-84287fd8df4f
github.com/vdemeester/k8s-pkg-credentialprovider v1.19.7
go.uber.org/zap v1.18.1
golang.org/x/crypto v0.0.0-20210513164829-c07d793c2f9a
golang.org/x/crypto v0.0.0-20210711020723-a769d52b0f97
golang.org/x/sync v0.0.0-20210220032951-036812b2e83c
k8s.io/api v0.20.7
k8s.io/apimachinery v0.20.7
k8s.io/client-go v0.20.7
k8s.io/api v0.21.3
k8s.io/apimachinery v0.21.3
k8s.io/client-go v0.21.3
k8s.io/code-generator v0.20.7
k8s.io/kube-openapi v0.0.0-20210113233702-8566a335510f
k8s.io/kube-openapi v0.0.0-20210305001622-591a79e4bda7
knative.dev/pkg v0.0.0-20210819054404-bda81c029160
)

replace (
github.com/prometheus/common => github.com/prometheus/common v0.26.0
k8s.io/api => k8s.io/api v0.20.7
k8s.io/client-go => k8s.io/client-go v0.20.7
)
Loading