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) } }