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

Add Cosign verification support #1598

Merged
merged 5 commits into from Jul 11, 2022
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion copy/copy.go
Expand Up @@ -654,7 +654,7 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli
sigs = []internalsig.Signature{}
} else {
c.Printf("Getting image source signatures\n")
s, err := src.SignaturesWithFormat(ctx)
s, err := src.UntrustedSignatures(ctx)
if err != nil {
return nil, "", "", perrors.Wrap(err, "reading signatures")
}
Expand Down
41 changes: 37 additions & 4 deletions docs/containers-policy.json.5.md
Expand Up @@ -149,7 +149,7 @@ This requirement rejects every image, and every signature.

### `signedBy`

This requirement requires an image to be signed with an expected identity, or accepts a signature if it is using an expected identity and key.
This requirement requires an image to be signed using “simple signing” with an expected identity, or accepts a signature if it is using an expected identity and key.

```js
{
Expand Down Expand Up @@ -236,6 +236,24 @@ used with `exactReference` or `exactRepository`.

<!-- ### `signedBaseLayer` -->


### `cosignSigned`

This requirement requires an image to be signed using a Cosign signature with an expected identity and key.

```js
{
"type": "cosignSigned",
"keyPath": "/path/to/local/keyring/file",
"keyData": "base64-encoded-keyring-data",
"signedIdentity": identity_requirement
}
```
Exactly one of `keyPath` and `keyData` must be present, containing a Cosign public key. Only signatures made by this key is accepted.

The `signedIdentity` field has the same semantics as in the `signedBy` requirement described above.
Note that `cosign`-created signatures only contain a repository, so only `matchRepository` and `exactRepository` can be used to accept them (and that does not protect against substitution of a signed image with an unexpected tag).
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

… and remapIdentity, useful for non-registries.conf mirroring, does not work for repo-only signatures like that.


## Examples

It is *strongly* recommended to set the `default` policy to `reject`, and then
Expand All @@ -255,9 +273,24 @@ selectively allow individual transports and scopes as desired.
"docker.io/openshift": [{"type": "insecureAcceptAnything"}],
/* Similarly, allow installing the “official” busybox images. Note how the fully expanded
form, with the explicit /library/, must be used. */
"docker.io/library/busybox": [{"type": "insecureAcceptAnything"}]
"docker.io/library/busybox": [{"type": "insecureAcceptAnything"}],
/* Allow installing images from all subdomains */
"*.temporary-project.example.com": [{"type": "insecureAcceptAnything"}]
"*.temporary-project.example.com": [{"type": "insecureAcceptAnything"}],
/* A Cosign-signed repository */
"hostname:5000/myns/cosign-signed-with-full-references": [
{
"type": "cosignSigned",
"keyPath": "/path/to/cosign-pubkey.key"
}
],
/* A Cosign-signed repository, accepts signatures by /usr/bin/cosign */
"hostname:5000/myns/cosign-signed-risky": [
{
"type": "cosignSigned",
"keyPath": "/path/to/cosign-pubkey.key",
"signedIdentity": {"type": "matchRepository"}
}
]
/* Other docker: images use the global default policy and are rejected */
},
"dir": {
Expand Down Expand Up @@ -301,7 +334,7 @@ selectively allow individual transports and scopes as desired.
"signedIdentity": {
"type": "remapIdentity",
"prefix": "private-mirror:5000/vendor-mirror",
"signedPrefix": "vendor.example.com",
"signedPrefix": "vendor.example.com"
}
}
]
Expand Down
1 change: 0 additions & 1 deletion go.mod
Expand Up @@ -37,7 +37,6 @@ require (
golang.org/x/net v0.0.0-20220624214902-1bab6f366d9e
golang.org/x/sync v0.0.0-20220601150217-0de741cfad7f
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211

)

require (
Expand Down
6 changes: 0 additions & 6 deletions internal/image/sourced.go
Expand Up @@ -6,7 +6,6 @@ package image
import (
"context"

"github.com/containers/image/v5/internal/signature"
"github.com/containers/image/v5/types"
)

Expand Down Expand Up @@ -130,11 +129,6 @@ func (i *SourcedImage) Manifest(ctx context.Context) ([]byte, string, error) {
return i.ManifestBlob, i.ManifestMIMEType, nil
}

// SignaturesWithFormat is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.
func (i *SourcedImage) SignaturesWithFormat(ctx context.Context) ([]signature.Signature, error) {
return i.UnparsedImage.signaturesWithFormat(ctx)
}

func (i *SourcedImage) LayerInfosForCopy(ctx context.Context) ([]types.BlobInfo, error) {
return i.UnparsedImage.src.LayerInfosForCopy(ctx, i.UnparsedImage.instanceDigest)
}
8 changes: 5 additions & 3 deletions internal/image/unparsed.go
Expand Up @@ -92,7 +92,9 @@ func (i *UnparsedImage) expectedManifestDigest() (digest.Digest, bool) {

// Signatures is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.
func (i *UnparsedImage) Signatures(ctx context.Context) ([][]byte, error) {
sigs, err := i.signaturesWithFormat(ctx)
// It would be consistent to make this an internal/unparsedimage/impl.Compat wrapper,
// but this is very likely to be the only implementation ever.
sigs, err := i.UntrustedSignatures(ctx)
if err != nil {
return nil, err
}
Expand All @@ -105,8 +107,8 @@ func (i *UnparsedImage) Signatures(ctx context.Context) ([][]byte, error) {
return simpleSigs, nil
}

// signaturesWithFormat is like ImageSource.GetSignatures, but the result is cached; it is OK to call this however often you need.
func (i *UnparsedImage) signaturesWithFormat(ctx context.Context) ([]signature.Signature, error) {
// UntrustedSignatures is like ImageSource.GetSignaturesWithFormat, but the result is cached; it is OK to call this however often you need.
func (i *UnparsedImage) UntrustedSignatures(ctx context.Context) ([]signature.Signature, error) {
if i.cachedSignatures == nil {
sigs, err := i.src.GetSignaturesWithFormat(ctx, i.instanceDigest)
if err != nil {
Expand Down
7 changes: 7 additions & 0 deletions internal/private/private.go
Expand Up @@ -137,3 +137,10 @@ type BadPartialRequestError struct {
func (e BadPartialRequestError) Error() string {
return e.Status
}

// UnparsedImage is an internal extension to the types.UnparsedImage interface.
type UnparsedImage interface {
types.UnparsedImage
// UntrustedSignatures is like ImageSource.GetSignaturesWithFormat, but the result is cached; it is OK to call this however often you need.
UntrustedSignatures(ctx context.Context) ([]signature.Signature, error)
}
6 changes: 6 additions & 0 deletions internal/testing/mocks/unparsed_image.go
Expand Up @@ -3,6 +3,7 @@ package mocks
import (
"context"

"github.com/containers/image/v5/internal/signature"
"github.com/containers/image/v5/types"
)

Expand All @@ -23,3 +24,8 @@ func (ref ForbiddenUnparsedImage) Manifest(ctx context.Context) ([]byte, string,
func (ref ForbiddenUnparsedImage) Signatures(context.Context) ([][]byte, error) {
panic("unexpected call to a mock function")
}

// UntrustedSignatures is a mock that panics.
func (ref ForbiddenUnparsedImage) UntrustedSignatures(ctx context.Context) ([]signature.Signature, error) {
panic("unexpected call to a mock function")
}
38 changes: 38 additions & 0 deletions internal/unparsedimage/wrapper.go
@@ -0,0 +1,38 @@
package unparsedimage

import (
"context"

"github.com/containers/image/v5/internal/private"
"github.com/containers/image/v5/internal/signature"
"github.com/containers/image/v5/types"
)

// wrapped provides the private.UnparsedImage operations
// for an object that only implements types.UnparsedImage
type wrapped struct {
types.UnparsedImage
}

// FromPublic(unparsed) returns an object that provides the private.UnparsedImage API
func FromPublic(unparsed types.UnparsedImage) private.UnparsedImage {
if unparsed2, ok := unparsed.(private.UnparsedImage); ok {
return unparsed2
}
return &wrapped{
UnparsedImage: unparsed,
}
}

// UntrustedSignatures is like ImageSource.GetSignaturesWithFormat, but the result is cached; it is OK to call this however often you need.
func (w *wrapped) UntrustedSignatures(ctx context.Context) ([]signature.Signature, error) {
sigs, err := w.Signatures(ctx)
if err != nil {
return nil, err
}
res := []signature.Signature{}
for _, sig := range sigs {
res = append(res, signature.SimpleSigningFromBlob(sig))
}
return res, nil
}
4 changes: 4 additions & 0 deletions signature/fixtures/cosign.pub
@@ -0,0 +1,4 @@
-----BEGIN PUBLIC KEY-----
MFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEFNLqFhf4fiN6o/glAuYnq2jYUeL0
vRuLu/z39pmbVwS9ff5AYnlwaP9sxREajdLY9ynM6G1sy6AAmb7Z63TsLg==
-----END PUBLIC KEY-----
1 change: 1 addition & 0 deletions signature/fixtures/dir-img-cosign-mixed/manifest.json
1 change: 1 addition & 0 deletions signature/fixtures/dir-img-cosign-mixed/signature-1
1 change: 1 addition & 0 deletions signature/fixtures/dir-img-cosign-mixed/signature-2
17 changes: 17 additions & 0 deletions signature/fixtures/dir-img-cosign-modified-manifest/manifest.json
@@ -0,0 +1,17 @@
{
"schemaVersion": 2,
"mediaType": "application/vnd.docker.distribution.manifest.v2+json",
"config": {
"mediaType": "application/vnd.docker.container.image.v1+json",
"size": 1512,
"digest": "sha256:961769676411f082461f9ef46626dd7a2d1e2b2a38e6a44364bcbecf51e66dd4"
},
"layers": [
{
"mediaType": "application/vnd.docker.image.rootfs.diff.tar.gzip",
"size": 2896510,
"digest": "sha256:9d16cba9fb961d1aafec9542f2bf7cb64acfc55245f9e4eb5abecd4cdc38d749"
}
],
"extra": "this manifest has been modified"
}
1 change: 1 addition & 0 deletions signature/fixtures/dir-img-cosign-no-manifest/signature-1
Binary file not shown.
1 change: 1 addition & 0 deletions signature/fixtures/dir-img-cosign-valid-2/manifest.json
1 change: 1 addition & 0 deletions signature/fixtures/dir-img-cosign-valid-2/signature-1
Binary file not shown.
@@ -0,0 +1 @@
{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","config":{"mediaType":"application/vnd.docker.container.image.v1+json","size":1512,"digest":"sha256:961769676411f082461f9ef46626dd7a2d1e2b2a38e6a44364bcbecf51e66dd4"},"layers":[{"mediaType":"application/vnd.docker.image.rootfs.diff.tar.gzip","size":2896510,"digest":"sha256:9d16cba9fb961d1aafec9542f2bf7cb64acfc55245f9e4eb5abecd4cdc38d749"}]}
Binary file not shown.
1 change: 1 addition & 0 deletions signature/fixtures/dir-img-cosign-valid/manifest.json
@@ -0,0 +1 @@
{"schemaVersion":2,"mediaType":"application/vnd.docker.distribution.manifest.v2+json","config":{"mediaType":"application/vnd.docker.container.image.v1+json","size":1512,"digest":"sha256:961769676411f082461f9ef46626dd7a2d1e2b2a38e6a44364bcbecf51e66dd4"},"layers":[{"mediaType":"application/vnd.docker.image.rootfs.diff.tar.gzip","size":2896510,"digest":"sha256:9d16cba9fb961d1aafec9542f2bf7cb64acfc55245f9e4eb5abecd4cdc38d749"}]}
Binary file added signature/fixtures/dir-img-cosign-valid/signature-1
Binary file not shown.
15 changes: 15 additions & 0 deletions signature/fixtures/policy.json
Expand Up @@ -132,6 +132,21 @@
"dockerReference": "registry.access.redhat.com/rhel7/rhel:latest"
}
}
],
"example.com/cosign/key-data-example": [
{
"type": "cosignSigned",
"keyData": "bm9uc2Vuc2U="
}
],
"example.com/cosign/key-Path-example": [
{
"type": "cosignSigned",
"keyPath": "/keys/public-key",
"signedIdentity": {
"type": "matchRepository"
}
}
]
}
}
Expand Down
Binary file added signature/fixtures/unknown-cosign-key.signature
Binary file not shown.
80 changes: 66 additions & 14 deletions signature/internal/cosign_payload.go
@@ -1,17 +1,22 @@
package internal

import (
"bytes"
"crypto"
"encoding/base64"
"encoding/json"
"errors"
"fmt"
"time"

"github.com/containers/image/v5/version"
digest "github.com/opencontainers/go-digest"
cosignSignature "github.com/sigstore/sigstore/pkg/signature"
)

const (
cosignSignatureType = "cosign container image signature"
cosignSignatureType = "cosign container image signature"
cosignHarcodedHashAlgorithm = crypto.SHA256
)

// UntrustedCosignPayload is a parsed content of a Cosign signature payload (not the full signature)
Expand Down Expand Up @@ -96,20 +101,23 @@ func (s *UntrustedCosignPayload) strictUnmarshalJSON(data []byte) error {
var creatorID string
var timestamp float64
var gotCreatorID, gotTimestamp = false, false
if err := ParanoidUnmarshalJSONObject(optional, func(key string) interface{} {
switch key {
case "creator":
gotCreatorID = true
return &creatorID
case "timestamp":
gotTimestamp = true
return &timestamp
default:
var ignore interface{}
return &ignore
// Cosign generates "optional": null if there are no user-specified annotations.
if !bytes.Equal(optional, []byte("null")) {
if err := ParanoidUnmarshalJSONObject(optional, func(key string) interface{} {
switch key {
case "creator":
gotCreatorID = true
return &creatorID
case "timestamp":
gotTimestamp = true
return &timestamp
default:
var ignore interface{}
return &ignore
}
}); err != nil {
return err
}
}); err != nil {
return err
}
if gotCreatorID {
s.UntrustedCreatorID = &creatorID
Expand Down Expand Up @@ -147,3 +155,47 @@ func (s *UntrustedCosignPayload) strictUnmarshalJSON(data []byte) error {
"docker-reference": &s.UntrustedDockerReference,
})
}

// CosignPayloadAcceptanceRules specifies how to decide whether an untrusted payload is acceptable.
// We centralize the actual parsing and data extraction in VerifyCosignPayload; this supplies
// the policy. We use an object instead of supplying func parameters to verifyAndExtractSignature
// because the functions have the same or similar types, so there is a risk of exchanging the functions;
// named members of this struct are more explicit.
type CosignPayloadAcceptanceRules struct {
ValidateSignedDockerReference func(string) error
ValidateSignedDockerManifestDigest func(digest.Digest) error
}

// VerifyCosignPayload verifies unverifiedBase64Signature of unverifiedPayload was correctly created by publicKey, and that its principal components
// match expected values, both as specified by rules, and returns it.
// We return an *UntrustedCosignPayload, although nothing actually uses it,
// just to double-check against stupid typos.
func VerifyCosignPayload(publicKey crypto.PublicKey, unverifiedPayload []byte, unverifiedBase64Signature string, rules CosignPayloadAcceptanceRules) (*UntrustedCosignPayload, error) {
verifier, err := cosignSignature.LoadVerifier(publicKey, cosignHarcodedHashAlgorithm)
if err != nil {
return nil, fmt.Errorf("creating verifier: %w", err)
}

unverifiedSignature, err := base64.StdEncoding.DecodeString(unverifiedBase64Signature)
if err != nil {
return nil, NewInvalidSignatureError(fmt.Sprintf("base64 decoding: %v", err))
}
// github.com/sigstore/cosign/pkg/cosign.verifyOCISignature uses signatureoptions.WithContext(),
// which seems to be not used by anything. So we don’t bother.
if err := verifier.VerifySignature(bytes.NewReader(unverifiedSignature), bytes.NewReader(unverifiedPayload)); err != nil {
mtrmac marked this conversation as resolved.
Show resolved Hide resolved
return nil, NewInvalidSignatureError(fmt.Sprintf("cryptographic signature verification failed: %v", err))
}

var unmatchedPayload UntrustedCosignPayload
if err := json.Unmarshal(unverifiedPayload, &unmatchedPayload); err != nil {
return nil, NewInvalidSignatureError(err.Error())
}
if err := rules.ValidateSignedDockerManifestDigest(unmatchedPayload.UntrustedDockerManifestDigest); err != nil {
return nil, err
}
if err := rules.ValidateSignedDockerReference(unmatchedPayload.UntrustedDockerReference); err != nil {
return nil, err
}
// CosignPayloadAcceptanceRules have accepted this value.
return &unmatchedPayload, nil
}