Re-work the way checksum matching works against Release file

Break up URL into base part and relative path. Match checksum against relative path
and never against full URL.

This might be fixing security issue if aptly was incorrectly matching against
wrong part of Release file.

deb/remote.go

@@ -7,7 +7,6 @@ import (
 	"net/url"
 	"os"
 	"path"
-	"path/filepath"
 	"sort"
 	"strconv"
 	"strings"
@@ -191,49 +190,49 @@ func (repo *RemoteRepo) CheckLock() error {
 	return nil
 }
 
-// ReleaseURL returns URL to Release* files in repo root
+// IndexesRootURL builds URL for various indexes
-func (repo *RemoteRepo) ReleaseURL(name string) *url.URL {
+func (repo *RemoteRepo) IndexesRootURL() *url.URL {
 	var path *url.URL
 
 	if !repo.IsFlat() {
-		path = &url.URL{Path: fmt.Sprintf("dists/%s/%s", repo.Distribution, name)}
+		path = &url.URL{Path: fmt.Sprintf("dists/%s/", repo.Distribution)}
 	} else {
-		path = &url.URL{Path: filepath.Join(repo.Distribution, name)}
+		path = &url.URL{Path: fmt.Sprintf("%s", repo.Distribution)}
 	}
 
 	return repo.archiveRootURL.ResolveReference(path)
 }
 
-// FlatBinaryURL returns URL to Packages files for flat repo
+// ReleaseURL returns URL to Release* files in repo root
-func (repo *RemoteRepo) FlatBinaryURL() *url.URL {
+func (repo *RemoteRepo) ReleaseURL(name string) *url.URL {
-	path := &url.URL{Path: filepath.Join(repo.Distribution, "Packages")}
+	return repo.IndexesRootURL().ResolveReference(&url.URL{Path: name})
-	return repo.archiveRootURL.ResolveReference(path)
 }
 
-// FlatSourcesURL returns URL to Sources files for flat repo
+// FlatBinaryPath returns path to Packages files for flat repo
-func (repo *RemoteRepo) FlatSourcesURL() *url.URL {
+func (repo *RemoteRepo) FlatBinaryPath() string {
-	path := &url.URL{Path: filepath.Join(repo.Distribution, "Sources")}
+	return "Packages"
-	return repo.archiveRootURL.ResolveReference(path)
 }
 
-// BinaryURL returns URL of Packages files for given component and
+// FlatSourcesPath returns path to Sources files for flat repo
+func (repo *RemoteRepo) FlatSourcesPath() string {
+	return "Sources"
+}
+
+// BinaryPath returns path to Packages files for given component and
 // architecture
-func (repo *RemoteRepo) BinaryURL(component string, architecture string) *url.URL {
+func (repo *RemoteRepo) BinaryPath(component string, architecture string) string {
-	path := &url.URL{Path: fmt.Sprintf("dists/%s/%s/binary-%s/Packages", repo.Distribution, component, architecture)}
+	return fmt.Sprintf("%s/binary-%s/Packages", component, architecture)
-	return repo.archiveRootURL.ResolveReference(path)
 }
 
-// SourcesURL returns URL of Sources files for given component
+// SourcesPath returns path to Sources files for given component
-func (repo *RemoteRepo) SourcesURL(component string) *url.URL {
+func (repo *RemoteRepo) SourcesPath(component string) string {
-	path := &url.URL{Path: fmt.Sprintf("dists/%s/%s/source/Sources", repo.Distribution, component)}
+	return fmt.Sprintf("%s/source/Sources", component)
-	return repo.archiveRootURL.ResolveReference(path)
 }
 
-// UdebURL returns URL of Packages files for given component and
+// UdebPath returns path of Packages files for given component and
 // architecture
-func (repo *RemoteRepo) UdebURL(component string, architecture string) *url.URL {
+func (repo *RemoteRepo) UdebPath(component string, architecture string) string {
-	path := &url.URL{Path: fmt.Sprintf("dists/%s/%s/debian-installer/binary-%s/Packages", repo.Distribution, component, architecture)}
+	return fmt.Sprintf("%s/debian-installer/binary-%s/Packages", component, architecture)
-	return repo.archiveRootURL.ResolveReference(path)
 }
 
 // PackageURL returns URL of package file relative to repository root
@@ -407,30 +406,30 @@ func (repo *RemoteRepo) DownloadPackageIndexes(progress aptly.Progress, d aptly.
 	repo.packageList = NewPackageList()
 
 	// Download and parse all Packages & Source files
-	packagesURLs := [][]string{}
+	packagesPaths := [][]string{}
 
 	if repo.IsFlat() {
-		packagesURLs = append(packagesURLs, []string{repo.FlatBinaryURL().String(), PackageTypeBinary})
+		packagesPaths = append(packagesPaths, []string{repo.FlatBinaryPath(), PackageTypeBinary})
 		if repo.DownloadSources {
-			packagesURLs = append(packagesURLs, []string{repo.FlatSourcesURL().String(), PackageTypeSource})
+			packagesPaths = append(packagesPaths, []string{repo.FlatSourcesPath(), PackageTypeSource})
 		}
 	} else {
 		for _, component := range repo.Components {
 			for _, architecture := range repo.Architectures {
-				packagesURLs = append(packagesURLs, []string{repo.BinaryURL(component, architecture).String(), PackageTypeBinary})
+				packagesPaths = append(packagesPaths, []string{repo.BinaryPath(component, architecture), PackageTypeBinary})
 				if repo.DownloadUdebs {
-					packagesURLs = append(packagesURLs, []string{repo.UdebURL(component, architecture).String(), PackageTypeUdeb})
+					packagesPaths = append(packagesPaths, []string{repo.UdebPath(component, architecture), PackageTypeUdeb})
 				}
 			}
 			if repo.DownloadSources {
-				packagesURLs = append(packagesURLs, []string{repo.SourcesURL(component).String(), PackageTypeSource})
+				packagesPaths = append(packagesPaths, []string{repo.SourcesPath(component), PackageTypeSource})
 			}
 		}
 	}
 
-	for _, info := range packagesURLs {
+	for _, info := range packagesPaths {
-		url, kind := info[0], info[1]
+		path, kind := info[0], info[1]
-		packagesReader, packagesFile, err := http.DownloadTryCompression(d, url, repo.ReleaseFiles, ignoreMismatch, maxTries)
+		packagesReader, packagesFile, err := http.DownloadTryCompression(d, repo.IndexesRootURL(), path, repo.ReleaseFiles, ignoreMismatch, maxTries)
 		if err != nil {
 			return err
 		}

deb/remote_test.go

@@ -157,24 +157,30 @@ func (s *RemoteRepoSuite) TestReleaseURL(c *C) {
 	c.Assert(s.flat.ReleaseURL("Release").String(), Equals, "http://repos.express42.com/virool/precise/Release")
 }
 
-func (s *RemoteRepoSuite) TestBinaryURL(c *C) {
+func (s *RemoteRepoSuite) TestIndexesRootURL(c *C) {
-	c.Assert(s.repo.BinaryURL("main", "amd64").String(), Equals, "http://mirror.yandex.ru/debian/dists/squeeze/main/binary-amd64/Packages")
+	c.Assert(s.repo.IndexesRootURL().String(), Equals, "http://mirror.yandex.ru/debian/dists/squeeze/")
+
+	c.Assert(s.flat.IndexesRootURL().String(), Equals, "http://repos.express42.com/virool/precise/")
 }
 
-func (s *RemoteRepoSuite) TestUdebURL(c *C) {
+func (s *RemoteRepoSuite) TestBinaryPath(c *C) {
-	c.Assert(s.repo.UdebURL("main", "amd64").String(), Equals, "http://mirror.yandex.ru/debian/dists/squeeze/main/debian-installer/binary-amd64/Packages")
+	c.Assert(s.repo.BinaryPath("main", "amd64"), Equals, "main/binary-amd64/Packages")
 }
 
-func (s *RemoteRepoSuite) TestSourcesURL(c *C) {
+func (s *RemoteRepoSuite) TestUdebPath(c *C) {
-	c.Assert(s.repo.SourcesURL("main").String(), Equals, "http://mirror.yandex.ru/debian/dists/squeeze/main/source/Sources")
+	c.Assert(s.repo.UdebPath("main", "amd64"), Equals, "main/debian-installer/binary-amd64/Packages")
 }
 
-func (s *RemoteRepoSuite) TestFlatBinaryURL(c *C) {
+func (s *RemoteRepoSuite) TestSourcesPath(c *C) {
-	c.Assert(s.flat.FlatBinaryURL().String(), Equals, "http://repos.express42.com/virool/precise/Packages")
+	c.Assert(s.repo.SourcesPath("main"), Equals, "main/source/Sources")
 }
 
-func (s *RemoteRepoSuite) TestFlatSourcesURL(c *C) {
+func (s *RemoteRepoSuite) TestFlatBinaryPath(c *C) {
-	c.Assert(s.flat.FlatSourcesURL().String(), Equals, "http://repos.express42.com/virool/precise/Sources")
+	c.Assert(s.flat.FlatBinaryPath(), Equals, "Packages")
+}
+
+func (s *RemoteRepoSuite) TestFlatSourcesPath(c *C) {
+	c.Assert(s.flat.FlatSourcesPath(), Equals, "Sources")
 }
 
 func (s *RemoteRepoSuite) TestPackageURL(c *C) {

http/compression.go

@@ -5,6 +5,7 @@ import (
 	"compress/gzip"
 	"fmt"
 	"io"
+	"net/url"
 	"os"
 	"strings"
 
@@ -38,19 +39,19 @@ var compressionMethods = []struct {
 
 // DownloadTryCompression tries to download from URL .bz2, .gz and raw extension until
 // it finds existing file.
-func DownloadTryCompression(downloader aptly.Downloader, url string, expectedChecksums map[string]utils.ChecksumInfo, ignoreMismatch bool, maxTries int) (io.Reader, *os.File, error) {
+func DownloadTryCompression(downloader aptly.Downloader, baseURL *url.URL, path string, expectedChecksums map[string]utils.ChecksumInfo, ignoreMismatch bool, maxTries int) (io.Reader, *os.File, error) {
 	var err error
 
 	for _, method := range compressionMethods {
 		var file *os.File
 
-		tryURL := url + method.extenstion
+		tryPath := path + method.extenstion
 		foundChecksum := false
 
 		bestSuffix := ""
 
 		for suffix := range expectedChecksums {
-			if strings.HasSuffix(tryURL, suffix) {
+			if strings.HasSuffix(tryPath, suffix) {
 				foundChecksum = true
 				if len(suffix) > len(bestSuffix) {
 					bestSuffix = suffix
@@ -58,15 +59,17 @@ func DownloadTryCompression(downloader aptly.Downloader, url string, expectedChe
 			}
 		}
 
+		tryURL := baseURL.ResolveReference(&url.URL{Path: tryPath})
+
 		if foundChecksum {
 			expected := expectedChecksums[bestSuffix]
-			file, err = DownloadTempWithChecksum(downloader, tryURL, &expected, ignoreMismatch, maxTries)
+			file, err = DownloadTempWithChecksum(downloader, tryURL.String(), &expected, ignoreMismatch, maxTries)
 		} else {
 			if !ignoreMismatch {
 				continue
 			}
 
-			file, err = DownloadTemp(downloader, tryURL)
+			file, err = DownloadTemp(downloader, tryURL.String())
 		}
 
 		if err != nil {
@@ -86,7 +89,7 @@ func DownloadTryCompression(downloader aptly.Downloader, url string, expectedChe
 	}
 
 	if err == nil {
-		err = fmt.Errorf("no candidates for %s found", url)
+		err = fmt.Errorf("no candidates for %s found", baseURL.ResolveReference(&url.URL{Path: path}))
 	}
 	return nil, nil, err
 }

http/compression_test.go

@@ -3,13 +3,16 @@ package http
 import (
 	"errors"
 	"io"
+	"net/url"
 
 	"github.com/smira/aptly/utils"
 
 	. "gopkg.in/check.v1"
 )
 
-type CompressionSuite struct{}
+type CompressionSuite struct {
+	baseURL *url.URL
+}
 
 var _ = Suite(&CompressionSuite{})
 
@@ -20,6 +23,10 @@ const (
 	rawData  = "test"
 )
 
+func (s *CompressionSuite) SetUpTest(c *C) {
+	s.baseURL, _ = url.Parse("http://example.com/")
+}
+
 func (s *CompressionSuite) TestDownloadTryCompression(c *C) {
 	var buf []byte
 
@@ -34,7 +41,7 @@ func (s *CompressionSuite) TestDownloadTryCompression(c *C) {
 	buf = make([]byte, 4)
 	d := NewFakeDownloader()
 	d.ExpectResponse("http://example.com/file.bz2", bzipData)
-	r, file, err := DownloadTryCompression(d, "http://example.com/file", expectedChecksums, false, 1)
+	r, file, err := DownloadTryCompression(d, s.baseURL, "file", expectedChecksums, false, 1)
 	c.Assert(err, IsNil)
 	defer file.Close()
 	io.ReadFull(r, buf)
@@ -46,7 +53,7 @@ func (s *CompressionSuite) TestDownloadTryCompression(c *C) {
 	d = NewFakeDownloader()
 	d.ExpectError("http://example.com/file.bz2", &Error{Code: 404})
 	d.ExpectResponse("http://example.com/file.gz", gzipData)
-	r, file, err = DownloadTryCompression(d, "http://example.com/file", expectedChecksums, false, 1)
+	r, file, err = DownloadTryCompression(d, s.baseURL, "file", expectedChecksums, false, 1)
 	c.Assert(err, IsNil)
 	defer file.Close()
 	io.ReadFull(r, buf)
@@ -59,7 +66,7 @@ func (s *CompressionSuite) TestDownloadTryCompression(c *C) {
 	d.ExpectError("http://example.com/file.bz2", &Error{Code: 404})
 	d.ExpectError("http://example.com/file.gz", &Error{Code: 404})
 	d.ExpectResponse("http://example.com/file.xz", xzData)
-	r, file, err = DownloadTryCompression(d, "http://example.com/file", expectedChecksums, false, 1)
+	r, file, err = DownloadTryCompression(d, s.baseURL, "file", expectedChecksums, false, 1)
 	c.Assert(err, IsNil)
 	defer file.Close()
 	io.ReadFull(r, buf)
@@ -73,7 +80,7 @@ func (s *CompressionSuite) TestDownloadTryCompression(c *C) {
 	d.ExpectError("http://example.com/file.gz", &Error{Code: 404})
 	d.ExpectError("http://example.com/file.xz", &Error{Code: 404})
 	d.ExpectResponse("http://example.com/file", rawData)
-	r, file, err = DownloadTryCompression(d, "http://example.com/file", expectedChecksums, false, 1)
+	r, file, err = DownloadTryCompression(d, s.baseURL, "file", expectedChecksums, false, 1)
 	c.Assert(err, IsNil)
 	defer file.Close()
 	io.ReadFull(r, buf)
@@ -84,7 +91,7 @@ func (s *CompressionSuite) TestDownloadTryCompression(c *C) {
 	d = NewFakeDownloader()
 	d.ExpectError("http://example.com/file.bz2", &Error{Code: 404})
 	d.ExpectResponse("http://example.com/file.gz", "x")
-	_, _, err = DownloadTryCompression(d, "http://example.com/file", nil, true, 1)
+	_, _, err = DownloadTryCompression(d, s.baseURL, "file", nil, true, 1)
 	c.Assert(err, ErrorMatches, "unexpected EOF")
 	c.Assert(d.Empty(), Equals, true)
 }
@@ -102,7 +109,7 @@ func (s *CompressionSuite) TestDownloadTryCompressionLongestSuffix(c *C) {
 	buf = make([]byte, 4)
 	d := NewFakeDownloader()
 	d.ExpectResponse("http://example.com/subdir/file.bz2", bzipData)
-	r, file, err := DownloadTryCompression(d, "http://example.com/subdir/file", expectedChecksums, false, 1)
+	r, file, err := DownloadTryCompression(d, s.baseURL, "subdir/file", expectedChecksums, false, 1)
 	c.Assert(err, IsNil)
 	defer file.Close()
 	io.ReadFull(r, buf)
@@ -112,7 +119,7 @@ func (s *CompressionSuite) TestDownloadTryCompressionLongestSuffix(c *C) {
 
 func (s *CompressionSuite) TestDownloadTryCompressionErrors(c *C) {
 	d := NewFakeDownloader()
-	_, _, err := DownloadTryCompression(d, "http://example.com/file", nil, true, 1)
+	_, _, err := DownloadTryCompression(d, s.baseURL, "file", nil, true, 1)
 	c.Assert(err, ErrorMatches, "unexpected request.*")
 
 	d = NewFakeDownloader()
@@ -120,7 +127,7 @@ func (s *CompressionSuite) TestDownloadTryCompressionErrors(c *C) {
 	d.ExpectError("http://example.com/file.gz", &Error{Code: 404})
 	d.ExpectError("http://example.com/file.xz", &Error{Code: 404})
 	d.ExpectError("http://example.com/file", errors.New("403"))
-	_, _, err = DownloadTryCompression(d, "http://example.com/file", nil, true, 1)
+	_, _, err = DownloadTryCompression(d, s.baseURL, "file", nil, true, 1)
 	c.Assert(err, ErrorMatches, "403")
 
 	d = NewFakeDownloader()
@@ -134,6 +141,6 @@ func (s *CompressionSuite) TestDownloadTryCompressionErrors(c *C) {
 		"file.xz":  {Size: 7},
 		"file":     {Size: 7},
 	}
-	_, _, err = DownloadTryCompression(d, "http://example.com/file", expectedChecksums, false, 1)
+	_, _, err = DownloadTryCompression(d, s.baseURL, "file", expectedChecksums, false, 1)
 	c.Assert(err, ErrorMatches, "checksums don't match.*")
 }