Skip to content

Verify manifest before accepting #12182

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

Merged
merged 2 commits into from
Dec 20, 2016
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
2 changes: 1 addition & 1 deletion pkg/dockerregistry/server/blobdescriptorservice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func TestBlobDescriptorServiceIsApplied(t *testing.T) {
}
os.Setenv("DOCKER_REGISTRY_URL", serverURL.Host)

desc, _, err := registrytest.UploadTestBlob(serverURL, "user/app")
desc, _, err := registrytest.UploadTestBlob(serverURL, nil, "user/app")
if err != nil {
t.Fatal(err)
}
Expand Down
66 changes: 66 additions & 0 deletions pkg/dockerregistry/server/manifesthandler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package server

import (
"fmt"

"github.com/docker/distribution"
"github.com/docker/distribution/context"
"github.com/docker/distribution/manifest/schema1"
"github.com/docker/distribution/manifest/schema2"

imageapi "github.com/openshift/origin/pkg/image/api"
)

// A ManifestHandler defines a common set of operations on all versions of manifest schema.
type ManifestHandler interface {
// FillImageMetadata fills a given image with metadata parsed from manifest. It also corrects layer sizes
// with blob sizes. Newer Docker client versions don't set layer sizes in the manifest schema 1 at all.
// Origin master needs correct layer sizes for proper image quota support. That's why we need to fill the
// metadata in the registry.
FillImageMetadata(ctx context.Context, image *imageapi.Image) error

// Manifest returns a deserialized manifest object.
Manifest() distribution.Manifest

// Payload returns manifest's media type, complete payload with signatures and canonical payload without
// signatures or an error if the information could not be fetched.
Payload() (mediaType string, payload []byte, canonical []byte, err error)

// Verify returns an error if the contained manifest is not valid or has missing dependencies.
Verify(ctx context.Context, skipDependencyVerification bool) error
}

// NewManifestHandler creates a manifest handler for the given manifest.
func NewManifestHandler(repo *repository, manifest distribution.Manifest) (ManifestHandler, error) {
switch t := manifest.(type) {
case *schema1.SignedManifest:
return &manifestSchema1Handler{repo: repo, manifest: t}, nil
case *schema2.DeserializedManifest:
return &manifestSchema2Handler{repo: repo, manifest: t}, nil
default:
return nil, fmt.Errorf("unsupported manifest type %T", manifest)
}
}

// NewManifestHandlerFromImage creates a new manifest handler for a manifest stored in the given image.
func NewManifestHandlerFromImage(repo *repository, image *imageapi.Image) (ManifestHandler, error) {
var (
manifest distribution.Manifest
err error
)

switch image.DockerImageManifestMediaType {
case "", schema1.MediaTypeManifest:
manifest, err = unmarshalManifestSchema1([]byte(image.DockerImageManifest), image.DockerImageSignatures)
case schema2.MediaTypeManifest:
manifest, err = unmarshalManifestSchema2([]byte(image.DockerImageManifest))
default:
return nil, fmt.Errorf("unsupported manifest media type %s", image.DockerImageManifestMediaType)
}

if err != nil {
return nil, err
}

return NewManifestHandler(repo, manifest)
}
174 changes: 174 additions & 0 deletions pkg/dockerregistry/server/manifestschema1handler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
package server

import (
"encoding/json"
"fmt"
"path"

"github.com/docker/distribution"
"github.com/docker/distribution/context"
"github.com/docker/distribution/manifest/schema1"
"github.com/docker/distribution/reference"
"github.com/docker/libtrust"

"k8s.io/kubernetes/pkg/util/sets"

imageapi "github.com/openshift/origin/pkg/image/api"
)

func unmarshalManifestSchema1(content []byte, signatures [][]byte) (distribution.Manifest, error) {
// prefer signatures from the manifest
if _, err := libtrust.ParsePrettySignature(content, "signatures"); err == nil {
sm := schema1.SignedManifest{Canonical: content}
if err = json.Unmarshal(content, &sm); err == nil {
return &sm, nil
}
}

jsig, err := libtrust.NewJSONSignature(content, signatures...)
if err != nil {
return nil, err
}

// Extract the pretty JWS
content, err = jsig.PrettySignature("signatures")
if err != nil {
return nil, err
}

var sm schema1.SignedManifest
if err = json.Unmarshal(content, &sm); err != nil {
return nil, err
}
return &sm, nil
}

