From e6e0b6ee467f37b547602364e17c49bc4f1eb7e9 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Tue, 27 Oct 2020 16:44:37 -0700 Subject: [PATCH] providercache: verify locked hashes for local package dirs Previously we were only verifying locked hashes for local archive zip files, but if we have non-ziphash hashes available then we can and should also verify that a local directory matches at least one of them. This does mean that folks using filesystem mirrors but yet also running Terraform across multiple platforms will need to take some extra care to ensure the hashes pass on all relevant platforms, which could mean using "terraform providers lock" to pre-seed their lock files with hashes across all platforms, or could mean using the "packed" directory layout for the filesystem mirror so that Terraform will end up in the install-from-archive codepath instead of this install-from-directory codepath, and can thus verify ziphash too. (There's no additional documentation about the above here because there's already general information about this in the lock file documentation due to some similar -- though not identical -- situations with network mirrors.) --- internal/providercache/installer_test.go | 23 +++--------- internal/providercache/package_install.go | 46 +++++++++++++++++++++++ 2 files changed, 52 insertions(+), 17 deletions(-) diff --git a/internal/providercache/installer_test.go b/internal/providercache/installer_test.go index c0598c6ee..fb49bec22 100644 --- a/internal/providercache/installer_test.go +++ b/internal/providercache/installer_test.go @@ -1085,6 +1085,8 @@ func TestEnsureProviderVersions(t *testing.T) { Reqs: getproviders.Requirements{ beepProvider: getproviders.MustParseVersionConstraints(">= 1.0.0"), }, + WantErr: `some providers could not be installed: +- example.com/foo/beep: the local package for example.com/foo/beep 1.0.0 doesn't match any of the checksums previously recorded in the dependency lock file (this might be because the available checksums are for packages targeting different platforms)`, WantEvents: func(inst *Installer, dir *Dir) map[addrs.Provider][]*testInstallerEventLogItem { return map[addrs.Provider][]*testInstallerEventLogItem{ noProvider: { @@ -1094,12 +1096,6 @@ func TestEnsureProviderVersions(t *testing.T) { beepProvider: getproviders.MustParseVersionConstraints(">= 1.0.0"), }, }, - { - Event: "ProvidersFetched", - Args: map[addrs.Provider]*getproviders.PackageAuthenticationResult{ - beepProvider: nil, - }, - }, }, beepProvider: { { @@ -1129,21 +1125,14 @@ func TestEnsureProviderVersions(t *testing.T) { }{"1.0.0", beepProviderDir}, }, { - // FIXME: This ending in success with "unauthenticated" - // is technically okay within the interface as stated - // but doesn't really match our intent of treating - // a mismatch error against the lockfile as - // an error. We should make this an error in future. - Event: "FetchPackageSuccess", + Event: "FetchPackageFailure", Provider: beepProvider, Args: struct { - Version string - LocalDir string - AuthResult string + Version string + Error string }{ "1.0.0", - filepath.Join(dir.BasePath(), "example.com/foo/beep/1.0.0/bleep_bloop"), - "unauthenticated", + `the local package for example.com/foo/beep 1.0.0 doesn't match any of the checksums previously recorded in the dependency lock file (this might be because the available checksums are for packages targeting different platforms)`, }, }, }, diff --git a/internal/providercache/package_install.go b/internal/providercache/package_install.go index ca8a3073d..ec76e7014 100644 --- a/internal/providercache/package_install.go +++ b/internal/providercache/package_install.go @@ -160,6 +160,52 @@ func installFromLocalDir(ctx context.Context, meta getproviders.PackageMeta, tar return nil, fmt.Errorf("failed to determine if %s and %s are the same: %s", sourceDir, targetDir, err) } + var authResult *getproviders.PackageAuthenticationResult + if meta.Authentication != nil { + // (we have this here for completeness but note that local filesystem + // mirrors typically don't include enough information for package + // authentication and so we'll rarely get in here in practice.) + var err error + if authResult, err = meta.Authentication.AuthenticatePackage(meta.Location); err != nil { + return nil, err + } + } + + // If the caller provided at least one hash in allowedHashes then at + // least one of those hashes ought to match. However, for local directories + // in particular we can't actually verify the legacy "zh:" hash scheme + // because it requires access to the original .zip archive, and so as a + // measure of pragmatism we'll treat a set of hashes where all are "zh:" + // the same as no hashes at all, and let anything pass. This is definitely + // non-ideal but accepted for two reasons: + // - Packages we find on local disk can be considered a little more trusted + // than packages coming from over the network, because we assume that + // they were either placed intentionally by an operator or they were + // automatically installed by a previous network operation that would've + // itself verified the hashes. + // - Our installer makes a concerted effort to record at least one new-style + // hash for each lock entry, so we should very rarely end up in this + // situation anyway. + suitableHashCount := 0 + for _, hash := range allowedHashes { + if !hash.HasScheme(getproviders.HashSchemeZip) { + suitableHashCount++ + } + } + if suitableHashCount > 0 { + if matches, err := meta.MatchesAnyHash(allowedHashes); err != nil { + return authResult, fmt.Errorf( + "failed to calculate checksum for %s %s package at %s: %s", + meta.Provider, meta.Version, meta.Location, err, + ) + } else if !matches { + return authResult, fmt.Errorf( + "the local package for %s %s doesn't match any of the checksums previously recorded in the dependency lock file (this might be because the available checksums are for packages targeting different platforms)", + meta.Provider, meta.Version, + ) + } + } + // Delete anything that's already present at this path first. err = os.RemoveAll(targetDir) if err != nil && !os.IsNotExist(err) {