From 86667375fd18b9e081dfd401d02f6e4f2e6c5fe4 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 6 Oct 2023 10:00:24 -0400 Subject: [PATCH 1/2] remove errant debug print --- internal/getproviders/registry_client_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/internal/getproviders/registry_client_test.go b/internal/getproviders/registry_client_test.go index d2267d2d6e0b..73d96ff4af46 100644 --- a/internal/getproviders/registry_client_test.go +++ b/internal/getproviders/registry_client_test.go @@ -6,7 +6,6 @@ package getproviders import ( "context" "encoding/json" - "fmt" "log" "net/http" "net/http/httptest" @@ -449,8 +448,6 @@ func TestFindClosestProtocolCompatibleVersion(t *testing.T) { t.Fatalf("wrong error\ngot: \nwant: %s", test.wantErr) } - fmt.Printf("Got: %s, Want: %s\n", got, test.wantSuggestion) - if !got.Same(test.wantSuggestion) { t.Fatalf("wrong result\ngot: %s\nwant: %s", got.String(), test.wantSuggestion.String()) } From 0d4a29f7f3968d3d7d9199f9c5bd10759acb934b Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 6 Oct 2023 10:00:50 -0400 Subject: [PATCH 2/2] ignore expired provider signing keys Provider developers are not currently required to keep the signing keys stored in the registry up to date, and older releases may be signed with a key which has since expired. For our purposes here however, we are validating the key and signature used at the time of publishing, and given that the registry has previously vouched for the validity of the key used, we can continue to trust that key returned by the registry for installation. --- .../getproviders/package_authentication.go | 36 +++++++++++++------ .../package_authentication_test.go | 10 ------ 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/internal/getproviders/package_authentication.go b/internal/getproviders/package_authentication.go index b7a98b277c1f..2335eff9548f 100644 --- a/internal/getproviders/package_authentication.go +++ b/internal/getproviders/package_authentication.go @@ -9,13 +9,14 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "io" "log" "strings" "github.com/ProtonMail/go-crypto/openpgp" openpgpArmor "github.com/ProtonMail/go-crypto/openpgp/armor" openpgpErrors "github.com/ProtonMail/go-crypto/openpgp/errors" - openpgpPacket "github.com/ProtonMail/go-crypto/openpgp/packet" + "github.com/ProtonMail/go-crypto/openpgp/packet" ) type packageAuthenticationResult int @@ -27,12 +28,6 @@ const ( communityProvider ) -var ( - // openpgpConfig is only populated during testing, so that a fake clock can be - // injected, preventing signature expiration errors. - openpgpConfig *openpgpPacket.Config -) - // PackageAuthenticationResult is returned from a PackageAuthentication // implementation. It is a mostly-opaque type intended for use in UI, which // implements Stringer. @@ -419,7 +414,7 @@ func (s signatureAuthentication) AuthenticatePackage(location PackageLocation) ( if err != nil { return nil, fmt.Errorf("error creating HashiCorp keyring: %s", err) } - _, err = openpgp.CheckDetachedSignature(hashicorpKeyring, bytes.NewReader(s.Document), bytes.NewReader(s.Signature), openpgpConfig) + _, err = s.checkDetachedSignature(hashicorpKeyring, bytes.NewReader(s.Document), bytes.NewReader(s.Signature), nil) if err == nil { return &PackageAuthenticationResult{result: officialProvider, KeyID: keyID}, nil } @@ -442,7 +437,7 @@ func (s signatureAuthentication) AuthenticatePackage(location PackageLocation) ( return nil, fmt.Errorf("error decoding trust signature: %s", err) } - _, err = openpgp.CheckDetachedSignature(hashicorpPartnersKeyring, authorKey.Body, trustSignature.Body, openpgpConfig) + _, err = s.checkDetachedSignature(hashicorpPartnersKeyring, authorKey.Body, trustSignature.Body, nil) if err != nil { return nil, fmt.Errorf("error verifying trust signature: %s", err) } @@ -455,6 +450,27 @@ func (s signatureAuthentication) AuthenticatePackage(location PackageLocation) ( return &PackageAuthenticationResult{result: communityProvider, KeyID: keyID}, nil } +func (s signatureAuthentication) checkDetachedSignature(keyring openpgp.KeyRing, signed, signature io.Reader, config *packet.Config) (*openpgp.Entity, error) { + entity, err := openpgp.CheckDetachedSignature(keyring, signed, signature, config) + // FIXME: it's not clear what should be done with provider signing key + // expiration. This check reverts the validation behavior to match that of + // the original x/crypto/openpgp package. + // + // We don't force providers to update keys for older releases, so they may + // have since expired. We are validating the original signature however, + // which was vouched for by the registry. The openpgp code always checks + // signature details last, so we know if we have ErrKeyExpired all other + // validation already passed. This is also checked in findSigningKey while + // iterating over the possible signers. + if err == openpgpErrors.ErrKeyExpired { + for id := range entity.Identities { + log.Printf("[WARN] expired openpgp key from %s\n", id) + } + err = nil + } + return entity, err +} + 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 @@ -513,7 +529,7 @@ func (s signatureAuthentication) findSigningKey() (*SigningKey, string, error) { return nil, "", fmt.Errorf("error decoding signing key: %s", err) } - entity, err := openpgp.CheckDetachedSignature(keyring, bytes.NewReader(s.Document), bytes.NewReader(s.Signature), openpgpConfig) + entity, err := s.checkDetachedSignature(keyring, bytes.NewReader(s.Document), bytes.NewReader(s.Signature), nil) // If the signature issuer does not match the the key, keep trying the // rest of the provided keys. diff --git a/internal/getproviders/package_authentication_test.go b/internal/getproviders/package_authentication_test.go index bdfee402ac51..59bcef44532d 100644 --- a/internal/getproviders/package_authentication_test.go +++ b/internal/getproviders/package_authentication_test.go @@ -11,23 +11,13 @@ import ( "os" "strings" "testing" - "time" "github.com/google/go-cmp/cmp" "github.com/ProtonMail/go-crypto/openpgp" - "github.com/ProtonMail/go-crypto/openpgp/packet" ) func TestMain(m *testing.M) { - openpgpConfig = &packet.Config{ - Time: func() time.Time { - // Scientifically chosen time that satisfies the validity periods of all - // of the keys and signatures used. - t, _ := time.Parse(time.RFC3339, "2021-04-25T16:00:00-07:00") - return t - }, - } os.Exit(m.Run()) }