Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: ignore expired provider signing keys from registry during init #34004

Merged
merged 2 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 26 additions & 10 deletions internal/getproviders/package_authentication.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down
10 changes: 0 additions & 10 deletions internal/getproviders/package_authentication_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}

Expand Down
3 changes: 0 additions & 3 deletions internal/getproviders/registry_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package getproviders
import (
"context"
"encoding/json"
"fmt"
"log"
"net/http"
"net/http/httptest"
Expand Down Expand Up @@ -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())
}
Expand Down