From cafb89f30f0507e8c093eb33c300a52c535d899c Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Mon, 22 May 2017 23:58:47 +0300 Subject: [PATCH] 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 | 67 ++++++++++++++++++++-------------------- deb/remote_test.go | 26 ++++++++++------ http/compression.go | 15 +++++---- http/compression_test.go | 27 ++++++++++------ 4 files changed, 75 insertions(+), 60 deletions(-) diff --git a/deb/remote.go b/deb/remote.go index ed87fa87..d631becf 100644 --- a/deb/remote.go +++ b/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 -func (repo *RemoteRepo) ReleaseURL(name string) *url.URL { +// IndexesRootURL builds URL for various indexes +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 -func (repo *RemoteRepo) FlatBinaryURL() *url.URL { - path := &url.URL{Path: filepath.Join(repo.Distribution, "Packages")} - return repo.archiveRootURL.ResolveReference(path) +// ReleaseURL returns URL to Release* files in repo root +func (repo *RemoteRepo) ReleaseURL(name string) *url.URL { + return repo.IndexesRootURL().ResolveReference(&url.URL{Path: name}) } -// FlatSourcesURL returns URL to Sources files for flat repo -func (repo *RemoteRepo) FlatSourcesURL() *url.URL { - path := &url.URL{Path: filepath.Join(repo.Distribution, "Sources")} - return repo.archiveRootURL.ResolveReference(path) +// FlatBinaryPath returns path to Packages files for flat repo +func (repo *RemoteRepo) FlatBinaryPath() string { + return "Packages" } -// 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 { - path := &url.URL{Path: fmt.Sprintf("dists/%s/%s/binary-%s/Packages", repo.Distribution, component, architecture)} - return repo.archiveRootURL.ResolveReference(path) +func (repo *RemoteRepo) BinaryPath(component string, architecture string) string { + return fmt.Sprintf("%s/binary-%s/Packages", component, architecture) } -// SourcesURL returns URL of Sources files for given component -func (repo *RemoteRepo) SourcesURL(component string) *url.URL { - path := &url.URL{Path: fmt.Sprintf("dists/%s/%s/source/Sources", repo.Distribution, component)} - return repo.archiveRootURL.ResolveReference(path) +// SourcesPath returns path to Sources files for given component +func (repo *RemoteRepo) SourcesPath(component string) string { + return fmt.Sprintf("%s/source/Sources", component) } -// 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 { - path := &url.URL{Path: fmt.Sprintf("dists/%s/%s/debian-installer/binary-%s/Packages", repo.Distribution, component, architecture)} - return repo.archiveRootURL.ResolveReference(path) +func (repo *RemoteRepo) UdebPath(component string, architecture string) string { + return fmt.Sprintf("%s/debian-installer/binary-%s/Packages", component, architecture) } // 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 { - url, kind := info[0], info[1] - packagesReader, packagesFile, err := http.DownloadTryCompression(d, url, repo.ReleaseFiles, ignoreMismatch, maxTries) + for _, info := range packagesPaths { + path, kind := info[0], info[1] + packagesReader, packagesFile, err := http.DownloadTryCompression(d, repo.IndexesRootURL(), path, repo.ReleaseFiles, ignoreMismatch, maxTries) if err != nil { return err } diff --git a/deb/remote_test.go b/deb/remote_test.go index 5dae0af8..974ca56c 100644 --- a/deb/remote_test.go +++ b/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) { - c.Assert(s.repo.BinaryURL("main", "amd64").String(), Equals, "http://mirror.yandex.ru/debian/dists/squeeze/main/binary-amd64/Packages") +func (s *RemoteRepoSuite) TestIndexesRootURL(c *C) { + 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) { - c.Assert(s.repo.UdebURL("main", "amd64").String(), Equals, "http://mirror.yandex.ru/debian/dists/squeeze/main/debian-installer/binary-amd64/Packages") +func (s *RemoteRepoSuite) TestBinaryPath(c *C) { + c.Assert(s.repo.BinaryPath("main", "amd64"), Equals, "main/binary-amd64/Packages") } -func (s *RemoteRepoSuite) TestSourcesURL(c *C) { - c.Assert(s.repo.SourcesURL("main").String(), Equals, "http://mirror.yandex.ru/debian/dists/squeeze/main/source/Sources") +func (s *RemoteRepoSuite) TestUdebPath(c *C) { + c.Assert(s.repo.UdebPath("main", "amd64"), Equals, "main/debian-installer/binary-amd64/Packages") } -func (s *RemoteRepoSuite) TestFlatBinaryURL(c *C) { - c.Assert(s.flat.FlatBinaryURL().String(), Equals, "http://repos.express42.com/virool/precise/Packages") +func (s *RemoteRepoSuite) TestSourcesPath(c *C) { + c.Assert(s.repo.SourcesPath("main"), Equals, "main/source/Sources") } -func (s *RemoteRepoSuite) TestFlatSourcesURL(c *C) { - c.Assert(s.flat.FlatSourcesURL().String(), Equals, "http://repos.express42.com/virool/precise/Sources") +func (s *RemoteRepoSuite) TestFlatBinaryPath(c *C) { + 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) { diff --git a/http/compression.go b/http/compression.go index 835e0a40..b0eab556 100644 --- a/http/compression.go +++ b/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 } diff --git a/http/compression_test.go b/http/compression_test.go index fbf46b1e..5dacc0dc 100644 --- a/http/compression_test.go +++ b/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.*") }