Skip to content

Commit

Permalink
Addressed most of vrothberg comments
Browse files Browse the repository at this point in the history 
Signed-off-by: Brandon Lum <lumjjb@gmail.com>
  • Loading branch information
@lumjjb
lumjjb committed Sep 11, 2019
1 parent 3165d1b commit fb0667f
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 42 deletions.
69 changes: 35 additions & 34 deletions copy/copy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,24 +41,24 @@ type digestingReader struct {
expectedDigest digest.Digest
validationFailed bool
validationSucceeded bool
skipValidation bool
validateDigests bool
}

var (
// ErrDecryptParamsMissing is returned if there is missing decryption parameters
ErrDecryptParamsMissing = errors.New("Necessary DecryptParameters not present")
)

// maxParallelDownloads is used to limit the maxmimum number of parallel
// downloads. Let's follow Firefox by limiting it to 6.
var maxParallelDownloads = 6
// maxParallelDownloads is used to limit the maxmimum number of parallel
// downloads. Let's follow Firefox by limiting it to 6.
maxParallelDownloads = 6
)

// newDigestingReader returns an io.Reader implementation with contents of source, which will eventually return a non-EOF error
// or set validationSucceeded/validationFailed to true if the source stream does/does not match expectedDigest.
// (neither is set if EOF is never reached).
func newDigestingReader(source io.Reader, expectedDigest digest.Digest, skipValidation bool) (*digestingReader, error) {
func newDigestingReader(source io.Reader, expectedDigest digest.Digest, validateDigests bool) (*digestingReader, error) {
var digester digest.Digester
if !skipValidation {
if validateDigests {
if err := expectedDigest.Validate(); err != nil {
return nil, errors.Errorf("Invalid digest specification %s", expectedDigest)
}
Expand All @@ -73,13 +73,13 @@ func newDigestingReader(source io.Reader, expectedDigest digest.Digest, skipVali
digester: digester,
expectedDigest: expectedDigest,
validationFailed: false,
skipValidation: skipValidation,
validateDigests: validateDigests,
}, nil
}

func (d *digestingReader) Read(p []byte) (int, error) {
n, err := d.source.Read(p)
if !d.skipValidation {
if d.validateDigests {
if n > 0 {
if n2, err := d.digester.Hash().Write(p[:n]); n2 != n || err != nil {
// Coverage: This should not happen, the hash.Hash interface requires
Expand All @@ -89,7 +89,7 @@ func (d *digestingReader) Read(p []byte) (int, error) {
}
}
}
if err == io.EOF && !d.skipValidation {
if err == io.EOF && d.validateDigests {
actualDigest := d.digester.Digest()
if actualDigest != d.expectedDigest {
d.validationFailed = true
Expand Down Expand Up @@ -141,11 +141,14 @@ type Options struct {
// manifest MIME type of image set by user. "" is default and means use the autodetection to the the manifest MIME type
ForceManifestMIMEType string
CheckAuthorization bool
// If non-nil indicates that image should be encrypted.
// If EncryptConfig is non-nil, it indicates that an image should be encrypted.
// The encryption options is derived from the construction of EncryptConfig object.
EncryptConfig *encconfig.EncryptConfig
// EncryptLayers represents the list of layers to encrypt.
// If nil, don't encrypt any layers
// If nil, don't encrypt any layers.
// If non-nil and len==0, denotes encrypt all layers.
// integers in the slice represent 0-indexed layer indices, with support for negative
// indexing. i.e. 0 is the first layer, -1 is the last (top-most) layer.
EncryptLayers *[]int
}

Expand Down Expand Up @@ -316,11 +319,9 @@ func (c *copier) copyOneImage(ctx context.Context, policyContext *signature.Poli

// Set up encryption structs
var (
//ec *encconfig.EncryptConfig
dc *encconfig.DecryptConfig
)
if options.SourceCtx.CryptoConfig != nil {
//ec = options.SourceCtx.CryptoConfig.EncryptConfig
dc = options.SourceCtx.CryptoConfig.DecryptConfig
}

Expand Down Expand Up @@ -542,14 +543,19 @@ func (ic *imageCopier) copyLayers(ctx context.Context) error {
// Create layer Encryption map
encLayerBitmap := map[int]bool{}
var encryptAll bool
encryptLayers := ic.encryptLayers != nil
if ic.encryptLayers != nil {
encryptAll = len(*ic.encryptLayers) == 0
totalLayers := len(srcInfos)
for _, l := range *ic.encryptLayers {
// if layer is negative, it is reverse indexed.
encLayerBitmap[(totalLayers+l)%totalLayers] = true
}

if encryptAll {
for i := 0; i < len(srcInfos); i++ {
encLayerBitmap[i] = true
}
}
}

func() { // A scope for defer
Expand All @@ -558,8 +564,7 @@ func (ic *imageCopier) copyLayers(ctx context.Context) error {

for i, srcLayer := range srcInfos {
copySemaphore.Acquire(ctx, 1)
toEncrypt := encryptLayers && (encryptAll || encLayerBitmap[i])
go copyLayerHelper(i, srcLayer, toEncrypt, progressPool)
go copyLayerHelper(i, srcLayer, encLayerBitmap[i], progressPool)
}

// Wait for all layers to be copied
Expand Down Expand Up @@ -739,28 +744,26 @@ func (ic *imageCopier) copyLayer(ctx context.Context, srcInfo types.BlobInfo, to
// the symmetric key with the provided private keys. If we fail, we will
// not allow the image to be provisioned.
if ic.checkAuthorization {
if srcInfo.MediaType == manifest.DockerV2Schema2LayerGzipEncMediaType ||
srcInfo.MediaType == manifest.DockerV2Schema2LayerEncMediaType {
if srcInfo.MediaType == manifest.OCILayerGzipEncMediaType ||
srcInfo.MediaType == manifest.OCILayerEncMediaType {

if ic.decryptConfig == nil {
return types.BlobInfo{}, "", errors.New("Necessary DecryptParameters not present")
}

dc := ic.decryptConfig

newDesc := ocispec.Descriptor{
Annotations: srcInfo.Annotations,
}

_, _, err := ocicrypt.DecryptLayer(dc, nil, newDesc, true)
_, _, err := ocicrypt.DecryptLayer(ic.decryptConfig, nil, newDesc, true)
if err != nil {
return types.BlobInfo{}, "", errors.Wrapf(err, "Image authentication failed for the digest %+v", srcInfo.Digest)
}
}
}

cachedDiffID := ic.c.blobInfoCache.UncompressedDigest(srcInfo.Digest) // May be ""
// Diffs are needed if we are encrypting an image
// Diffs are needed if we are encrypting an image
diffIDIsNeeded := ic.diffIDsAreNeeded && cachedDiffID == "" || ic.encryptConfig != nil

// If we already have the blob, and we don't need to compute the diffID, then we don't need to read it from the source.
Expand Down Expand Up @@ -894,39 +897,37 @@ func (c *copier) copyBlobFromStream(ctx context.Context, srcStream io.Reader, sr

var decrypted bool
var err error
if srcInfo.MediaType == manifest.DockerV2Schema2LayerGzipEncMediaType ||
srcInfo.MediaType == manifest.DockerV2Schema2LayerEncMediaType {
if srcInfo.MediaType == manifest.OCILayerGzipEncMediaType ||
srcInfo.MediaType == manifest.OCILayerEncMediaType {

if c.decryptConfig == nil {
return types.BlobInfo{}, ErrDecryptParamsMissing
}

dc := c.decryptConfig

newDesc := ocispec.Descriptor{
Annotations: srcInfo.Annotations,
}

var d digest.Digest
srcStream, d, err = ocicrypt.DecryptLayer(dc, srcStream, newDesc, false)
srcStream, d, err = ocicrypt.DecryptLayer(c.decryptConfig, srcStream, newDesc, false)
if err != nil {
return types.BlobInfo{}, errors.Wrapf(err, "Error decrypting layer %s", srcInfo.Digest)
}

srcInfo.Digest = d
srcInfo.Size = -1
switch srcInfo.MediaType {
case manifest.DockerV2Schema2LayerGzipEncMediaType:
case manifest.OCILayerGzipEncMediaType:
srcInfo.MediaType = ocispec.MediaTypeImageLayerGzip
case manifest.DockerV2Schema2LayerEncMediaType:
case manifest.OCILayerEncMediaType:
srcInfo.MediaType = ocispec.MediaTypeImageLayer
}
decrypted = true
}

skipDigestValidation := srcInfo.Digest == ""
validateDigest := srcInfo.Digest != ""

digestingReader, err := newDigestingReader(srcStream, srcInfo.Digest, skipDigestValidation)
digestingReader, err := newDigestingReader(srcStream, srcInfo.Digest, validateDigest)
if err != nil {
return types.BlobInfo{}, errors.Wrapf(err, "Error preparing to verify blob %s", srcInfo.Digest)
}
Expand Down Expand Up @@ -993,9 +994,9 @@ func (c *copier) copyBlobFromStream(ctx context.Context, srcStream io.Reader, sr
if toEncrypt {
switch srcInfo.MediaType {
case manifest.DockerV2Schema2LayerMediaType, ocispec.MediaTypeImageLayerGzip:
encryptMediaType = manifest.DockerV2Schema2LayerGzipEncMediaType
encryptMediaType = manifest.OCILayerGzipEncMediaType
case ocispec.MediaTypeImageLayer:
encryptMediaType = manifest.DockerV2Schema2LayerEncMediaType
encryptMediaType = manifest.OCILayerEncMediaType
}

if encryptMediaType != "" && c.encryptConfig != nil {
Expand Down
8 changes: 4 additions & 4 deletions copy/copy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func TestNewDigestingReader(t *testing.T) {
"sha256:0", // Invalid hex value
"sha256:01", // Invalid length of hex value
} {
_, err := newDigestingReader(source, input, false)
_, err := newDigestingReader(source, input, true)
assert.Error(t, err, input.String())
}
}
Expand All @@ -43,7 +43,7 @@ func TestDigestingReaderRead(t *testing.T) {
// Valid input
for _, c := range cases {
source := bytes.NewReader(c.input)
reader, err := newDigestingReader(source, c.digest, false)
reader, err := newDigestingReader(source, c.digest, true)
require.NoError(t, err, c.digest.String())
dest := bytes.Buffer{}
n, err := io.Copy(&dest, reader)
Expand All @@ -56,7 +56,7 @@ func TestDigestingReaderRead(t *testing.T) {
// Modified input
for _, c := range cases {
source := bytes.NewReader(bytes.Join([][]byte{c.input, []byte("x")}, nil))
reader, err := newDigestingReader(source, c.digest, false)
reader, err := newDigestingReader(source, c.digest, true)
require.NoError(t, err, c.digest.String())
dest := bytes.Buffer{}
_, err = io.Copy(&dest, reader)
Expand All @@ -67,7 +67,7 @@ func TestDigestingReaderRead(t *testing.T) {
// Truncated input
for _, c := range cases {
source := bytes.NewReader(c.input)
reader, err := newDigestingReader(source, c.digest, false)
reader, err := newDigestingReader(source, c.digest, true)
require.NoError(t, err, c.digest.String())
if len(c.input) != 0 {
dest := bytes.Buffer{}
Expand Down
8 changes: 4 additions & 4 deletions manifest/manifest.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@ const (
DockerV2Schema2ConfigMediaType = "application/vnd.docker.container.image.v1+json"
// DockerV2Schema2LayerMediaType is the MIME type used for schema 2 layers.
DockerV2Schema2LayerMediaType = "application/vnd.docker.image.rootfs.diff.tar.gzip"
// DockerV2Schema2LayerGzipEncMediaType is MIME type used for encrypted compressed layers.
DockerV2Schema2LayerGzipEncMediaType = "application/vnd.docker.image.rootfs.diff.tar.gzip+enc"
// DockerV2Schema2LayerEncMediaType is MIME type used for encrypted layers.
DockerV2Schema2LayerEncMediaType = "application/vnd.docker.image.rootfs.diff.tar+enc"
// DockerV2ListMediaType MIME type represents Docker manifest schema 2 list
DockerV2ListMediaType = "application/vnd.docker.distribution.manifest.list.v2+json"
// DockerV2Schema2ForeignLayerMediaType is the MIME type used for schema 2 foreign layers.
DockerV2Schema2ForeignLayerMediaType = "application/vnd.docker.image.rootfs.foreign.diff.tar.gzip"
// OCILayerEncMediaType is MIME type used for encrypted layers.
OCILayerEncMediaType = "application/vnd.oci.image.layer.v1.tar+enc"
// OCILayerGzipEncMediaType is MIME type used for encrypted compressed layers.
OCILayerGzipEncMediaType = "application/vnd.oci.image.layer.v1.tar+gzip+enc"
)

// DefaultRequestedManifestMIMETypes is a list of MIME types a types.ImageSource
Expand Down

0 comments on commit fb0667f

Please sign in to comment.