From 0ad4c1be2fc62c7369ea5083c8922c0bdb3f6839 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Fri, 3 Apr 2020 12:11:57 -0700 Subject: [PATCH] internal/getproviders: Tidy up some confusion about package hashes Earlier on in the stubbing of this package we realized that it wasn't going to be possible to populate the authentication-related bits for all packages because the relevant metadata just isn't available for packages that are already local. However, we just moved ahead with that awkward design at the time because we needed to get other work done, and so we've been mostly producing PackageMeta values with all-zeros hashes and just ignoring them entirely as a temporary workaround. This is a first step towards what is hopefully a more intuitive model: authentication is an optional thing in a PackageMeta that is currently populated only for packages coming from a registry. So far this still just models checking a SHA256 hash, which is not a sufficient set of checks for a real release but hopefully the "real" implementation is a natural iteration of this starting point, and if not then at least this interim step is a bit more honest about the fact that Authentication will not be populated on every PackageMeta. --- .../getproviders/package_authentication.go | 93 +++++++++++++++++++ internal/getproviders/registry_client.go | 9 +- internal/getproviders/registry_source_test.go | 2 +- internal/getproviders/types.go | 22 +++-- internal/providercache/dir_modify.go | 5 + 5 files changed, 118 insertions(+), 13 deletions(-) create mode 100644 internal/getproviders/package_authentication.go diff --git a/internal/getproviders/package_authentication.go b/internal/getproviders/package_authentication.go new file mode 100644 index 0000000000..7aba5eb0f2 --- /dev/null +++ b/internal/getproviders/package_authentication.go @@ -0,0 +1,93 @@ +package getproviders + +import ( + "bytes" + "crypto/sha256" + "fmt" + "io" + "os" +) + +// PackageAuthentication is an interface implemented by the optional package +// authentication implementations a source may include on its PackageMeta +// objects. +// +// A PackageAuthentication implementation is responsible for authenticating +// that a package is what its distributor intended to distribute and that it +// has not been tampered with. +type PackageAuthentication interface { + // AuthenticatePackage takes the metadata about the package as returned + // by its original source, and also the "localLocation" where it has + // been staged for local inspection (which may or may not be the same + // as the original source location) and returns an error if the + // authentication checks fail. + // + // The localLocation is guaranteed not to be a PackageHTTPURL: a + // remote package will always be staged locally for inspection first. + AuthenticatePackage(meta PackageMeta, localLocation PackageLocation) error +} + +type packageAuthenticationAll []PackageAuthentication + +// PackageAuthenticationAll combines several authentications together into a +// single check value, which passes only if all of the given ones pass. +// +// The checks are processed in the order given, so a failure of an earlier +// check will prevent execution of a later one. +func PackageAuthenticationAll(checks ...PackageAuthentication) PackageAuthentication { + return packageAuthenticationAll(checks) +} + +func (checks packageAuthenticationAll) AuthenticatePackage(meta PackageMeta, localLocation PackageLocation) error { + for _, check := range checks { + err := check.AuthenticatePackage(meta, localLocation) + if err != nil { + return err + } + } + return nil +} + +type archiveHashAuthentication struct { + WantSHA256Sum [sha256.Size]byte +} + +// NewArchiveChecksumAuthentication returns a PackageAuthentication +// implementation that checks that the original distribution archive matches +// the given hash. +// +// This authentication is suitable only for PackageHTTPURL and +// PackageLocalArchive source locations, because the unpacked layout +// (represented by PackageLocalDir) does not retain access to the original +// source archive. Therefore this authenticator will return an error if its +// given localLocation is not PackageLocalArchive. +func NewArchiveChecksumAuthentication(wantSHA256Sum [sha256.Size]byte) PackageAuthentication { + return archiveHashAuthentication{wantSHA256Sum} +} + +func (a archiveHashAuthentication) AuthenticatePackage(meta PackageMeta, localLocation PackageLocation) error { + archiveLocation, ok := localLocation.(PackageLocalArchive) + if !ok { + // A source should not use this authentication type for non-archive + // locations. + return fmt.Errorf("cannot check archive hash for non-archive location %s", localLocation) + } + + f, err := os.Open(string(archiveLocation)) + if err != nil { + return err + } + defer f.Close() + + h := sha256.New() + _, err = io.Copy(h, f) + if err != nil { + return err + } + + gotHash := h.Sum(nil) + if !bytes.Equal(gotHash, a.WantSHA256Sum[:]) { + return fmt.Errorf("archive has incorrect SHA-256 checksum %x (expected %x)", gotHash, a.WantSHA256Sum[:]) + } + return nil +} diff --git a/internal/getproviders/registry_client.go b/internal/getproviders/registry_client.go index a1d1532d71..5e021beddb 100644 --- a/internal/getproviders/registry_client.go +++ b/internal/getproviders/registry_client.go @@ -1,6 +1,7 @@ package getproviders import ( + "crypto/sha256" "encoding/hex" "encoding/json" "errors" @@ -220,22 +221,24 @@ func (c *registryClient) PackageMeta(provider addrs.Provider, version Version, t }, Filename: body.Filename, Location: PackageHTTPURL(downloadURL.String()), - // SHA256Sum is populated below + // "Authentication" is populated below } - if len(body.SHA256Sum) != len(ret.SHA256Sum)*2 { + if len(body.SHA256Sum) != sha256.Size*2 { // *2 because it's hex-encoded return PackageMeta{}, c.errQueryFailed( provider, fmt.Errorf("registry response includes invalid SHA256 hash %q: %s", body.SHA256Sum, err), ) } - _, err = hex.Decode(ret.SHA256Sum[:], []byte(body.SHA256Sum)) + var checksum [sha256.Size]byte + _, err = hex.Decode(checksum[:], []byte(body.SHA256Sum)) if err != nil { return PackageMeta{}, c.errQueryFailed( provider, fmt.Errorf("registry response includes invalid SHA256 hash %q: %s", body.SHA256Sum, err), ) } + ret.Authentication = NewArchiveChecksumAuthentication(checksum) return ret, nil } diff --git a/internal/getproviders/registry_source_test.go b/internal/getproviders/registry_source_test.go index 377bac245c..12e05ac42a 100644 --- a/internal/getproviders/registry_source_test.go +++ b/internal/getproviders/registry_source_test.go @@ -125,7 +125,7 @@ func TestSourcePackageMeta(t *testing.T) { TargetPlatform: Platform{"linux", "amd64"}, Filename: "happycloud_1.2.0.zip", Location: PackageHTTPURL(baseURL + "/pkg/happycloud_1.2.0.zip"), - SHA256Sum: [32]uint8{30: 0xf0, 31: 0x0d}, // fake registry uses a memorable sum + Authentication: archiveHashAuthentication{[32]uint8{30: 0xf0, 31: 0x0d}}, // fake registry uses a memorable sum }, ``, }, diff --git a/internal/getproviders/types.go b/internal/getproviders/types.go index 137fd8adac..3ee0d817f2 100644 --- a/internal/getproviders/types.go +++ b/internal/getproviders/types.go @@ -1,7 +1,6 @@ package getproviders import ( - "crypto/sha256" "fmt" "runtime" "sort" @@ -170,14 +169,19 @@ type PackageMeta struct { Filename string Location PackageLocation - // FIXME: Our current hashing scheme only works for sources that have - // access to the original distribution archives, so this isn't always - // populated. Need to figure out a different approach where we can - // consistently hash both from an archive file and from an extracted - // archive to detect inconsistencies. - SHA256Sum [sha256.Size]byte - - // TODO: Extra metadata for signature verification + // Authentication, if non-nil, is a request from the source that produced + // this meta for verification of the target package after it has been + // retrieved from the indicated Location. + // + // Different sources will support different authentication strategies -- + // or possibly no strategies at all -- depending on what metadata they + // have available to them, such as checksums provided out-of-band by the + // original package author, expected signing keys, etc. + // + // If Authentication is non-nil then no authentication is requested. + // This is likely appropriate only for packages that are already available + // on the local system. + Authentication PackageAuthentication } // LessThan returns true if the receiver should sort before the given other diff --git a/internal/providercache/dir_modify.go b/internal/providercache/dir_modify.go index 9441f93fe5..5b6fd2311a 100644 --- a/internal/providercache/dir_modify.go +++ b/internal/providercache/dir_modify.go @@ -23,6 +23,11 @@ func (d *Dir) InstallPackage(ctx context.Context, meta getproviders.PackageMeta) // incorporate any changes we make here. d.metaCache = nil + // TODO: If meta.Authentication is non-nil, we should call it at some point + // in the rest of this process (perhaps inside installFromLocalArchive and + // installFromLocalDir, so we already have the local copy?) and return an + // error if the authentication fails. + log.Printf("[TRACE] providercache.Dir.InstallPackage: installing %s v%s from %s", meta.Provider, meta.Version, meta.Location) switch location := meta.Location.(type) { case getproviders.PackageHTTPURL: