Merge pull request #34004 from hashicorp/jbardin/openpgp-key-expiration

cli: ignore expired provider signing keys from registry during init

Author: jbardin

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.

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

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:  <nil>\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())
 			}