type manifestSchema1Handler struct {
repo *repository
manifest *schema1.SignedManifest
}

var _ ManifestHandler = &manifestSchema1Handler{}

func (h *manifestSchema1Handler) FillImageMetadata(ctx context.Context, image *imageapi.Image) error {
signatures, err := h.manifest.Signatures()
if err != nil {
return err
}

for _, signDigest := range signatures {
image.DockerImageSignatures = append(image.DockerImageSignatures, signDigest)
}

if err := imageapi.ImageWithMetadata(image); err != nil {
return err
}

refs := h.manifest.References()

blobSet := sets.NewString()
image.DockerImageMetadata.Size = int64(0)

blobs := h.repo.Blobs(ctx)
for i := range image.DockerImageLayers {
layer := &image.DockerImageLayers[i]
// DockerImageLayers represents h.manifest.Manifest.FSLayers in reversed order
desc, err := blobs.Stat(ctx, refs[len(image.DockerImageLayers)-i-1].Digest)
if err != nil {
context.GetLogger(ctx).Errorf("failed to stat blob %s of image %s", layer.Name, image.DockerImageReference)
return err
}
// The MediaType appeared in manifest schema v2. We need to fill it
// manually in the old images if it is not already filled.
if len(layer.MediaType) == 0 {
if len(desc.MediaType) > 0 {
layer.MediaType = desc.MediaType
} else {
layer.MediaType = schema1.MediaTypeManifestLayer
}
}
layer.LayerSize = desc.Size
// count empty layer just once (empty layer may actually have non-zero size)
if !blobSet.Has(layer.Name) {
image.DockerImageMetadata.Size += desc.Size
blobSet.Insert(layer.Name)
}
}

return nil
}

func (h *manifestSchema1Handler) Manifest() distribution.Manifest {
return h.manifest
}

func (h *manifestSchema1Handler) Payload() (mediaType string, payload []byte, canonical []byte, err error) {
mt, payload, err := h.manifest.Payload()
return mt, payload, h.manifest.Canonical, err
}

func (h *manifestSchema1Handler) Verify(ctx context.Context, skipDependencyVerification bool) error {
var errs distribution.ErrManifestVerification

// we want to verify that referenced blobs exist locally - thus using upstream repository object directly
repo := h.repo.Repository

if len(path.Join(h.repo.registryAddr, h.manifest.Name)) > reference.NameTotalLengthMax {
errs = append(errs,
distribution.ErrManifestNameInvalid{
Name: h.manifest.Name,
Reason: fmt.Errorf("<registry-host>/<manifest-name> must not be more than %d characters", reference.NameTotalLengthMax),
})
}

if !reference.NameRegexp.MatchString(h.manifest.Name) {
errs = append(errs,
distribution.ErrManifestNameInvalid{
Name: h.manifest.Name,
Reason: fmt.Errorf("invalid manifest name format"),
})
}

if len(h.manifest.History) != len(h.manifest.FSLayers) {
errs = append(errs, fmt.Errorf("mismatched history and fslayer cardinality %d != %d",
len(h.manifest.History), len(h.manifest.FSLayers)))
}

if _, err := schema1.Verify(h.manifest); err != nil {
switch err {
case libtrust.ErrMissingSignatureKey, libtrust.ErrInvalidJSONContent, libtrust.ErrMissingSignatureKey:
errs = append(errs, distribution.ErrManifestUnverified{})
default:
if err.Error() == "invalid signature" {
errs = append(errs, distribution.ErrManifestUnverified{})
} else {
errs = append(errs, err)
}
}
}

if skipDependencyVerification {
if len(errs) > 0 {
return errs
}
return nil
}

for _, fsLayer := range h.manifest.References() {
_, err := repo.Blobs(ctx).Stat(ctx, fsLayer.Digest)
if err != nil {
if err != distribution.ErrBlobUnknown {
errs = append(errs, err)
continue
}
Copy link
Contributor

Choose a reason for hiding this comment

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

You probably want to continue here otherwise you will append the error below too, right?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch!

Copy link
Contributor

Choose a reason for hiding this comment

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

Same for the schema2 handler


// On error here, we always append unknown blob errors.
errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.Digest})
}
}

