diff --git a/lib/common/common.go b/lib/common/common.go index 5fd1d460..5275d5a4 100644 --- a/lib/common/common.go +++ b/lib/common/common.go @@ -16,6 +16,9 @@ package common import ( + "fmt" + "regexp" + "github.com/appc/docker2aci/lib/internal/docker" "github.com/appc/docker2aci/lib/internal/types" ) @@ -27,6 +30,10 @@ const ( GzipCompression ) +var ( + validId = regexp.MustCompile(`^(\w+:)?([A-Fa-f0-9]+)$`) +) + type ParsedDockerURL types.ParsedDockerURL const ( @@ -60,3 +67,11 @@ func ParseDockerURL(arg string) (*ParsedDockerURL, error) { p, err := docker.ParseDockerURL(arg) return (*ParsedDockerURL)(p), err } + +// ValidateLayerId validates a layer ID +func ValidateLayerId(id string) error { + if ok := validId.MatchString(id); !ok { + return fmt.Errorf("invalid layer ID %q", id) + } + return nil +} diff --git a/lib/internal/backend/file/file.go b/lib/internal/backend/file/file.go index a83402df..708e333a 100644 --- a/lib/internal/backend/file/file.go +++ b/lib/internal/backend/file/file.go @@ -73,6 +73,9 @@ func (lb *FileBackend) BuildACI(layerIDs []string, dockerURL *types.ParsedDocker var aciManifests []*schema.ImageManifest var curPwl []string for i := len(layerIDs) - 1; i >= 0; i-- { + if err := common.ValidateLayerId(layerIDs[i]); err != nil { + return nil, nil, err + } tmpDir, err := ioutil.TempDir(tmpBaseDir, "docker2aci-") if err != nil { return nil, nil, fmt.Errorf("error creating dir: %v", err) @@ -279,14 +282,24 @@ func extractEmbeddedLayer(file *os.File, layerID string, outputPath string) (*os return layerFile, nil } +// getAncestry computes an image ancestry, returning an ordered list +// of dependencies starting from the topmost image to the base. +// It checks for dependency loops via duplicate detection in the image +// chain and errors out in such cases. func getAncestry(file *os.File, imgID string) ([]string, error) { var ancestry []string + deps := make(map[string]bool) curImgID := imgID var err error for curImgID != "" { + if deps[curImgID] { + return nil, fmt.Errorf("dependency loop detected at image %q", curImgID) + } + deps[curImgID] = true ancestry = append(ancestry, curImgID) + log.Debug(fmt.Sprintf("Getting ancestry for layer %q", curImgID)) curImgID, err = getParent(file, curImgID) if err != nil { return nil, err @@ -328,5 +341,6 @@ func getParent(file *os.File, imgID string) (string, error) { return "", err } + log.Debug(fmt.Sprintf("Layer %q depends on layer %q", imgID, parent)) return parent, nil } diff --git a/lib/internal/backend/repository/repository1.go b/lib/internal/backend/repository/repository1.go index bc54e47c..7d4fe917 100644 --- a/lib/internal/backend/repository/repository1.go +++ b/lib/internal/backend/repository/repository1.go @@ -75,6 +75,9 @@ func (rb *RepositoryBackend) buildACIV1(layerIDs []string, dockerURL *types.Pars var doneChannels []chan error for i, layerID := range layerIDs { + if err := common.ValidateLayerId(layerID); err != nil { + return nil, nil, err + } doneChan := make(chan error) doneChannels = append(doneChannels, doneChan) // https://github.com/golang/go/wiki/CommonMistakes diff --git a/lib/internal/backend/repository/repository2.go b/lib/internal/backend/repository/repository2.go index d074e9a0..208c9540 100644 --- a/lib/internal/backend/repository/repository2.go +++ b/lib/internal/backend/repository/repository2.go @@ -23,7 +23,6 @@ import ( "net/http" "os" "path" - "regexp" "strconv" "strings" "sync" @@ -43,8 +42,6 @@ const ( defaultIndexURL = "registry-1.docker.io" ) -var validHex = regexp.MustCompile(`^([a-f0-9]{64})$`) - // A manifest conforming to the docker v2.1 spec type v2Manifest struct { Name string `json:"name"` @@ -91,6 +88,9 @@ func (rb *RepositoryBackend) buildACIV21(layerIDs []string, dockerURL *types.Par closers := make([]io.ReadCloser, len(layerIDs)) var wg sync.WaitGroup for i, layerID := range layerIDs { + if err := common.ValidateLayerId(layerID); err != nil { + return nil, nil, err + } wg.Add(1) errChan := make(chan error, 1) errChannels = append(errChannels, errChan) @@ -195,6 +195,9 @@ func (rb *RepositoryBackend) buildACIV22(layerIDs []string, dockerURL *types.Par resultChan := make(chan layer, len(layerIDs)) for i, layerID := range layerIDs { + if err := common.ValidateLayerId(layerID); err != nil { + return nil, nil, err + } // https://github.com/golang/go/wiki/CommonMistakes i := i // golang-- layerID := layerID @@ -436,7 +439,7 @@ func fixManifestLayers(manifest *v2Manifest) error { } imgs[i] = img - if err := validateV1ID(img.ID); err != nil { + if err := common.ValidateLayerId(img.ID); err != nil { return err } } @@ -471,13 +474,6 @@ func fixManifestLayers(manifest *v2Manifest) error { return nil } -func validateV1ID(id string) error { - if ok := validHex.MatchString(id); !ok { - return fmt.Errorf("image ID %q is invalid", id) - } - return nil -} - func (rb *RepositoryBackend) getLayerV2(layerID string, dockerURL *types.ParsedDockerURL, tmpDir string, copier *progressutil.CopyProgressPrinter) (*os.File, io.ReadCloser, error) { var ( err error diff --git a/tests/fixture-test-depsloop/check.sh b/tests/fixture-test-depsloop/check.sh new file mode 100755 index 00000000..798f4570 --- /dev/null +++ b/tests/fixture-test-depsloop/check.sh @@ -0,0 +1,15 @@ +#!/bin/sh + +DOCKER2ACI=../bin/docker2aci +TESTDIR=$1 +TESTNAME=$2 + +timeout 10s ${DOCKER2ACI} "${TESTDIR}/${TESTNAME}/${TESTNAME}.docker" +if [ $? -eq 1 ]; then + echo "### Test case ${TESTNAME}: SUCCESS" + exit 0 +else + echo "### Test case ${TESTNAME}: FAIL" + exit 1 +fi + diff --git a/tests/fixture-test-depsloop/fixture-test-depsloop.docker b/tests/fixture-test-depsloop/fixture-test-depsloop.docker new file mode 100644 index 00000000..f5fe5513 Binary files /dev/null and b/tests/fixture-test-depsloop/fixture-test-depsloop.docker differ diff --git a/tests/fixture-test-invalidlayerid/check.sh b/tests/fixture-test-invalidlayerid/check.sh new file mode 100755 index 00000000..bbf0cb4f --- /dev/null +++ b/tests/fixture-test-invalidlayerid/check.sh @@ -0,0 +1,15 @@ +#!/bin/sh + +DOCKER2ACI=../bin/docker2aci +TESTDIR=$1 +TESTNAME=$2 + +sudo ${DOCKER2ACI} "${TESTDIR}/${TESTNAME}/${TESTNAME}.docker" +if [ $? -eq 1 ]; then + echo "### Test case ${TESTNAME}: SUCCESS" + exit 0 +else + echo "### Test case ${TESTNAME}: FAIL" + exit 1 +fi + diff --git a/tests/fixture-test-invalidlayerid/fixture-test-invalidlayerid.docker b/tests/fixture-test-invalidlayerid/fixture-test-invalidlayerid.docker new file mode 100644 index 00000000..ef246448 Binary files /dev/null and b/tests/fixture-test-invalidlayerid/fixture-test-invalidlayerid.docker differ diff --git a/tests/test.sh b/tests/test.sh index 5844c9b0..658b7150 100755 --- a/tests/test.sh +++ b/tests/test.sh @@ -45,6 +45,12 @@ RKT=$(which rkt) DOCKER_STORAGE_BACKEND=$(sudo docker info|grep '^Storage Driver:'|sed 's/Storage Driver: //') +for i in $(find . -maxdepth 1 -type d -name 'fixture-test*') ; do + TESTNAME=$(basename $i) + echo "### Test case ${TESTNAME}..." + $TESTDIR/${TESTNAME}/check.sh "${TESTDIR}" "${TESTNAME}" +done + for i in $(find . -maxdepth 1 -type d -name 'test-*') ; do TESTNAME=$(basename $i) echo "### Test case ${TESTNAME}: build..." @@ -81,4 +87,3 @@ for i in $(find . -maxdepth 1 -type d -name 'test-*') ; do sudo docker rmi $PREFIX/${TESTNAME} done -