From 55855fca706ef00aaf196b764ce2d181973618a3 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Fri, 21 Mar 2025 17:25:25 -0700 Subject: [PATCH] getproviders: Unify package authentication with hash lock selection As discussed in opentofu/opentofu#2656, this consolidates the two concerns of the PackageAuthentication interface into a single function that deals both with package authentication _and_ with reporting all of the package hashes that were used to make the authentication decision. This means that any .zip archive that OpenTofu directly verifies during installation can now have its hash recorded in the dependency lock file even if that package didn't come from the provider's origin registry, which is beneficial when the first installation of a provider comes from a secondary ("mirror") source because it creates an additional hook by which that dependency lock file entry can be "upgraded" to be complete in a future "tofu init" run against the origin registry, or by the "tofu providers lock" command. Signed-off-by: Martin Atkins --- CHANGELOG.md | 1 + internal/command/init.go | 2 +- internal/command/init_test.go | 15 +- internal/command/providers_lock.go | 2 +- .../filesystem_mirror_source_test.go | 4 - internal/getproviders/hash.go | 251 +++++++++-- internal/getproviders/hash_test.go | 251 +++++++++++ .../getproviders/http_mirror_source_test.go | 6 - .../oci_registry_mirror_source_test.go | 2 +- .../getproviders/package_authentication.go | 311 ++++++++------ .../package_authentication_helper_test.go | 83 ++++ .../package_authentication_test.go | 390 +++++++++++------- .../package_location_oci_blob_archive_test.go | 7 +- internal/getproviders/registry_client.go | 2 +- internal/getproviders/registry_source_test.go | 24 +- internal/getproviders/types.go | 33 -- internal/providercache/installer.go | 51 ++- internal/providercache/installer_test.go | 184 ++++++++- 18 files changed, 1235 insertions(+), 384 deletions(-) create mode 100644 internal/getproviders/package_authentication_helper_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b61fa2b08a..c49ba10f10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ ENHANCEMENTS: * Provider instance keys now automatically converted to string ([#2378](https://github.com/opentofu/opentofu/issues/2378)) * Remove progress messages from commands using -concise argument ([#2549](https://github.com/opentofu/opentofu/issues/2549)) * Upgrade aws-sdk version to include `mx-central-1` region. ([#2596](https://github.com/opentofu/opentofu/pull/2596)) +* When installing a provider from a source that offers a `.zip` archive of a provider package but that cannot also offer a signed set of official checksums for the provider, OpenTofu will now include its locally-verified zip archive checksum (`zh:` scheme) in the dependency lock file in addition to the package contents checksum (`h1:` checksum) previously recorded. This makes it more likely that a future reinstall of the same package from a different source will be verified successfully. ([#2656](https://github.com/opentofu/opentofu/pull/2656)) BUG FIXES: diff --git a/internal/command/init.go b/internal/command/init.go index ff04c885a6..5a65057044 100644 --- a/internal/command/init.go +++ b/internal/command/init.go @@ -880,7 +880,7 @@ func (c *InitCommand) getProviders(ctx context.Context, config *configs.Config, FetchPackageSuccess: func(provider addrs.Provider, version getproviders.Version, localDir string, authResult *getproviders.PackageAuthenticationResult) { var keyID string if authResult != nil && authResult.Signed() { - keyID = authResult.KeyID + keyID = authResult.GPGKeyIDsString() } if keyID != "" { keyID = c.Colorize().Color(fmt.Sprintf(", key ID [reset][bold]%s[reset]", keyID)) diff --git a/internal/command/init_test.go b/internal/command/init_test.go index 365368ccc1..0b5a9d818c 100644 --- a/internal/command/init_test.go +++ b/internal/command/init_test.go @@ -1778,6 +1778,7 @@ func TestInit_providerSource(t *testing.T) { getproviders.MustParseVersionConstraints("= 1.2.4"), []getproviders.Hash{ getproviders.HashScheme1.New("vEthLkqAecdQimaW6JHZ0SBRNtHibLnOb31tX9ZXlcI="), + getproviders.HashSchemeZip.New("ec7c3fd6eb575c06f0e6957e1ee8531a588805c4eeb8abb5e4156911e080eb31"), }, ), addrs.NewDefaultProvider("test"): depsfile.NewProviderLock( @@ -1786,6 +1787,7 @@ func TestInit_providerSource(t *testing.T) { getproviders.MustParseVersionConstraints("= 1.2.3"), []getproviders.Hash{ getproviders.HashScheme1.New("8CjxaUBuegKZSFnRos39Fs+CS78ax0Dyb7aIA5XBiNI="), + getproviders.HashSchemeZip.New("6f85a1f747dd09455cd77683c0e06da647d8240461b8b36b304b9056814d91f2"), }, ), addrs.NewDefaultProvider("source"): depsfile.NewProviderLock( @@ -1794,6 +1796,7 @@ func TestInit_providerSource(t *testing.T) { getproviders.MustParseVersionConstraints("= 1.2.3"), []getproviders.Hash{ getproviders.HashScheme1.New("ACYytVQ2Q6JfoEs7xxCqa1yGFf9HwF3SEHzJKBoJfo0="), + getproviders.HashSchemeZip.New("69f700dbf9eda586abef22ab08e3a3896760e01885f6cbda4460ceeca4e3c0ba"), }, ), } @@ -1805,6 +1808,10 @@ func TestInit_providerSource(t *testing.T) { if got, want := ui.OutputWriter.String(), "Installed hashicorp/test v1.2.3 (verified checksum)"; !strings.Contains(got, want) { t.Fatalf("unexpected output: %s\nexpected to include %q", got, want) } + // On stderr we should've written a warning about the dependency lock file + // entry being incomplete for these three providers, because we installed + // from a non-origin-registry source and so registry-promised hashes + // are not available. if got, want := ui.ErrorWriter.String(), "\n - hashicorp/source\n - hashicorp/test\n - hashicorp/test-beta"; !strings.Contains(got, want) { t.Fatalf("wrong error message\nshould contain: %s\ngot:\n%s", want, got) } @@ -1994,6 +2001,7 @@ func TestInit_getUpgradePlugins(t *testing.T) { getproviders.MustParseVersionConstraints("> 1.0.0, < 3.0.0"), []getproviders.Hash{ getproviders.HashScheme1.New("ntfa04OlRqIfGL/Gkd+nGMJSHGWyAgMQplFWk7WEsOk="), + getproviders.HashSchemeZip.New("29e1045215056680ac59fe95554f0eb1323534a3d411aae2a7a04495ac884258"), }, ), addrs.NewDefaultProvider("exact"): depsfile.NewProviderLock( @@ -2002,6 +2010,7 @@ func TestInit_getUpgradePlugins(t *testing.T) { getproviders.MustParseVersionConstraints("= 1.2.3"), []getproviders.Hash{ getproviders.HashScheme1.New("Xgk+LFrzi9Mop6+d01TCTaD3kgSrUASCAUU1aDsEsJU="), + getproviders.HashSchemeZip.New("9cb7a3006b9c1344b2d838a5bb03c1e0f04b8c046beb38901eaf3cc99fceb870"), }, ), addrs.NewDefaultProvider("greater-than"): depsfile.NewProviderLock( @@ -2010,6 +2019,7 @@ func TestInit_getUpgradePlugins(t *testing.T) { getproviders.MustParseVersionConstraints(">= 2.3.3"), []getproviders.Hash{ getproviders.HashScheme1.New("8M5DXICmUiVjbkxNNO0zXNsV6duCVNWzq3/Kf0mNIo4="), + getproviders.HashSchemeZip.New("bfb683ee94027efb191986484352ada8219cd45e856d25c2ddcb489e100a9a02"), }, ), } @@ -2176,8 +2186,8 @@ func TestInit_providerLockFile(t *testing.T) { t.Fatalf("failed to read dependency lock file %s: %s", lockFile, err) } buf = bytes.TrimSpace(buf) - // The hash in here is for the fake package that newMockProviderSource produces - // (so it'll change if newMockProviderSource starts producing different contents) + // The hashes in here are for the fake package that newMockProviderSource produces + // (so they'll change if newMockProviderSource starts producing different contents) wantLockFile := strings.TrimSpace(` # This file is maintained automatically by "tofu init". # Manual edits may be lost in future updates. @@ -2187,6 +2197,7 @@ provider "registry.opentofu.org/hashicorp/test" { constraints = "1.2.3" hashes = [ "h1:8CjxaUBuegKZSFnRos39Fs+CS78ax0Dyb7aIA5XBiNI=", + "zh:6f85a1f747dd09455cd77683c0e06da647d8240461b8b36b304b9056814d91f2", ] } `) diff --git a/internal/command/providers_lock.go b/internal/command/providers_lock.go index 3b1554f3b4..b7af46d176 100644 --- a/internal/command/providers_lock.go +++ b/internal/command/providers_lock.go @@ -235,7 +235,7 @@ func (c *ProvidersLockCommand) Run(args []string) int { FetchPackageSuccess: func(provider addrs.Provider, version getproviders.Version, localDir string, auth *getproviders.PackageAuthenticationResult) { var keyID string if auth != nil && auth.Signed() { - keyID = auth.KeyID + keyID = auth.GPGKeyIDsString() } if keyID != "" { keyID = c.Colorize().Color(fmt.Sprintf(", key ID [reset][bold]%s[reset]", keyID)) diff --git a/internal/getproviders/filesystem_mirror_source_test.go b/internal/getproviders/filesystem_mirror_source_test.go index 24ecd899f1..92d6654c2c 100644 --- a/internal/getproviders/filesystem_mirror_source_test.go +++ b/internal/getproviders/filesystem_mirror_source_test.go @@ -174,10 +174,6 @@ func TestFilesystemMirrorSourcePackageMeta(t *testing.T) { if diff := cmp.Diff(want, got); diff != "" { t.Errorf("incorrect result\n%s", diff) } - - if gotHashes := got.AcceptableHashes(); len(gotHashes) != 0 { - t.Errorf("wrong acceptable hashes\ngot: %#v\nwant: none", gotHashes) - } }) t.Run("unavailable platform", func(t *testing.T) { source := NewFilesystemMirrorSource("testdata/filesystem-mirror") diff --git a/internal/getproviders/hash.go b/internal/getproviders/hash.go index 3624f3caf0..ec4d112b16 100644 --- a/internal/getproviders/hash.go +++ b/internal/getproviders/hash.go @@ -9,10 +9,15 @@ import ( "crypto/sha256" "fmt" "io" + "iter" + "maps" "os" "path/filepath" + "slices" + "sort" "strings" + "github.com/opentofu/opentofu/internal/collections" "golang.org/x/mod/sumdb/dirhash" ) @@ -219,47 +224,75 @@ func PackageMatchesHash(loc PackageLocation, want Hash) (bool, error) { // contents of the indicated package in order to compute the hash. If given // a non-local location this function will always return an error. func PackageMatchesAnyHash(loc PackageLocation, allowed []Hash) (bool, error) { + for _, err := range HashesMatchingPackage(loc, allowed) { + if err != nil { + return false, err + } + // If we get at least one non-error iteration then that's + // enough to answer this question. + return true, nil + } + // If we don't get at least one non-error iteration then + // we know that none of the hashes matched. + return false, nil +} + +// HashesMatchingPackage compares the given package location to each +// of the given hashes in turn and returns a sequence of only those +// that match the package. +// +// Each iteration can potentially fail due to being unable to read +// from the given location, in which case it will yield NilHash and +// an error and no further iteration will be possible. +func HashesMatchingPackage(loc PackageLocation, toTest []Hash) iter.Seq2[Hash, error] { // It's likely that we'll have multiple hashes of the same scheme in // the "allowed" set, in which case we'll avoid repeatedly re-reading the // given package by caching its result for each of the two // currently-supported hash formats. These will be NilHash until we // encounter the first hash of the corresponding scheme. var v1Hash, zipHash Hash - for _, want := range allowed { - switch want.Scheme() { - case HashScheme1: - if v1Hash == NilHash { - got, err := PackageHashV1(loc) - if err != nil { - return false, err + return func(yield func(Hash, error) bool) { + for _, want := range toTest { + switch want.Scheme() { + case HashScheme1: + if v1Hash == NilHash { + got, err := PackageHashV1(loc) + if err != nil { + yield(NilHash, err) + return + } + v1Hash = got } - v1Hash = got - } - if v1Hash == want { - return true, nil - } - case HashSchemeZip: - archiveLoc, ok := loc.(PackageLocalArchive) - if !ok { - // A zip hash can never match an unpacked directory + if v1Hash == want { + if keepGoing := yield(want, nil); !keepGoing { + return + } + } + case HashSchemeZip: + archiveLoc, ok := loc.(PackageLocalArchive) + if !ok { + // A zip hash can never match an unpacked directory + continue + } + if zipHash == NilHash { + got, err := PackageHashLegacyZipSHA(archiveLoc) + if err != nil { + yield(NilHash, err) + return + } + zipHash = got + } + if zipHash == want { + if keepGoing := yield(want, nil); !keepGoing { + return + } + } + default: + // If it's not a supported format then it can't match. continue } - if zipHash == NilHash { - got, err := PackageHashLegacyZipSHA(archiveLoc) - if err != nil { - return false, err - } - zipHash = got - } - if zipHash == want { - return true, nil - } - default: - // If it's not a supported format then it can't match. - continue } } - return false, nil } // PreferredHashes examines all of the given hash strings and returns the one @@ -454,3 +487,161 @@ func (m PackageMeta) MatchesAnyHash(acceptable []Hash) (bool, error) { func (m PackageMeta) HashV1() (Hash, error) { return PackageHashV1(m.Location) } + +// HashDisposition describes in what way a particular hash is related to +// a particular [PackageAuthenticationResult], and thus what a caller might +// be able to assume about the trustworthiness of that hash. +type HashDisposition struct { + // SignedByGPGKeyIDs is a set of GPG key IDs which provided signatures + // covering the associated hash. + // + // A hash that has at least one signing key ID but was not otherwise + // verified (as indicated by the other fields of this type) should be + // trusted only if at least one of the given signing keys is trusted. + // It's the responsibility of any subsystem relying on this information + // to define what it means for a key to be "trusted". + SignedByGPGKeyIDs collections.Set[string] + + // ReportedByRegistry is set if this hash was reported by the associated + // provider's origin registry as being one of the official hashes for + // this provider release. + // + // Note that this signal alone is only trustworthy to the extent that + // the origin registry is trusted. Unless there is also at least one + // entry in SignedByGPGKeyIDs, this hash was not covered by any GPG + // signing key. + // + // This field must be set only by a source that directly interacted + // with the associated provider's origin registry. It MUST NOT be + // set by a source that interacts with any sort of "mirror". + ReportedByRegistry bool + + // VerifiedLocally is set for any hash that was calculated locally, + // directly from a package for the provider that the hash is intended + // to verify. + // + // Note that this represents only that the artifact matched the + // associated hash; it does NOT mean that the associated hash is + // one of the official hashes as designated by the provider developer, + // unless the provider developer's signing key also appears in + // SignedByGPGKeyIDs. + VerifiedLocally bool +} + +// SignedByAnyGPGKeys returns true if the reciever has at least one GPG key +// ID that signed an assertion that the associated hash is valid for the +// associated provider version. +// +// Note that relying _only_ on the result of this function to make a trust +// decision implies that the caller considers all keys to be equally +// trustworthy, which is probably a risky assumption! +func (d HashDisposition) SignedByAnyGPGKeys() bool { + return len(d.SignedByGPGKeyIDs) != 0 +} + +// GPGSigningKeysString returns a string representation of any GPG signing +// key IDs that signed an assertion that the associated hash is valid for the +// associated provider version. +// +// If there are no such keys then the result is an empty string. +// +// The result of this is intended for display to a human in the UI, rather +// than for machine-readable purposes. The exact format might change in future +// versions. +func (d HashDisposition) GPGSigningKeysString() string { + if !d.SignedByAnyGPGKeys() { + return "" + } + // We want to return the keys in a predictable order, so we'll + // first collect them into a slice and sort them. + keyIDs := slices.Collect(maps.Keys(d.SignedByGPGKeyIDs)) + sort.Strings(keyIDs) + return strings.Join(keyIDs, ", ") +} + +// MergeHashDisposition takes two [HashDisposition] objects and returns a +// new object that represents the union of the information from both. +func MergeHashDisposition(a, b *HashDisposition) *HashDisposition { + ret := &HashDisposition{} + gpgKeyIDCount := len(a.SignedByGPGKeyIDs) + len(b.SignedByGPGKeyIDs) + if gpgKeyIDCount > 0 { + ret.SignedByGPGKeyIDs = make(collections.Set[string], gpgKeyIDCount) + for key := range a.SignedByGPGKeyIDs { + ret.SignedByGPGKeyIDs[key] = struct{}{} + } + for key := range b.SignedByGPGKeyIDs { + ret.SignedByGPGKeyIDs[key] = struct{}{} + } + } + ret.ReportedByRegistry = a.ReportedByRegistry || b.ReportedByRegistry + ret.VerifiedLocally = a.VerifiedLocally || b.VerifiedLocally + return ret +} + +// HashDispositions represents a collection of hashes that are associated +// with a provider as a result of installing it, each of which has a +// "disposition" that calling code can use to decide in what ways it is +// appropriate to make use of the hash. +// +// For example, the provider installer might choose to ignore hashes that +// were not verified locally unless are marked as having been signed by a +// trusted GPG key. +type HashDispositions map[Hash]*HashDisposition + +// AllGPGSigningKeysString returns a string representation of all GPG signing +// key IDs that signed an assertion that one of the hashes is valid for the +// associated provider version. +// +// If there are no such keys then the result is an empty string. +// +// The result of this is intended for display to a human in the UI, rather +// than for machine-readable purposes. The exact format might change in future +// versions. +func (ds HashDispositions) AllGPGSigningKeysString() string { + allKeyIDs := make(collections.Set[string]) + for _, disp := range ds { + for keyID := range disp.SignedByGPGKeyIDs { + allKeyIDs[keyID] = struct{}{} + } + } + // We want to return the keys in a predictable order, so we'll + // first collect them into a slice and sort them. + keyIDs := slices.Collect(maps.Keys(allKeyIDs)) + sort.Strings(keyIDs) + return strings.Join(keyIDs, ", ") +} + +func (ds HashDispositions) HasAnyReportedByRegistry() bool { + for _, disp := range ds { + if disp.ReportedByRegistry { + return true + } + } + return false +} + +func (ds HashDispositions) HasAnySignedByGPGKeys() bool { + for _, disp := range ds { + if disp.SignedByAnyGPGKeys() { + return true + } + } + return false +} + +// Merge modifies the receiever to also include all of the hashes and +// associated dispositions from the given other [HashDispositions] object. +// +// If both collections contain the same hash then their dispositions are +// also merged, so that the reciever is left representing the union +// of the disposition information from both collections. +func (ds HashDispositions) Merge(other HashDispositions) { + for hash, disp := range other { + haveDisp, ok := ds[hash] + if ok { + ds[hash] = MergeHashDisposition(haveDisp, disp) + } else { + ds[hash] = disp + } + } +} diff --git a/internal/getproviders/hash_test.go b/internal/getproviders/hash_test.go index 640417137b..b7ed31767d 100644 --- a/internal/getproviders/hash_test.go +++ b/internal/getproviders/hash_test.go @@ -6,7 +6,11 @@ package getproviders import ( + "maps" "testing" + + "github.com/google/go-cmp/cmp" + "github.com/opentofu/opentofu/internal/collections" ) func TestParseHash(t *testing.T) { @@ -73,3 +77,250 @@ func TestParseHash(t *testing.T) { }) } } + +func TestMergeHashDisposition(t *testing.T) { + tests := map[string]struct { + a, b *HashDisposition + want *HashDisposition + }{ + "empties": { + a: &HashDisposition{}, + b: &HashDisposition{}, + want: &HashDisposition{}, + }, + "empty with VerifiedLocally": { + a: &HashDisposition{ + VerifiedLocally: true, + }, + b: &HashDisposition{}, + want: &HashDisposition{ + VerifiedLocally: true, + }, + }, + "empty with ReportedByRegistry": { + a: &HashDisposition{ + ReportedByRegistry: true, + }, + b: &HashDisposition{}, + want: &HashDisposition{ + ReportedByRegistry: true, + }, + }, + "empty with one GPG key": { + a: &HashDisposition{ + SignedByGPGKeyIDs: collections.NewSet("abc123"), + }, + b: &HashDisposition{}, + want: &HashDisposition{ + SignedByGPGKeyIDs: collections.NewSet("abc123"), + }, + }, + "many GPG keys": { + a: &HashDisposition{ + SignedByGPGKeyIDs: collections.NewSet("abc123", "def456"), + }, + b: &HashDisposition{ + SignedByGPGKeyIDs: collections.NewSet("def456", "ghi789"), + }, + want: &HashDisposition{ + SignedByGPGKeyIDs: collections.NewSet("abc123", "def456", "ghi789"), + }, + }, + "VerifiedLocally with ReportedByRegistry": { + a: &HashDisposition{ + ReportedByRegistry: true, + }, + b: &HashDisposition{ + VerifiedLocally: true, + }, + want: &HashDisposition{ + ReportedByRegistry: true, + VerifiedLocally: true, + }, + }, + "VerifiedLocally with itself": { + a: &HashDisposition{ + VerifiedLocally: true, + }, + b: &HashDisposition{ + VerifiedLocally: true, + }, + want: &HashDisposition{ + VerifiedLocally: true, + }, + }, + "ReportedByRegistry with itself": { + a: &HashDisposition{ + ReportedByRegistry: true, + }, + b: &HashDisposition{ + ReportedByRegistry: true, + }, + want: &HashDisposition{ + ReportedByRegistry: true, + }, + }, + "Everything at once": { + a: &HashDisposition{ + SignedByGPGKeyIDs: collections.NewSet("def456", "ghi789"), + ReportedByRegistry: true, + }, + b: &HashDisposition{ + VerifiedLocally: true, + }, + want: &HashDisposition{ + SignedByGPGKeyIDs: collections.NewSet("def456", "ghi789"), + ReportedByRegistry: true, + VerifiedLocally: true, + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + // MergeHashDisposition is supposed to be commutative, so + // we'll test each case in both orders and expect an + // equivalent result in each case. + t.Run("a,b", func(t *testing.T) { + got := MergeHashDisposition(test.a, test.b) + if diff := cmp.Diff(test.want, got); diff != "" { + t.Error("wrong result\n" + diff) + } + }) + t.Run("b,a", func(t *testing.T) { + got := MergeHashDisposition(test.b, test.a) + if diff := cmp.Diff(test.want, got); diff != "" { + t.Error("wrong result\n" + diff) + } + }) + }) + } +} + +func TestHashDispositionsMerge(t *testing.T) { + // HashDispositions.Merge delegates to MergeHashDisposition when both + // arguments refer to the same hash. We already have lots of tests for + // MergeHashDisposition in TestMergeHashDisposition, so this test + // intentionally does not duplicate all of those cases and focuses + // only on the different cases that HashDispositions.Merge is directly + // concernd with. + + tests := map[string]struct { + a, b HashDispositions + want HashDispositions + }{ + "empties": { + a: HashDispositions{}, + b: HashDispositions{}, + want: HashDispositions{}, + }, + "one into empty": { + a: HashDispositions{}, + b: HashDispositions{ + Hash("test:foo"): &HashDisposition{ + ReportedByRegistry: true, + }, + }, + want: HashDispositions{ + Hash("test:foo"): &HashDisposition{ + ReportedByRegistry: true, + }, + }, + }, + "independent hashes": { + a: HashDispositions{ + Hash("test:foo"): &HashDisposition{ + ReportedByRegistry: true, + }, + }, + b: HashDispositions{ + Hash("test:bar"): &HashDisposition{ + VerifiedLocally: true, + }, + }, + want: HashDispositions{ + Hash("test:foo"): &HashDisposition{ + ReportedByRegistry: true, + }, + Hash("test:bar"): &HashDisposition{ + VerifiedLocally: true, + }, + }, + }, + "overlapping hashes": { + a: HashDispositions{ + Hash("test:foo"): &HashDisposition{ + ReportedByRegistry: true, + }, + }, + b: HashDispositions{ + Hash("test:foo"): &HashDisposition{ + VerifiedLocally: true, + }, + }, + want: HashDispositions{ + // This should be the result of MergeHashDispositions + // on the two different entries for test:foo. + Hash("test:foo"): &HashDisposition{ + ReportedByRegistry: true, + VerifiedLocally: true, + }, + }, + }, + "mix of overlapping and independent": { + a: HashDispositions{ + Hash("test:foo"): &HashDisposition{ + SignedByGPGKeyIDs: collections.NewSet("abc123"), + }, + Hash("test:bar"): &HashDisposition{ + ReportedByRegistry: true, + }, + }, + b: HashDispositions{ + Hash("test:foo"): &HashDisposition{ + SignedByGPGKeyIDs: collections.NewSet("def456"), + }, + Hash("test:baz"): &HashDisposition{ + VerifiedLocally: true, + }, + }, + want: HashDispositions{ + Hash("test:foo"): &HashDisposition{ + SignedByGPGKeyIDs: collections.NewSet("abc123", "def456"), + }, + Hash("test:bar"): &HashDisposition{ + ReportedByRegistry: true, + }, + Hash("test:baz"): &HashDisposition{ + VerifiedLocally: true, + }, + }, + }, + } + + for name, test := range tests { + t.Run(name, func(t *testing.T) { + // HashDispositions.Merge is supposed to be commutative, + // so we'll test each case in both orders and expect an + // equivalent result in each case. + t.Run("a,b", func(t *testing.T) { + // We'll make a shallow copy of test.a so that we + // aren't directly modifying the test table, since + // otherwise we'll pollute the input to the + // opposite order test below. + got := maps.Clone(test.a) + got.Merge(test.b) + if diff := cmp.Diff(test.want, got); diff != "" { + t.Error("wrong result\n" + diff) + } + }) + t.Run("b,a", func(t *testing.T) { + got := maps.Clone(test.b) + got.Merge(test.a) + if diff := cmp.Diff(test.want, got); diff != "" { + t.Error("wrong result\n" + diff) + } + }) + }) + } +} diff --git a/internal/getproviders/http_mirror_source_test.go b/internal/getproviders/http_mirror_source_test.go index 79a3f4a9b2..3f6a5c7243 100644 --- a/internal/getproviders/http_mirror_source_test.go +++ b/internal/getproviders/http_mirror_source_test.go @@ -137,12 +137,6 @@ func TestHTTPMirrorSource(t *testing.T) { if diff := cmp.Diff(want, got); diff != "" { t.Errorf("wrong result\n%s", diff) } - - gotHashes := got.AcceptableHashes() - wantHashes := []Hash{"h1:placeholder-hash", "h0:unacceptable-hash"} - if diff := cmp.Diff(wantHashes, gotHashes); diff != "" { - t.Errorf("wrong acceptable hashes\n%s", diff) - } }) t.Run("PackageMeta for a version that exists and has no hash", func(t *testing.T) { version := MustParseVersion("1.0.1") diff --git a/internal/getproviders/oci_registry_mirror_source_test.go b/internal/getproviders/oci_registry_mirror_source_test.go index 1a40d40f93..1231795b8c 100644 --- a/internal/getproviders/oci_registry_mirror_source_test.go +++ b/internal/getproviders/oci_registry_mirror_source_test.go @@ -192,7 +192,7 @@ func TestOCIRegistryMirrorSource(t *testing.T) { if err != nil { t.Fatal(err) } - if got, want := authResult.result, verifiedChecksum; got != want { + if got, want := authResult.summaryResult(), verifiedChecksum; got != want { t.Errorf("wrong authentication result\ngot: %#v\nwant: %#v", got, want) } exeContent, err := os.ReadFile(filepath.Join(pkgDir, "terraform-provider-foo")) diff --git a/internal/getproviders/package_authentication.go b/internal/getproviders/package_authentication.go index 4e01888636..290e9910db 100644 --- a/internal/getproviders/package_authentication.go +++ b/internal/getproviders/package_authentication.go @@ -12,19 +12,22 @@ import ( "encoding/hex" "errors" "fmt" + "iter" "log" "os" "strings" "github.com/ProtonMail/go-crypto/openpgp" openpgpErrors "github.com/ProtonMail/go-crypto/openpgp/errors" - tfaddr "github.com/opentofu/registry-address" + "github.com/opentofu/opentofu/internal/addrs" + "github.com/opentofu/opentofu/internal/collections" ) type packageAuthenticationResult int const ( - verifiedChecksum packageAuthenticationResult = iota + unauthenticated packageAuthenticationResult = iota + verifiedChecksum signed signingSkipped ) @@ -41,19 +44,90 @@ const ( // A failed PackageAuthentication attempt will return an "unauthenticated" // result, which is represented by nil. type PackageAuthenticationResult struct { - result packageAuthenticationResult - KeyID string + hashes HashDispositions +} + +// NewPackageAuthenticationResult constructs a new [PackageAuthenticationResult] +// based on the given hash dispositions. +// +// This is here primarily to allow constructing expected result values for tests +// in other packages. There isn't really any reason for non-test code outside +// of this package to directly construct package authentication results, since +// all of the "real" package authentication implementations should live in this +// package. +func NewPackageAuthenticationResult(hashes HashDispositions) *PackageAuthenticationResult { + return &PackageAuthenticationResult{hashes} +} + +func (t *PackageAuthenticationResult) summaryResult() packageAuthenticationResult { + if t == nil { + return unauthenticated + } + signedCount := 0 + registryReportCount := 0 + locallyVerifiedCount := 0 + for _, disp := range t.hashes { + if disp.SignedByAnyGPGKeys() { + signedCount++ + } + if disp.ReportedByRegistry { + registryReportCount++ + } + if disp.VerifiedLocally { + locallyVerifiedCount++ + } + } + switch { + case signedCount > 0: + return signed + case registryReportCount > 0: + return signingSkipped + case locallyVerifiedCount > 0: + return verifiedChecksum + default: + return unauthenticated + } } func (t *PackageAuthenticationResult) String() string { + return map[packageAuthenticationResult]string{ + unauthenticated: "unauthenticated", + verifiedChecksum: "verified checksum", + signingSkipped: "signing skipped", + signed: "signed", + }[t.summaryResult()] +} + +// HashesWithDisposition returns a sequence of hashes whose disposition after +// authentication matches the rule implemented by the given function cond. +// +// Use this to select the appropriate subset of hashes to record for the +// associated provider version in the dependency lock file, with the selection +// condition varying based on the authentication result and the current +// policy for whether signature verification is required and which keys +// are trusted. +func (t *PackageAuthenticationResult) HashesWithDisposition(cond func(*HashDisposition) bool) iter.Seq[Hash] { if t == nil { - return "unauthenticated" + // A nil result has no hashes at all + return func(yield func(Hash) bool) {} } - return []string{ - "verified checksum", - "signed", - "signing skipped", - }[t.result] + return func(yield func(Hash) bool) { + for hash, disp := range t.hashes { + if !cond(disp) { + continue + } + if keepGoing := yield(hash); !keepGoing { + break + } + } + } +} + +// GPGKeyIDsString returns a UI-oriented string representation of all of the +// GPG key IDs that asserted the validity of at least one of the hashes +// related to this package's provider version. +func (t *PackageAuthenticationResult) GPGKeyIDsString() string { + return t.hashes.AllGPGSigningKeysString() } // Signed returns whether the package was authenticated as signed by anyone. @@ -61,7 +135,7 @@ func (t *PackageAuthenticationResult) Signed() bool { if t == nil { return false } - return t.result == signed + return t.hashes.HasAnySignedByGPGKeys() } // SigningSkipped returns whether the package was authenticated but the key @@ -70,7 +144,7 @@ func (t *PackageAuthenticationResult) SigningSkipped() bool { if t == nil { return false } - return t.result == signingSkipped + return t.hashes.HasAnyReportedByRegistry() } // SigningKey represents a key used to sign packages from a registry. These are @@ -99,46 +173,6 @@ type PackageAuthentication interface { AuthenticatePackage(localLocation PackageLocation) (*PackageAuthenticationResult, error) } -// PackageAuthenticationHashes is an optional interface implemented by -// PackageAuthentication implementations that are able to return a set of -// hashes they would consider valid if a given PackageLocation referred to -// a package that matched that hash string. -// -// This can be used to record a set of acceptable hashes for a particular -// package in a lock file so that future install operations can determine -// whether the package has changed since its initial installation. -type PackageAuthenticationHashes interface { - PackageAuthentication - - // AcceptableHashes returns a set of hashes that this authenticator - // considers to be valid for the current package or, where possible, - // equivalent packages on other platforms. The order of the items in - // the result is not significant, and it may contain duplicates - // that are also not significant. - // - // This method's result should only be used to create a "lock" for a - // particular provider if an earlier call to AuthenticatePackage for - // the corresponding package succeeded. A caller might choose to apply - // differing levels of trust for the acceptable hashes depending on - // the authentication result: a "verified checksum" result only checked - // that the downloaded package matched what the source claimed, which - // could be considered to be less trustworthy than a check that includes - // verifying a signature from the origin registry, depending on what the - // hashes are going to be used for. - // - // Implementations of PackageAuthenticationHashes may return multiple - // hashes with different schemes, which means that all of them are equally - // acceptable. Implementors may also return hashes that use schemes the - // current version of the authenticator would not allow but that could be - // accepted by other versions of OpenTofu, e.g. if a particular hash - // scheme has been deprecated. - // - // Authenticators that don't use hashes as their authentication procedure - // will either not implement this interface or will have an implementation - // that returns an empty result. - AcceptableHashes() []Hash -} - type packageAuthenticationAll []PackageAuthentication // PackageAuthenticationAll combines several authentications together into a @@ -147,49 +181,30 @@ type packageAuthenticationAll []PackageAuthentication // The checks are processed in the order given, so a failure of an earlier // check will prevent execution of a later one. // -// The returned result is from the last authentication, so callers should -// take care to order the authentications such that the strongest is last. -// -// The returned object also implements the AcceptableHashes method from -// interface PackageAuthenticationHashes, returning the hashes from the -// last of the given checks that indicates at least one acceptable hash, -// or no hashes at all if none of the constituents indicate any. The result -// may therefore be incomplete if there is more than one check that can provide -// hashes and they disagree about which hashes are acceptable. +// The returned result is the union of the results of all authentications, +// describing all of the checksums that were somehow involved in the +// authentication process and what we learned about each one along the way. func PackageAuthenticationAll(checks ...PackageAuthentication) PackageAuthentication { return packageAuthenticationAll(checks) } func (checks packageAuthenticationAll) AuthenticatePackage(localLocation PackageLocation) (*PackageAuthenticationResult, error) { - var authResult *PackageAuthenticationResult + authResult := &PackageAuthenticationResult{ + hashes: make(HashDispositions), + } for _, check := range checks { - var err error - authResult, err = check.AuthenticatePackage(localLocation) + thisAuthResult, err := check.AuthenticatePackage(localLocation) if err != nil { - return authResult, err + return nil, err } + if thisAuthResult == nil { + continue // this result has nothing to contribute to our overall result + } + authResult.hashes.Merge(thisAuthResult.hashes) } return authResult, nil } -func (checks packageAuthenticationAll) AcceptableHashes() []Hash { - // The elements of checks are expected to be ordered so that the strongest - // one is later in the list, so we'll visit them in reverse order and - // take the first one that implements the interface and returns a non-empty - // result. - for i := len(checks) - 1; i >= 0; i-- { - check, ok := checks[i].(PackageAuthenticationHashes) - if !ok { - continue - } - allHashes := check.AcceptableHashes() - if len(allHashes) > 0 { - return allHashes - } - } - return nil -} - type packageHashAuthentication struct { RequiredHashes []Hash AllHashes []Hash @@ -221,13 +236,18 @@ func (a packageHashAuthentication) AuthenticatePackage(localLocation PackageLoca return nil, fmt.Errorf("this version of OpenTofu does not support any of the checksum formats given for this provider") } - matches, err := PackageMatchesAnyHash(localLocation, a.RequiredHashes) - if err != nil { - return nil, fmt.Errorf("failed to verify provider package checksums: %w", err) + hashes := make(HashDispositions, len(a.RequiredHashes)) + for verifiedHash, err := range HashesMatchingPackage(localLocation, a.RequiredHashes) { + if err != nil { + return nil, fmt.Errorf("failed to verify provider package checksums: %w", err) + } + hashes[verifiedHash] = &HashDisposition{ + VerifiedLocally: true, + } } - if matches { - return &PackageAuthenticationResult{result: verifiedChecksum}, nil + if len(hashes) > 0 { + return &PackageAuthenticationResult{hashes: hashes}, nil } if len(a.RequiredHashes) == 1 { return nil, fmt.Errorf("provider package doesn't match the expected checksum %q", a.RequiredHashes[0].String()) @@ -242,14 +262,6 @@ func (a packageHashAuthentication) AuthenticatePackage(localLocation PackageLoca return nil, fmt.Errorf("provider package doesn't match the any of the expected checksums") } -func (a packageHashAuthentication) AcceptableHashes() []Hash { - // In this case we include even hashes the current version of OpenTofu - // doesn't prefer, because this result is used for building a lock file - // and so it's helpful to include older hash formats that other OpenTofu - // versions might need in order to do authentication successfully. - return a.AllHashes -} - type archiveHashAuthentication struct { Platform Platform WantSHA256Sum [sha256.Size]byte @@ -288,11 +300,13 @@ func (a archiveHashAuthentication) AuthenticatePackage(localLocation PackageLoca if gotHash != wantHash { return nil, fmt.Errorf("archive has incorrect checksum %s (expected %s)", gotHash, wantHash) } - return &PackageAuthenticationResult{result: verifiedChecksum}, nil -} - -func (a archiveHashAuthentication) AcceptableHashes() []Hash { - return []Hash{HashLegacyZipSHAFromSHA(a.WantSHA256Sum)} + return &PackageAuthenticationResult{ + hashes: HashDispositions{ + gotHash: &HashDisposition{ + VerifiedLocally: true, + }, + }, + }, nil } type matchingChecksumAuthentication struct { @@ -355,7 +369,7 @@ type signatureAuthentication struct { Document []byte Signature []byte Keys []SigningKey - ProviderSource *tfaddr.Provider + ProviderSource addrs.Provider Meta PackageMeta } @@ -375,7 +389,7 @@ type signatureAuthentication struct { // // Any failure in the process of validating the signature will result in an // unauthenticated result. -func NewSignatureAuthentication(meta PackageMeta, document, signature []byte, keys []SigningKey, source *tfaddr.Provider) PackageAuthentication { +func NewSignatureAuthentication(meta PackageMeta, document, signature []byte, keys []SigningKey, source addrs.Provider) PackageAuthentication { return signatureAuthentication{ Document: document, Signature: signature, @@ -388,21 +402,58 @@ func NewSignatureAuthentication(meta PackageMeta, document, signature []byte, ke // ErrUnknownIssuer indicates an error when no valid signature for a provider could be found. var ErrUnknownIssuer = fmt.Errorf("authentication signature from unknown issuer") -func (s signatureAuthentication) shouldEnforceGPGValidation() bool { - // we should enforce validation for all provider sources that are not the default provider registry - if s.ProviderSource != nil && s.ProviderSource.Hostname != tfaddr.DefaultProviderRegistryHost { +// ShouldEnforceGPGValidationForProvider returns true if GPG signature +// validation must be enforced for the given provider. +// +// OpenTofu requires a valid GPG signature for any provider for which this +// function returns true. The result of this function only applies if the +// provider's origin registry does not return any signing keys for the +// provider; GPG signature is always required for any provider whose +// origin registry returns a signing key. +// +// The situations where this function returns false are part of a pragmatic +// compromise to allow the main OpenTofu registry to serve providers for +// which it does not currently know a signing key. For more information, +// refer to: +// +// https://github.com/opentofu/opentofu/issues/266 +// +// The result of this function also determines whether signature +// verification is required in order for a particular hash to be tracked +// for the given provider in a dependency lock file. If this returns +// false then the dependency lock file should include any hash that +// was reported by the provider's origin registry, even if not signed. +func ShouldEnforceGPGValidationForProvider(addr addrs.Provider) bool { + // GPG verification is always required for everything except the main + // OpenTofu registry, since our possibility of skipping verification + // is a concession to allow our official registry to distribute + // providers that we don't have known private keys for, in which + // case we're relying on the TLS certificate authentication of the + // registry server as an alternative mechanism. (The registry + // is ultimately what reports which keys would be valid anyway, so + // if someone is able to compromise the connection to the registry + // then they could arrange for it to report any signing key they like.) + if addr.Hostname != addrs.DefaultProviderRegistryHost { return true } - // if we have been provided keys, we should enforce GPG validation + // For the primary registry we allow providers that don't have + // GPG keys by default, but allow operators to opt out of this + // special exception using an environment variable. + enforceEnvVar, exists := os.LookupEnv(enforceGPGValidationEnvName) + return exists && enforceEnvVar == "true" +} + +func (s signatureAuthentication) shouldEnforceGPGValidation() bool { + // If the registry returned at least one signing key then validation is always required. if len(s.Keys) > 0 { return true } - // otherwise if the environment variable is set to true, we should enforce GPG validation - enforceEnvVar, exists := os.LookupEnv(enforceGPGValidationEnvName) - return exists && enforceEnvVar == "true" + // Otherwise the policy varies depending on what provider is being authenticated. + return ShouldEnforceGPGValidationForProvider(s.ProviderSource) } + func (s signatureAuthentication) shouldEnforceGPGExpiration() bool { // otherwise if the environment variable is set to true, we should enforce GPG expiration enforceEnvVar, exists := os.LookupEnv(enforceGPGExpirationEnvName) @@ -412,32 +463,36 @@ func (s signatureAuthentication) shouldEnforceGPGExpiration() bool { func (s signatureAuthentication) AuthenticatePackage(location PackageLocation) (*PackageAuthenticationResult, error) { shouldValidate := s.shouldEnforceGPGValidation() - if !shouldValidate { + var signingKeyIDs collections.Set[string] + if shouldValidate { + log.Printf("[DEBUG] Validating GPG signature of provider package %s", location) + + _, keyID, err := s.findSigningKey() + if err != nil { + return nil, fmt.Errorf("the provider is not signed with a valid signing key; please contact the provider author (%w)", err) + } + signingKeyIDs = collections.NewSet(keyID) + } else { // As this is a temporary measure, we will log a warning to the user making it very clear what is happening // and why. This will be removed in a future release. log.Printf("[WARN] Skipping GPG validation of provider package %s as no keys were provided by the registry. See https://github.com/opentofu/opentofu/pull/309 for more information.", location) - - // construct an empty keyID to indicate that we are not validating and return no errors - // this is to force a successful authentication - // TODO: discuss if this key should be hardcoded to a value such as "UNKNOWN"? - return &PackageAuthenticationResult{result: signingSkipped, KeyID: ""}, nil - } else { - log.Printf("[DEBUG] Validating GPG signature of provider package %s", location) } - // Find the key that signed the checksum file. This can fail if there is no - // valid signature for any of the provided keys. - - _, keyID, err := s.findSigningKey() - if err != nil { - return nil, fmt.Errorf("the provider is not signed with a valid signing key; please contact the provider author (%w)", err) + // For each of the hashes mentioned in the document that was signed we'll announce that + // it was reported by the provider's origin registry, since that's the only place that + // this kind of signed hash file can come from, and _possibly_ report the key IDs that + // signed it unless we decided above that validation wasn't actually needed. + hashes := make(HashDispositions) + for _, hash := range s.acceptableHashes() { + hashes[hash] = &HashDisposition{ + ReportedByRegistry: true, + SignedByGPGKeyIDs: signingKeyIDs, + } } - - // We have a valid signature. - return &PackageAuthenticationResult{result: signed, KeyID: keyID}, nil + return &PackageAuthenticationResult{hashes: hashes}, nil } -func (s signatureAuthentication) AcceptableHashes() []Hash { +func (s signatureAuthentication) acceptableHashes() []Hash { // This is a bit of an abstraction leak because signatureAuthentication // otherwise just treats the document as an opaque blob that's been // signed, but here we're making assumptions about its format because diff --git a/internal/getproviders/package_authentication_helper_test.go b/internal/getproviders/package_authentication_helper_test.go new file mode 100644 index 0000000000..bbf4f81c79 --- /dev/null +++ b/internal/getproviders/package_authentication_helper_test.go @@ -0,0 +1,83 @@ +// Copyright (c) The OpenTofu Authors +// SPDX-License-Identifier: MPL-2.0 +// Copyright (c) 2023 HashiCorp, Inc. +// SPDX-License-Identifier: MPL-2.0 + +package getproviders + +import ( + "bytes" + "testing" + + "github.com/ProtonMail/go-crypto/openpgp" + "github.com/ProtonMail/go-crypto/openpgp/armor" +) + +// This file contains only some helper functions for the tests in package_authentication_test.go. +// There are not actually any tests in here. +// +// We use these helpers sparingly to minimize the number of opaque-to-humans artifacts +// we need to include as hard-coded constants, instead deriving the opaque artifacts from +// more human-accessible artifacts where possible. However, we must be careful to make sure +// that our use of these helpers does not cause a test to effectively "cheat" by creating +// a situation that could not possibly occur in realistic use. + +// pgpTestEntity returns a freshly-generated [openpgp.Entity] with the given name and +// a hard-coded comment and fake email address. The private key is generated randomly +// on request. +// +// We generate keys on the fly, rather than hard-coding them, to avoid dealiing with the +// noise generated by zealous security scanners that get (justifyably) nervous when +// they find a private key included directly in the source code of a codebase. However, +// this just-in-time generation does unfortunately come at a small performance cost, +// so tests should avoid calling this from inside a loop. +func pgpTestEntity(t *testing.T, name string) *openpgp.Entity { + t.Helper() + + entity, err := openpgp.NewEntity(name, "throwaway key used only for testing", "testing@invalid", nil) + if err != nil { + t.Fatalf("failed to generate a PGP key for testing: %s", err) + } + return entity +} + +// pgpPublicKeyForTestEntity returns an "armored" representation of the public key +// of the given entity, as we'd expect to obtain from an OpenTofu provider +// registry when performing GPG verification of a provider release, and +// a hex representation of the key's ID that we'd use to report the use of +// the key in a successful authentication result. +func pgpPublicKeyForTestEntity(t *testing.T, entity *openpgp.Entity) ([]byte, string) { + // The "armored" representation, described in RFC 4880 chapter 6, + // is a PEM-like ASCII-only representation of key data. Therefore + // we have two layers to deal with here: the armor encoding, and + // the inner raw representation of the public key. + + var buf bytes.Buffer + w, err := armor.Encode(&buf, "PGP PUBLIC KEY BLOCK", nil) + if err != nil { + t.Fatalf("failed to begin ASCII-armored public key block: %s", err) + } + err = entity.Serialize(w) + if err != nil { + t.Fatalf("failed to serialize the public key part of the PGP entity: %s", err) + } + err = w.Close() + if err != nil { + t.Fatalf("failed to add ASCII-armor footer: %s", err) + } + return buf.Bytes(), entity.PrimaryKey.KeyIdString() +} + +// pgpSignForTesting generates a signature for the given message using the private +// key from the given signer, in the raw binary format we'd normally expect to +// receive from an artifact linked in an OpenTofu provider registry. +func pgpSignForTesting(t *testing.T, message []byte, signer *openpgp.Entity) []byte { + t.Helper() + + var buf bytes.Buffer + err := openpgp.DetachSign(&buf, signer, bytes.NewReader(message), nil) + if err != nil { + t.Fatalf("failed to PGP sign message for testing: %s", err) + } + return buf.Bytes() +} diff --git a/internal/getproviders/package_authentication_test.go b/internal/getproviders/package_authentication_test.go index 472c49ccc9..b88de5566f 100644 --- a/internal/getproviders/package_authentication_test.go +++ b/internal/getproviders/package_authentication_test.go @@ -10,35 +10,110 @@ import ( "encoding/base64" "errors" "fmt" + "slices" + "sort" "strings" "testing" + "github.com/ProtonMail/go-crypto/openpgp" openpgpErrors "github.com/ProtonMail/go-crypto/openpgp/errors" - tfaddr "github.com/opentofu/registry-address" - "github.com/google/go-cmp/cmp" - "github.com/ProtonMail/go-crypto/openpgp" + "github.com/opentofu/opentofu/internal/addrs" + "github.com/opentofu/opentofu/internal/collections" ) func TestPackageAuthenticationResult(t *testing.T) { - tests := []struct { + tests := map[string]struct { result *PackageAuthenticationResult want string }{ - { + "nil": { nil, "unauthenticated", }, - { - &PackageAuthenticationResult{result: signed}, + "SignedByGPGKeyIDs": { + &PackageAuthenticationResult{ + hashes: HashDispositions{ + Hash("test:placeholder"): { + SignedByGPGKeyIDs: collections.NewSet("abc123"), + }, + }, + }, + "signed", + }, + "VerifiedLocally": { + &PackageAuthenticationResult{ + hashes: HashDispositions{ + Hash("test:placeholder"): { + VerifiedLocally: true, + }, + }, + }, + "verified checksum", + }, + "ReportedByRegistry": { + &PackageAuthenticationResult{ + hashes: HashDispositions{ + Hash("test:placeholder"): { + ReportedByRegistry: true, + }, + }, + }, + "signing skipped", + }, + "SignedByGPGKeyIDs+VerifiedLocally": { + &PackageAuthenticationResult{ + hashes: HashDispositions{ + Hash("test:placeholder"): { + SignedByGPGKeyIDs: collections.NewSet("abc123"), + VerifiedLocally: true, + }, + }, + }, + "signed", + }, + "SignedByGPGKeyIDs+ReportedByRegistry": { + &PackageAuthenticationResult{ + hashes: HashDispositions{ + Hash("test:placeholder"): { + SignedByGPGKeyIDs: collections.NewSet("abc123"), + ReportedByRegistry: true, + }, + }, + }, + "signed", + }, + "ReportedByRegistry+VerifiedLocally": { + &PackageAuthenticationResult{ + hashes: HashDispositions{ + Hash("test:placeholder"): { + ReportedByRegistry: true, + VerifiedLocally: true, + }, + }, + }, + "signing skipped", + }, + "SignedByGPGKeyIDs+ReportedByRegistry+VerifiedLocally": { + &PackageAuthenticationResult{ + hashes: HashDispositions{ + Hash("test:placeholder"): { + SignedByGPGKeyIDs: collections.NewSet("abc123"), + ReportedByRegistry: true, + VerifiedLocally: true, + }, + }, + }, "signed", }, } - for _, test := range tests { - if got := test.result.String(); got != test.want { - t.Errorf("wrong value: got %q, want %q", got, test.want) - } + for name, test := range tests { + t.Run(name, func(t *testing.T) { + if got := test.result.String(); got != test.want { + t.Errorf("wrong value\ngot: %q\nwant: %q", got, test.want) + } + }) } } @@ -46,13 +121,13 @@ func TestPackageAuthenticationResult(t *testing.T) { // interface which returns fixed values. This is used to test the combining // logic of PackageAuthenticationAll. type mockAuthentication struct { - result packageAuthenticationResult + hashes HashDispositions err error } func (m mockAuthentication) AuthenticatePackage(localLocation PackageLocation) (*PackageAuthenticationResult, error) { if m.err == nil { - return &PackageAuthenticationResult{result: m.result}, nil + return &PackageAuthenticationResult{hashes: m.hashes}, nil } else { return nil, m.err } @@ -60,20 +135,27 @@ func (m mockAuthentication) AuthenticatePackage(localLocation PackageLocation) ( var _ PackageAuthentication = (*mockAuthentication)(nil) -// If all authentications succeed, the returned result should come from the -// last authentication. +// If all authentications succeed, the returned result is based on the merger +// of all of the individual results. func TestPackageAuthenticationAll_success(t *testing.T) { result, err := PackageAuthenticationAll( - &mockAuthentication{result: verifiedChecksum}, - &mockAuthentication{result: signed}, + &mockAuthentication{hashes: HashDispositions{ + Hash("test:a"): { + VerifiedLocally: true, + }, + }}, + &mockAuthentication{hashes: HashDispositions{ + Hash("test:a"): { + SignedByGPGKeyIDs: collections.NewSet("abc123"), + }, + }}, ).AuthenticatePackage(nil) - - want := PackageAuthenticationResult{result: signed} - if result == nil || *result != want { - t.Errorf("wrong result: want %#v, got %#v", want, result) - } if err != nil { - t.Errorf("wrong err: got %#v, want nil", err) + t.Fatalf("unexpected error: %s", err) + } + + if got, want := result.String(), "signed"; got != want { + t.Errorf("wrong result summary\ngot: %s\nwant: %s", got, want) } } @@ -82,16 +164,24 @@ func TestPackageAuthenticationAll_success(t *testing.T) { func TestPackageAuthenticationAll_failure(t *testing.T) { someError := errors.New("some error") result, err := PackageAuthenticationAll( - &mockAuthentication{result: verifiedChecksum}, + &mockAuthentication{hashes: HashDispositions{ + Hash("test:a"): { + VerifiedLocally: true, + }, + }}, &mockAuthentication{err: someError}, - &mockAuthentication{result: signed}, + &mockAuthentication{hashes: HashDispositions{ + Hash("test:a"): { + SignedByGPGKeyIDs: collections.NewSet("abc123"), + }, + }}, ).AuthenticatePackage(nil) if result != nil { t.Errorf("wrong result: got %#v, want nil", result) } if err != someError { - t.Errorf("wrong err: got %#v, want %#v", err, someError) + t.Errorf("wrong error\ngot: %s\nwant %s", err, someError) } } @@ -109,13 +199,12 @@ func TestPackageHashAuthentication_success(t *testing.T) { auth := NewPackageHashAuthentication(Platform{"linux", "amd64"}, wantHashes) result, err := auth.AuthenticatePackage(location) - - wantResult := PackageAuthenticationResult{result: verifiedChecksum} - if result == nil || *result != wantResult { - t.Errorf("wrong result: got %#v, want %#v", result, wantResult) - } if err != nil { - t.Errorf("wrong err: got %s, want nil", err) + t.Fatalf("unexpected error: %s", err) + } + + if got, want := result.String(), "verified checksum"; got != want { + t.Errorf("wrong result summary\ngot: %s\nwant: %s", got, want) } } @@ -172,13 +261,12 @@ func TestArchiveChecksumAuthentication_success(t *testing.T) { auth := NewArchiveChecksumAuthentication(Platform{"linux", "amd64"}, wantSHA256Sum) result, err := auth.AuthenticatePackage(location) - - wantResult := PackageAuthenticationResult{result: verifiedChecksum} - if result == nil || *result != wantResult { - t.Errorf("wrong result: got %#v, want %#v", result, wantResult) - } if err != nil { - t.Errorf("wrong err: got %s, want nil", err) + t.Fatalf("unexpected error: %s", err) + } + + if got, want := result.String(), "verified checksum"; got != want { + t.Errorf("wrong result summary\ngot: %s\nwant: %s", got, want) } } @@ -318,63 +406,94 @@ func TestMatchingChecksumAuthentication_failure(t *testing.T) { // authentication is successful. The value of the result depends on the signing // key. func TestSignatureAuthentication_success(t *testing.T) { + // To make it easier for us to make changes to the constants we use + // to test this process we hard-code only the data that is to be signed + // and then dynamically generate a signing key and associated signature + // on each test run. In realistic use the signature would be generated + // in the provider's release process and OpenTofu would have access only + // to the public part of the GPG key. + pgpEntity := pgpTestEntity(t, "TestSignatureAuthentication_success") + publicKeyArmor, keyID := pgpPublicKeyForTestEntity(t, pgpEntity) + signature := pgpSignForTesting(t, []byte(testShaSumsRealistic), pgpEntity) + t.Logf("generated PGP key %s\n%s", keyID, publicKeyArmor) + t.Logf("generated signature\n%x", signature) + + // The following are the hashes included in testShaSumsRealistic, which + // should therefore be reported in a successful result based on that input. + wantHashes := []Hash{ + Hash("zh:086119a26576d06b8281a97e8644380da89ce16197cd955f74ea5ee664e9358b"), + Hash("zh:0e9fd0f3e2254b526a0e81e0cfdfc82583b0cd343778c53ead21aa7d52f776d7"), + Hash("zh:17e0b496022bc4e4137be15e96d2b051c8acd6e14cb48d9b13b262330464f6cc"), + Hash("zh:1e5f7a5f3ade7b8b1d1d59c5cea2e1a2f8d2f8c3f41962dbbe8647e222be8239"), + Hash("zh:2696c86228f491bc5425561c45904c9ce39b1c676b1e17734cb2ee6b578c4bcd"), + Hash("zh:48f1826ec31d6f104e46cc2022b41f30cd1019ef48eaec9697654ef9ec37a879"), + Hash("zh:66a947e7de1c74caf9f584c3ed4e91d2cb1af6fe5ce8abaf1cf8f7ff626a09d1"), + Hash("zh:7d7e888fdd28abfe00894f9055209b9eec785153641de98e6852aa071008d4ee"), + Hash("zh:a5ba9945606bb7bfb821ba303957eeb40dd9ee4e706ba8da1eaf7cbeb0356e63"), + Hash("zh:def1b73849bec0dc57a04405847921bf9206c75b52ae9de195476facb26bd85e"), + Hash("zh:df3a5a8d6ffff7bacf19c92d10d0d500f98169ea17b3764b01a789f563d1aad7"), + Hash("zh:f8b6cf9ade087c17826d49d89cef21261cdc22bd27065bbc5b27d7dbf7fbbf6c"), + } + tests := map[string]struct { - signature string - keys []SigningKey - result PackageAuthenticationResult + signature []byte + keys []SigningKey + wantResult string + wantKeyID string + wantHashes []Hash }{ - "community provider": { - testAuthorSignatureGoodBase64, + "validly-signed provider": { + signature, []SigningKey{ - { - ASCIIArmor: testAuthorKeyArmor, - }, - }, - PackageAuthenticationResult{ - result: signed, - KeyID: testAuthorKeyID, + {ASCIIArmor: string(publicKeyArmor)}, }, + "signed", + keyID, + wantHashes, }, "multiple signing keys": { - testAuthorSignatureGoodBase64, + signature, []SigningKey{ - { - ASCIIArmor: anotherPublicKey, - }, - { - ASCIIArmor: testAuthorKeyArmor, - }, - }, - PackageAuthenticationResult{ - result: signed, - KeyID: testAuthorKeyID, + {ASCIIArmor: anotherPublicKey}, + {ASCIIArmor: string(publicKeyArmor)}, }, + "signed", + keyID, + wantHashes, }, } for name, test := range tests { t.Run(name, func(t *testing.T) { - // Location is unused location := PackageLocalArchive("testdata/my-package.zip") - // - //providerSource, err := tfaddr.ParseProviderSource("testdata/my-package.zip") - //if err != nil { - // t.Fatal(err) - //} - signature, err := base64.StdEncoding.DecodeString(test.signature) - if err != nil { - t.Fatal(err) - } - - auth := NewSignatureAuthentication(PackageMeta{Location: location}, []byte(testShaSumsPlaceholder), signature, test.keys, nil) + auth := NewSignatureAuthentication( + PackageMeta{Location: location}, + []byte(testShaSumsRealistic), + test.signature, + test.keys, + addrs.NewDefaultProvider("test"), + ) result, err := auth.AuthenticatePackage(location) - - if result == nil || *result != test.result { - t.Errorf("wrong result: got %#v, want %#v", result, test.result) - } if err != nil { - t.Errorf("wrong err: got %s, want nil", err) + t.Fatalf("unexpected error: %s", err) + } + + if got, want := result.String(), test.wantResult; got != want { + t.Errorf("wrong result\ngot: %s\nwant: %s", got, want) + } + if got, want := result.GPGKeyIDsString(), test.wantKeyID; got != want { + t.Errorf("wrong GPG key IDs string\ngot: %s\nwant: %s", got, want) + } + + gotHashes := slices.Collect(result.HashesWithDisposition(func(hd *HashDisposition) bool { + return hd.SignedByGPGKeyIDs.Has(test.wantKeyID) + })) + sort.Slice(gotHashes, func(i, j int) bool { + return gotHashes[i].String() < gotHashes[j].String() + }) + if diff := cmp.Diff(test.wantHashes, gotHashes); diff != "" { + t.Error("wrong hashes\n" + diff) } }) } @@ -382,9 +501,10 @@ func TestSignatureAuthentication_success(t *testing.T) { func TestNewSignatureAuthentication_success(t *testing.T) { tests := map[string]struct { - signature string - keys []SigningKey - result PackageAuthenticationResult + signature string + keys []SigningKey + wantResult string + wantKeyID string }{ "official provider": { testHashicorpSignatureGoodBase64, @@ -393,10 +513,8 @@ func TestNewSignatureAuthentication_success(t *testing.T) { ASCIIArmor: TestingPublicKey, }, }, - PackageAuthenticationResult{ - result: signed, - KeyID: testHashiCorpPublicKeyID, - }, + "signed", + testHashiCorpPublicKeyID, }, } @@ -410,14 +528,23 @@ func TestNewSignatureAuthentication_success(t *testing.T) { t.Fatal(err) } - auth := NewSignatureAuthentication(PackageMeta{Location: location}, []byte(testProviderShaSums), signature, test.keys, nil) + auth := NewSignatureAuthentication( + PackageMeta{Location: location}, + []byte(testProviderShaSums), + signature, + test.keys, + addrs.NewDefaultProvider("test"), + ) result, err := auth.AuthenticatePackage(location) - - if result == nil || *result != test.result { - t.Errorf("wrong result: got %#v, want %#v", result, test.result) - } if err != nil { - t.Errorf("wrong err: got %s, want nil", err) + t.Fatalf("unexpected error: %s", err) + } + + if got, want := result.String(), test.wantResult; got != want { + t.Errorf("wrong result\ngot: %s\nwant: %s", got, want) + } + if got, want := result.GPGKeyIDsString(), test.wantKeyID; got != want { + t.Errorf("wrong GPG key IDs string\ngot: %s\nwant: %s", got, want) } }) } @@ -448,7 +575,13 @@ func TestNewSignatureAuthentication_expired(t *testing.T) { t.Fatal(err) } - auth := NewSignatureAuthentication(PackageMeta{Location: location}, []byte(testProviderShaSums), signature, test.keys, nil) + auth := NewSignatureAuthentication( + PackageMeta{Location: location}, + []byte(testProviderShaSums), + signature, + test.keys, + addrs.NewDefaultProvider("test"), + ) _, err = auth.AuthenticatePackage(location) if err == nil { @@ -511,7 +644,13 @@ func TestSignatureAuthentication_failure(t *testing.T) { t.Fatal(err) } - auth := NewSignatureAuthentication(PackageMeta{Location: location}, []byte(testShaSumsPlaceholder), signature, test.keys, nil) + auth := NewSignatureAuthentication( + PackageMeta{Location: location}, + []byte(testShaSumsPlaceholder), + signature, + test.keys, + addrs.NewDefaultProvider("test"), + ) result, err := auth.AuthenticatePackage(location) if result != nil { @@ -537,33 +676,6 @@ func TestSignatureAuthentication_failure(t *testing.T) { } } -func TestSignatureAuthentication_acceptableHashes(t *testing.T) { - auth := NewSignatureAuthentication(PackageMeta{}, []byte(testShaSumsRealistic), nil, nil, nil) - authWithHashes, ok := auth.(PackageAuthenticationHashes) - if !ok { - t.Fatalf("%T does not implement PackageAuthenticationHashes", auth) - } - got := authWithHashes.AcceptableHashes() - want := []Hash{ - // These are the hashes encoded in constant testShaSumsRealistic - "zh:7d7e888fdd28abfe00894f9055209b9eec785153641de98e6852aa071008d4ee", - "zh:f8b6cf9ade087c17826d49d89cef21261cdc22bd27065bbc5b27d7dbf7fbbf6c", - "zh:a5ba9945606bb7bfb821ba303957eeb40dd9ee4e706ba8da1eaf7cbeb0356e63", - "zh:df3a5a8d6ffff7bacf19c92d10d0d500f98169ea17b3764b01a789f563d1aad7", - "zh:086119a26576d06b8281a97e8644380da89ce16197cd955f74ea5ee664e9358b", - "zh:1e5f7a5f3ade7b8b1d1d59c5cea2e1a2f8d2f8c3f41962dbbe8647e222be8239", - "zh:0e9fd0f3e2254b526a0e81e0cfdfc82583b0cd343778c53ead21aa7d52f776d7", - "zh:66a947e7de1c74caf9f584c3ed4e91d2cb1af6fe5ce8abaf1cf8f7ff626a09d1", - "zh:def1b73849bec0dc57a04405847921bf9206c75b52ae9de195476facb26bd85e", - "zh:48f1826ec31d6f104e46cc2022b41f30cd1019ef48eaec9697654ef9ec37a879", - "zh:17e0b496022bc4e4137be15e96d2b051c8acd6e14cb48d9b13b262330464f6cc", - "zh:2696c86228f491bc5425561c45904c9ce39b1c676b1e17734cb2ee6b578c4bcd", - } - if diff := cmp.Diff(want, got); diff != "" { - t.Errorf("wrong result\n%s", diff) - } -} - const testAuthorKeyID = `37A6AB3BCF2C170A` // testAuthorKeyArmor is test key ID 5BFEEC4317E746008621970637A6AB3BCF2C170A. @@ -746,15 +858,15 @@ func testReadArmoredEntity(t *testing.T, armor string) *openpgp.Entity { func TestShouldEnforceGPGValidation(t *testing.T) { tests := []struct { name string - providerSource *tfaddr.Provider + providerSource addrs.Provider keys []SigningKey envVarValue string expected bool }{ { name: "default provider registry, no keys", - providerSource: &tfaddr.Provider{ - Hostname: tfaddr.DefaultProviderRegistryHost, + providerSource: addrs.Provider{ + Hostname: addrs.DefaultProviderRegistryHost, }, keys: []SigningKey{}, envVarValue: "", @@ -762,8 +874,8 @@ func TestShouldEnforceGPGValidation(t *testing.T) { }, { name: "default provider registry, some keys", - providerSource: &tfaddr.Provider{ - Hostname: tfaddr.DefaultProviderRegistryHost, + providerSource: addrs.Provider{ + Hostname: addrs.DefaultProviderRegistryHost, }, keys: []SigningKey{ { @@ -775,7 +887,7 @@ func TestShouldEnforceGPGValidation(t *testing.T) { }, { name: "non-default provider registry, no keys", - providerSource: &tfaddr.Provider{ + providerSource: addrs.Provider{ Hostname: "my-registry.com", }, keys: []SigningKey{}, @@ -784,7 +896,7 @@ func TestShouldEnforceGPGValidation(t *testing.T) { }, { name: "non-default provider registry, some keys", - providerSource: &tfaddr.Provider{ + providerSource: addrs.Provider{ Hostname: "my-registry.com", }, keys: []SigningKey{ @@ -798,8 +910,8 @@ func TestShouldEnforceGPGValidation(t *testing.T) { // env var "true" { name: "default provider registry, no keys, env var true", - providerSource: &tfaddr.Provider{ - Hostname: tfaddr.DefaultProviderRegistryHost, + providerSource: addrs.Provider{ + Hostname: addrs.DefaultProviderRegistryHost, }, keys: []SigningKey{}, envVarValue: "true", @@ -807,8 +919,8 @@ func TestShouldEnforceGPGValidation(t *testing.T) { }, { name: "default provider registry, some keys, env var true", - providerSource: &tfaddr.Provider{ - Hostname: tfaddr.DefaultProviderRegistryHost, + providerSource: addrs.Provider{ + Hostname: addrs.DefaultProviderRegistryHost, }, keys: []SigningKey{ { @@ -819,7 +931,7 @@ func TestShouldEnforceGPGValidation(t *testing.T) { expected: true, }, { name: "non-default provider registry, no keys, env var true", - providerSource: &tfaddr.Provider{ + providerSource: addrs.Provider{ Hostname: "my-registry.com", }, keys: []SigningKey{}, @@ -828,7 +940,7 @@ func TestShouldEnforceGPGValidation(t *testing.T) { }, { name: "non-default provider registry, some keys, env var true", - providerSource: &tfaddr.Provider{ + providerSource: addrs.Provider{ Hostname: "my-registry.com", }, keys: []SigningKey{ @@ -842,8 +954,8 @@ func TestShouldEnforceGPGValidation(t *testing.T) { // env var "false" { name: "default provider registry, no keys, env var false", - providerSource: &tfaddr.Provider{ - Hostname: tfaddr.DefaultProviderRegistryHost, + providerSource: addrs.Provider{ + Hostname: addrs.DefaultProviderRegistryHost, }, keys: []SigningKey{}, envVarValue: "false", @@ -851,8 +963,8 @@ func TestShouldEnforceGPGValidation(t *testing.T) { }, { name: "default provider registry, some keys, env var false", - providerSource: &tfaddr.Provider{ - Hostname: tfaddr.DefaultProviderRegistryHost, + providerSource: addrs.Provider{ + Hostname: addrs.DefaultProviderRegistryHost, }, keys: []SigningKey{ { @@ -863,7 +975,7 @@ func TestShouldEnforceGPGValidation(t *testing.T) { expected: true, }, { name: "non-default provider registry, no keys, env var false", - providerSource: &tfaddr.Provider{ + providerSource: addrs.Provider{ Hostname: "my-registry.com", }, keys: []SigningKey{}, @@ -872,7 +984,7 @@ func TestShouldEnforceGPGValidation(t *testing.T) { }, { name: "non-default provider registry, some keys, env var false", - providerSource: &tfaddr.Provider{ + providerSource: addrs.Provider{ Hostname: "my-registry.com", }, keys: []SigningKey{ diff --git a/internal/getproviders/package_location_oci_blob_archive_test.go b/internal/getproviders/package_location_oci_blob_archive_test.go index 9a26a77ec5..b9aac2e671 100644 --- a/internal/getproviders/package_location_oci_blob_archive_test.go +++ b/internal/getproviders/package_location_oci_blob_archive_test.go @@ -22,6 +22,7 @@ import ( orasMemoryStore "oras.land/oras-go/v2/content/memory" "github.com/opentofu/opentofu/internal/addrs" + "github.com/opentofu/opentofu/internal/collections" ) func TestPackageOCIBlobArchive(t *testing.T) { @@ -57,7 +58,11 @@ func TestPackageOCIBlobArchive(t *testing.T) { TargetPlatform: CurrentPlatform, Location: loc, Authentication: &mockAuthentication{ - result: signed, + hashes: HashDispositions{ + Hash("test:placeholder"): { + SignedByGPGKeyIDs: collections.NewSet("abc123"), + }, + }, }, } targetDir := t.TempDir() diff --git a/internal/getproviders/registry_client.go b/internal/getproviders/registry_client.go index d60e13dc07..7c1bce5033 100644 --- a/internal/getproviders/registry_client.go +++ b/internal/getproviders/registry_client.go @@ -359,7 +359,7 @@ func (c *registryClient) PackageMeta(ctx context.Context, provider addrs.Provide ret.Authentication = PackageAuthenticationAll( NewMatchingChecksumAuthentication(document, body.Filename, checksum), NewArchiveChecksumAuthentication(ret.TargetPlatform, checksum), - NewSignatureAuthentication(ret, document, signature, keys, &provider), + NewSignatureAuthentication(ret, document, signature, keys, provider), ) return ret, nil diff --git a/internal/getproviders/registry_source_test.go b/internal/getproviders/registry_source_test.go index 74d6bc3402..63d1484314 100644 --- a/internal/getproviders/registry_source_test.go +++ b/internal/getproviders/registry_source_test.go @@ -142,17 +142,16 @@ func TestSourcePackageMeta(t *testing.T) { []SigningKey{ {ASCIIArmor: TestingPublicKey}, }, - &tfaddr.Provider{Hostname: "example.com", Namespace: "awesomesauce", Type: "happycloud"}, + tfaddr.Provider{Hostname: "example.com", Namespace: "awesomesauce", Type: "happycloud"}, ), ) tests := []struct { - provider string - version string - os, arch string - want PackageMeta - wantHashes []Hash - wantErr string + provider string + version string + os, arch string + want PackageMeta + wantErr string }{ // These test cases are relying on behaviors of the fake provider // registry server implemented in registry_client_test.go. @@ -161,10 +160,6 @@ func TestSourcePackageMeta(t *testing.T) { "1.2.0", "linux", "amd64", validMeta, - []Hash{ - "zh:000000000000000000000000000000000000000000000000000000000000f00d", - "zh:000000000000000000000000000000000000000000000000000000000000face", - }, ``, }, { @@ -172,7 +167,6 @@ func TestSourcePackageMeta(t *testing.T) { "1.2.0", "nonexist", "amd64", PackageMeta{}, - nil, `provider example.com/awesomesauce/happycloud 1.2.0 is not available for nonexist_amd64`, }, { @@ -180,7 +174,6 @@ func TestSourcePackageMeta(t *testing.T) { "1.2.0", "linux", "amd64", PackageMeta{}, - nil, `host not.example.com does not offer a OpenTofu provider registry`, }, { @@ -188,7 +181,6 @@ func TestSourcePackageMeta(t *testing.T) { "1.2.0", "linux", "amd64", PackageMeta{}, - nil, `host too-new.example.com does not support the provider registry protocol required by this OpenTofu version, but may be compatible with a different OpenTofu version`, }, { @@ -196,7 +188,6 @@ func TestSourcePackageMeta(t *testing.T) { "1.2.0", "linux", "amd64", PackageMeta{}, - nil, `could not query provider registry for fails.example.com/awesomesauce/happycloud: the request failed after 2 attempts, please try again later: Get "http://placeholder-origin/fails-immediately/awesomesauce/happycloud/1.2.0/download/linux/amd64": EOF`, }, } @@ -241,9 +232,6 @@ func TestSourcePackageMeta(t *testing.T) { if diff := cmp.Diff(got, test.want, cmpOpts); diff != "" { t.Errorf("wrong result\n%s", diff) } - if diff := cmp.Diff(test.wantHashes, got.AcceptableHashes()); diff != "" { - t.Errorf("wrong AcceptableHashes result\n%s", diff) - } }) } diff --git a/internal/getproviders/types.go b/internal/getproviders/types.go index a253bd8520..42af2f45e6 100644 --- a/internal/getproviders/types.go +++ b/internal/getproviders/types.go @@ -292,39 +292,6 @@ func (m PackageMeta) PackedFilePath(baseDir string) string { return PackedFilePathForPackage(baseDir, m.Provider, m.Version, m.TargetPlatform) } -// AcceptableHashes returns a set of hashes that could be recorded for -// comparison to future results for the same provider version, to implement a -// "trust on first use" scheme. -// -// The AcceptableHashes result is a platform-agnostic set of hashes, with the -// intent that in most cases it will be used as an additional cross-check in -// addition to a platform-specific hash check made during installation. However, -// there are some situations (such as verifying an already-installed package -// that's on local disk) where OpenTofu would check only against the results -// of this function, meaning that it would in principle accept another -// platform's package as a substitute for the correct platform. That's a -// pragmatic compromise to allow lock files derived from the result of this -// method to be portable across platforms. -// -// Callers of this method should typically also verify the package using the -// object in the Authentication field, and consider how much trust to give -// the result of this method depending on the authentication result: an -// unauthenticated result or one that only verified a checksum could be -// considered less trustworthy than one that checked the package against -// a signature provided by the origin registry. -// -// The AcceptableHashes result is actually provided by the object in the -// Authentication field. AcceptableHashes therefore returns an empty result -// for a PackageMeta that has no authentication object, or has one that does -// not make use of hashes. -func (m PackageMeta) AcceptableHashes() []Hash { - auth, ok := m.Authentication.(PackageAuthenticationHashes) - if !ok { - return nil - } - return auth.AcceptableHashes() -} - // PackageLocation represents a location where a provider distribution package // can be obtained. type PackageLocation interface { diff --git a/internal/providercache/installer.go b/internal/providercache/installer.go index fed13275c8..edf3a74b6d 100644 --- a/internal/providercache/installer.go +++ b/internal/providercache/installer.go @@ -9,6 +9,7 @@ import ( "context" "fmt" "log" + "slices" "sort" "strings" @@ -624,34 +625,50 @@ func (i *Installer) ensureProviderVersionsInstall( continue } - var signedHashes []getproviders.Hash - // For now, we will temporarily trust the hashes returned by the - // installation process that are "SigningSkipped" or "Signed". - // This is only intended to be temporary, see https://github.com/opentofu/opentofu/issues/266 for more information - if authResult.Signed() || authResult.SigningSkipped() { - // We'll trust new hashes from upstream only if they were verified - // as signed by a suitable key or if the signing validation was skipped. - // Otherwise, we'd record only - // a new hash we just calculated ourselves from the bytes on disk, - // and so the hashes would cover only the current platform. - signedHashes = append(signedHashes, meta.AcceptableHashes()...) - } + // localHashes is the set of hashes that we were able to verify locally + // based on the data we downloaded. + localHashes := slices.Collect(authResult.HashesWithDisposition(func(hd *getproviders.HashDisposition) bool { + return hd.VerifiedLocally + })) + localHashes = append(localHashes, newHash) // the hash we calculated above was _also_ verified locally + + // We have different rules for what subset of hashes we track in + // the dependency lock file depending on the provider. Refer to + // the documentation of the following function for more information. + signingRequired := getproviders.ShouldEnforceGPGValidationForProvider(provider) + signedHashes := slices.Collect(authResult.HashesWithDisposition(func(hd *getproviders.HashDisposition) bool { + if !signingRequired { + // When signing isn't required, we pretend that anything + // that was reported by the origin registry was "signed", + // just for the purposes of updating the lock file and + // reporting that lock file update to the UI layer through + // the evts object. + // Note that the "tofu init" UI relies on us pretending + // that these are "signed" to avoid generating its warning + // that the dependency lock file might be incomplete. + return hd.ReportedByRegistry + } + return hd.SignedByAnyGPGKeys() + })) var newHashes []getproviders.Hash newHashes = append(newHashes, newHash) newHashes = append(newHashes, priorHashes...) + newHashes = append(newHashes, localHashes...) newHashes = append(newHashes, signedHashes...) locks.SetProvider(provider, version, reqs[provider], newHashes) if cb := evts.ProvidersLockUpdated; cb != nil { - // newHash and priorHashes are already sorted. - // But we do need to sort signedHashes so we can reason about it - // sensibly. + // priorHashes is already sorted, but we do need to sort + // the newly-generated localHashes and signedHashes. + sort.Slice(localHashes, func(i, j int) bool { + return localHashes[i].String() < localHashes[j].String() + }) sort.Slice(signedHashes, func(i, j int) bool { - return string(signedHashes[i]) < string(signedHashes[j]) + return signedHashes[i].String() < signedHashes[j].String() }) - cb(provider, version, []getproviders.Hash{newHash}, signedHashes, priorHashes) + cb(provider, version, localHashes, signedHashes, priorHashes) } if cb := evts.FetchPackageSuccess; cb != nil { diff --git a/internal/providercache/installer_test.go b/internal/providercache/installer_test.go index 5decdad7b2..13fceaa9ef 100644 --- a/internal/providercache/installer_test.go +++ b/internal/providercache/installer_test.go @@ -6,12 +6,14 @@ package providercache import ( + "archive/zip" "context" "encoding/json" "fmt" "log" "net/http" "net/http/httptest" + "os" "path/filepath" "strings" "testing" @@ -49,11 +51,52 @@ func TestEnsureProviderVersions(t *testing.T) { // using in this test as the key for installer events that are not // specific to a particular provider. var noProvider addrs.Provider + + // beepProvider (and its associated derived values) represent a provider package + // fixture that's realistic enough to really be installed, although the + // plugin executable in it is really just a text file placeholder so it + // cannot actually be executed after installation. beepProvider := addrs.MustParseProviderSourceString("example.com/foo/beep") beepProviderDir := getproviders.PackageLocalDir("testdata/beep-provider") + beepProviderHash := getproviders.HashScheme1.New("2y06Ykj0FRneZfGCTxI9wRTori8iB7ZL5kQ6YyEnh84=") + + // We also derive a zip archive of the beep provider that we can use to test + // the slightly-different treatment of installation sources that can provide + // an uncompressed archive: specifically, that we can also calculate and record + // a "ziphash"-style hash that is based on the zip file itself instead of its content. + tempDir := t.TempDir() + beepProviderZip := getproviders.PackageLocalArchive(filepath.Join(tempDir, "beep-provider.zip")) + { + f, err := os.Create(string(beepProviderZip)) + if err != nil { + t.Fatal(err) + } + zw := zip.NewWriter(f) + err = zw.AddFS(os.DirFS(string(beepProviderDir))) + if err != nil { + t.Fatal(err) + } + err = zw.Close() + if err != nil { + t.Fatal(err) + } + err = f.Close() + if err != nil { + t.Fatal(err) + } + } + // We calculate this particular hash dynamically, rather than hard-coding it + // as we normally do for these situations in tests, because archive/zip + // can generate slightly different entry metadata depending on the platform + // where the test is running and other details of the execution environment. + // We only care that the final result uses a hash that matches the zip file. + beepProviderZipHash, err := getproviders.PackageHashLegacyZipSHA(beepProviderZip) + if err != nil { + t.Fatal(err) + } + fakePlatform := getproviders.Platform{OS: "bleep", Arch: "bloop"} wrongPlatform := getproviders.Platform{OS: "wrong", Arch: "wrong"} - beepProviderHash := getproviders.HashScheme1.New("2y06Ykj0FRneZfGCTxI9wRTori8iB7ZL5kQ6YyEnh84=") terraformProvider := addrs.MustParseProviderSourceString("terraform.io/builtin/terraform") tests := map[string]Test{ @@ -210,6 +253,143 @@ func TestEnsureProviderVersions(t *testing.T) { } }, }, + "successful initial install of one provider from a .zip archive": { + Source: getproviders.NewMockSource( + []getproviders.PackageMeta{ + { + Provider: beepProvider, + Version: getproviders.MustParseVersion("2.1.0"), + TargetPlatform: fakePlatform, + Location: beepProviderZip, + Authentication: getproviders.NewPackageHashAuthentication( + fakePlatform, []getproviders.Hash{beepProviderZipHash}, + ), + }, + }, + nil, + ), + Mode: InstallNewProvidersOnly, + Reqs: getproviders.Requirements{ + beepProvider: getproviders.MustParseVersionConstraints(">= 2.0.0"), + }, + Check: func(t *testing.T, dir *Dir, locks *depsfile.Locks) { + if allCached := dir.AllAvailablePackages(); len(allCached) != 1 { + t.Errorf("wrong number of cache directory entries; want only one\n%s", spew.Sdump(allCached)) + } + if allLocked := locks.AllProviders(); len(allLocked) != 1 { + t.Errorf("wrong number of provider lock entries; want only one\n%s", spew.Sdump(allLocked)) + } + + gotLock := locks.Provider(beepProvider) + wantLock := depsfile.NewProviderLock( + beepProvider, + getproviders.MustParseVersion("2.1.0"), + getproviders.MustParseVersionConstraints(">= 2.0.0"), + []getproviders.Hash{ + beepProviderHash, + beepProviderZipHash, // additional hash should appear when we install from zip + }, + ) + if diff := cmp.Diff(wantLock, gotLock, depsfile.ProviderLockComparer); diff != "" { + t.Errorf("wrong lock entry\n%s", diff) + } + + gotEntry := dir.ProviderLatestVersion(beepProvider) + wantEntry := &CachedProvider{ + Provider: beepProvider, + Version: getproviders.MustParseVersion("2.1.0"), + PackageDir: filepath.Join(dir.BasePath(), "example.com/foo/beep/2.1.0/bleep_bloop"), + } + if diff := cmp.Diff(wantEntry, gotEntry); diff != "" { + t.Errorf("wrong cache entry\n%s", diff) + } + }, + WantEvents: func(inst *Installer, dir *Dir) map[addrs.Provider][]*testInstallerEventLogItem { + return map[addrs.Provider][]*testInstallerEventLogItem{ + noProvider: { + { + Event: "PendingProviders", + Args: map[addrs.Provider]getproviders.VersionConstraints{ + beepProvider: getproviders.MustParseVersionConstraints(">= 2.0.0"), + }, + }, + { + Event: "ProvidersFetched", + Args: map[addrs.Provider]*getproviders.PackageAuthenticationResult{ + beepProvider: getproviders.NewPackageAuthenticationResult(getproviders.HashDispositions{ + beepProviderZipHash: { + VerifiedLocally: true, + }, + }), + }, + }, + }, + beepProvider: { + { + Event: "QueryPackagesBegin", + Provider: beepProvider, + Args: struct { + Constraints string + Locked bool + }{">= 2.0.0", false}, + }, + { + Event: "QueryPackagesSuccess", + Provider: beepProvider, + Args: "2.1.0", + }, + { + Event: "FetchPackageMeta", + Provider: beepProvider, + Args: "2.1.0", + }, + { + Event: "FetchPackageBegin", + Provider: beepProvider, + Args: struct { + Version string + Location getproviders.PackageLocation + }{"2.1.0", beepProviderZip}, + }, + { + Event: "ProvidersLockUpdated", + Provider: beepProvider, + Args: struct { + Version string + Local []getproviders.Hash + Signed []getproviders.Hash + Prior []getproviders.Hash + }{ + "2.1.0", + []getproviders.Hash{ + "h1:2y06Ykj0FRneZfGCTxI9wRTori8iB7ZL5kQ6YyEnh84=", + // When installing from a .zip archive and using + // getproviders.NewPackageHashAuthentication the lock file also + // records the "ziphash" of the .zip archive itself, whereas the + // "h1" hash is calculated from only the files inside the archive. + beepProviderZipHash, + }, + nil, + nil, + }, + }, + { + Event: "FetchPackageSuccess", + Provider: beepProvider, + Args: struct { + Version string + LocalDir string + AuthResult string + }{ + "2.1.0", + filepath.Join(dir.BasePath(), "example.com/foo/beep/2.1.0/bleep_bloop"), + "verified checksum", + }, + }, + }, + } + }, + }, "successful initial install of one provider through a cold global cache": { Source: getproviders.NewMockSource( []getproviders.PackageMeta{ @@ -2266,7 +2446,7 @@ func TestEnsureProviderVersions(t *testing.T) { if test.WantEvents != nil { wantEvents := test.WantEvents(inst, outputDir) - if diff := cmp.Diff(wantEvents, providerEvents); diff != "" { + if diff := cmp.Diff(wantEvents, providerEvents, cmp.AllowUnexported(getproviders.PackageAuthenticationResult{})); diff != "" { t.Errorf("wrong installer events\n%s", diff) } }