if len(errs) > 0 {
return errs
}
return nil
}
112 changes: 112 additions & 0 deletions pkg/dockerregistry/server/manifestschema2handler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
package server

import (
"encoding/json"
"errors"

"github.com/docker/distribution"
"github.com/docker/distribution/context"
"github.com/docker/distribution/manifest/schema2"

imageapi "github.com/openshift/origin/pkg/image/api"
)

var (
errMissingURL = errors.New("missing URL on layer")
errUnexpectedURL = errors.New("unexpected URL on layer")
)

func unmarshalManifestSchema2(content []byte) (distribution.Manifest, error) {
var deserializedManifest schema2.DeserializedManifest
if err := json.Unmarshal(content, &deserializedManifest); err != nil {
return nil, err
}

return &deserializedManifest, nil
}

type manifestSchema2Handler struct {
repo *repository
manifest *schema2.DeserializedManifest
}

var _ ManifestHandler = &manifestSchema2Handler{}

func (h *manifestSchema2Handler) FillImageMetadata(ctx context.Context, image *imageapi.Image) error {
// The manifest.Config references a configuration object for a container by its digest.
// It needs to be fetched in order to fill an image object metadata below.
configBytes, err := h.repo.Blobs(ctx).Get(ctx, h.manifest.Config.Digest)
if err != nil {
context.GetLogger(ctx).Errorf("failed to get image config %s: %v", h.manifest.Config.Digest.String(), err)
return err
}
image.DockerImageConfig = string(configBytes)

if err := imageapi.ImageWithMetadata(image); err != nil {
return err
}

return nil
}

func (h *manifestSchema2Handler) Manifest() distribution.Manifest {
return h.manifest
}

func (h *manifestSchema2Handler) Payload() (mediaType string, payload []byte, canonical []byte, err error) {
mt, p, err := h.manifest.Payload()
return mt, p, p, err
}

func (h *manifestSchema2Handler) Verify(ctx context.Context, skipDependencyVerification bool) error {
var errs distribution.ErrManifestVerification

if skipDependencyVerification {
return nil
}

// we want to verify that referenced blobs exist locally - thus using upstream repository object directly
repo := h.repo.Repository

target := h.manifest.Target()
_, err := repo.Blobs(ctx).Stat(ctx, target.Digest)
if err != nil {
if err != distribution.ErrBlobUnknown {
errs = append(errs, err)
}

// On error here, we always append unknown blob errors.
errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: target.Digest})
}

for _, fsLayer := range h.manifest.References() {
var err error
if fsLayer.MediaType != schema2.MediaTypeForeignLayer {
if len(fsLayer.URLs) == 0 {
_, err = repo.Blobs(ctx).Stat(ctx, fsLayer.Digest)
} else {
err = errUnexpectedURL
}
} else {
// Clients download this layer from an external URL, so do not check for
// its presense.
if len(fsLayer.URLs) == 0 {
err = errMissingURL
}
}
if err != nil {
if err != distribution.ErrBlobUnknown {
errs = append(errs, err)
continue
}

// On error here, we always append unknown blob errors.
errs = append(errs, distribution.ErrManifestBlobUnknown{Digest: fsLayer.Digest})
}
}

if len(errs) > 0 {
return errs
}
return nil
}
4 changes: 2 additions & 2 deletions pkg/dockerregistry/server/pullthroughblobstore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ func TestPullthroughServeBlob(t *testing.T) {

client.AddReactor("get", "imagestreams", imagetest.GetFakeImageStreamGetHandler(t, *testImageStream))

blob1Desc, blob1Content, err := registrytest.UploadTestBlob(serverURL, "user/app")
blob1Desc, blob1Content, err := registrytest.UploadTestBlob(serverURL, nil, "user/app")
if err != nil {
t.Fatal(err)
}
blob2Desc, blob2Content, err := registrytest.UploadTestBlob(serverURL, "user/app")
blob2Desc, blob2Content, err := registrytest.UploadTestBlob(serverURL, nil, "user/app")
if err != nil {
t.Fatal(err)
}
Expand Down
Loading