Skip to content

Commit 00d8263

Browse files
committed
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.
1 parent 8666737 commit 00d8263

File tree

2 files changed

+32
-20
lines changed

2 files changed

+32
-20
lines changed

internal/getproviders/package_authentication.go

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@ import (
99
"crypto/sha256"
1010
"encoding/hex"
1111
"fmt"
12+
"io"
1213
"log"
1314
"strings"
1415

1516
"github.com/ProtonMail/go-crypto/openpgp"
1617
openpgpArmor "github.com/ProtonMail/go-crypto/openpgp/armor"
1718
openpgpErrors "github.com/ProtonMail/go-crypto/openpgp/errors"
18-
openpgpPacket "github.com/ProtonMail/go-crypto/openpgp/packet"
19+
"github.com/ProtonMail/go-crypto/openpgp/packet"
1920
)
2021

2122
type packageAuthenticationResult int
@@ -27,12 +28,6 @@ const (
2728
communityProvider
2829
)
2930

30-
var (
31-
// openpgpConfig is only populated during testing, so that a fake clock can be
32-
// injected, preventing signature expiration errors.
33-
openpgpConfig *openpgpPacket.Config
34-
)
35-
3631
// PackageAuthenticationResult is returned from a PackageAuthentication
3732
// implementation. It is a mostly-opaque type intended for use in UI, which
3833
// implements Stringer.
@@ -419,7 +414,7 @@ func (s signatureAuthentication) AuthenticatePackage(location PackageLocation) (
419414
if err != nil {
420415
return nil, fmt.Errorf("error creating HashiCorp keyring: %s", err)
421416
}
422-
_, err = openpgp.CheckDetachedSignature(hashicorpKeyring, bytes.NewReader(s.Document), bytes.NewReader(s.Signature), openpgpConfig)
417+
_, err = s.checkDetachedSignature(hashicorpKeyring, bytes.NewReader(s.Document), bytes.NewReader(s.Signature), nil)
423418
if err == nil {
424419
return &PackageAuthenticationResult{result: officialProvider, KeyID: keyID}, nil
425420
}
@@ -442,7 +437,7 @@ func (s signatureAuthentication) AuthenticatePackage(location PackageLocation) (
442437
return nil, fmt.Errorf("error decoding trust signature: %s", err)
443438
}
444439

445-
_, err = openpgp.CheckDetachedSignature(hashicorpPartnersKeyring, authorKey.Body, trustSignature.Body, openpgpConfig)
440+
_, err = s.checkDetachedSignature(hashicorpPartnersKeyring, authorKey.Body, trustSignature.Body, nil)
446441
if err != nil {
447442
return nil, fmt.Errorf("error verifying trust signature: %s", err)
448443
}
@@ -455,6 +450,27 @@ func (s signatureAuthentication) AuthenticatePackage(location PackageLocation) (
455450
return &PackageAuthenticationResult{result: communityProvider, KeyID: keyID}, nil
456451
}
457452

453+
func (s signatureAuthentication) checkDetachedSignature(keyring openpgp.KeyRing, signed, signature io.Reader, config *packet.Config) (*openpgp.Entity, error) {
454+
entity, err := openpgp.CheckDetachedSignature(keyring, signed, signature, config)
455+
// FIXME: it's not clear what should be done with provider signing key
456+
// expiration. This check reverts the validation behavior to match that of
457+
// the original x/crypto/openpgp package.
458+
//
459+
// We don't force providers to update keys for older releases, so they may
460+
// have since expired. We are validating the original signature however,
461+
// which was vouched for by the registry. The openpgp code always checks
462+
// signature details last, so we know if we have ErrKeyExpired all other
463+
// validation already passed. This is also checked in findSigningKey while
464+
// iterating over the possible signers.
465+
if err == openpgpErrors.ErrKeyExpired {
466+
for id := range entity.Identities {
467+
log.Printf("[WARN] expired openpgp key from %s\n", id)
468+
}
469+
err = nil
470+
}
471+
return entity, err
472+
}
473+
458474
func (s signatureAuthentication) AcceptableHashes() []Hash {
459475
// This is a bit of an abstraction leak because signatureAuthentication
460476
// otherwise just treats the document as an opaque blob that's been
@@ -513,14 +529,20 @@ func (s signatureAuthentication) findSigningKey() (*SigningKey, string, error) {
513529
return nil, "", fmt.Errorf("error decoding signing key: %s", err)
514530
}
515531

516-
entity, err := openpgp.CheckDetachedSignature(keyring, bytes.NewReader(s.Document), bytes.NewReader(s.Signature), openpgpConfig)
532+
entity, err := s.checkDetachedSignature(keyring, bytes.NewReader(s.Document), bytes.NewReader(s.Signature), nil)
517533

518534
// If the signature issuer does not match the the key, keep trying the
519535
// rest of the provided keys.
520536
if err == openpgpErrors.ErrUnknownIssuer {
521537
continue
522538
}
523539

540+
// we don't force providers to update keys, so they may be expired for
541+
// older releases.
542+
if err == openpgpErrors.ErrKeyExpired {
543+
err = nil
544+
}
545+
524546
// Any other signature error is terminal.
525547
if err != nil {
526548
return nil, "", fmt.Errorf("error checking signature: %s", err)

internal/getproviders/package_authentication_test.go

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -11,23 +11,13 @@ import (
1111
"os"
1212
"strings"
1313
"testing"
14-
"time"
1514

1615
"github.com/google/go-cmp/cmp"
1716

1817
"github.com/ProtonMail/go-crypto/openpgp"
19-
"github.com/ProtonMail/go-crypto/openpgp/packet"
2018
)
2119

2220
func TestMain(m *testing.M) {
23-
openpgpConfig = &packet.Config{
24-
Time: func() time.Time {
25-
// Scientifically chosen time that satisfies the validity periods of all
26-
// of the keys and signatures used.
27-
t, _ := time.Parse(time.RFC3339, "2021-04-25T16:00:00-07:00")
28-
return t
29-
},
30-
}
3121
os.Exit(m.Run())
3222
}
3323

0 commit comments

Comments
 (0)