From bae0fb29bc0b142f2a640f37e0ac39a9fa453165 Mon Sep 17 00:00:00 2001 From: Easwar Swaminathan Date: Fri, 24 Jul 2020 16:27:31 -0700 Subject: [PATCH] certprovider: API update to include certificate name. --- credentials/tls/certprovider/distributor.go | 40 +- .../tls/certprovider/distributor_test.go | 21 +- credentials/tls/certprovider/provider.go | 33 +- credentials/tls/certprovider/store.go | 165 +++++-- credentials/tls/certprovider/store_test.go | 437 ++++++++++-------- 5 files changed, 424 insertions(+), 272 deletions(-) diff --git a/credentials/tls/certprovider/distributor.go b/credentials/tls/certprovider/distributor.go index d6feb3afeba1..fd4f85ed8659 100644 --- a/credentials/tls/certprovider/distributor.go +++ b/credentials/tls/certprovider/distributor.go @@ -25,24 +25,27 @@ import ( "google.golang.org/grpc/internal/grpcsync" ) -// Distributor makes it easy for provider implementations to furnish new key -// materials by handling synchronization between the producer and consumers of -// the key material. +// Distributor is a simple, single key materials cache which makes it easy for +// provider implementations to furnish new key materials by handling +// synchronization between the producer and consumers of the key material. // -// Provider implementations which choose to use a Distributor should do the -// following: -// - create a new Distributor using the NewDistributor() function. -// - invoke the Set() method whenever they have new key material or errors to -// report. -// - delegate to the distributor when handing calls to KeyMaterial(). -// - invoke the Stop() method when they are done using the distributor. +// Distributor implements the KeyMaterialReader interface. Provider +// implementations may choose to do the following: +// - return a distributor as part of their KeyMaterialReader() method. +// - invoke the distributor's Set() method whenever they have new key material. +// - watch the channel returned by the distributor's Done() method to get +// notified when the distributor is closed. type Distributor struct { // mu protects the underlying key material. mu sync.Mutex km *KeyMaterial pErr error - ready *grpcsync.Event + // ready channel to unblock KeyMaterialReader() invocations blocked on + // availability of key material. + ready *grpcsync.Event + // done channel to notify provider implementations and unblock any + // KeyMaterialReader() calls, once the distributor is closed. closed *grpcsync.Event } @@ -57,12 +60,12 @@ func NewDistributor() *Distributor { // Set updates the key material in the distributor with km. // // Provider implementations which use the distributor must not modify the -// contents of the KeyMaterial struct pointed to by km. +// contents of the KeyMaterialReader struct pointed to by km. // // A non-nil err value indicates the error that the provider implementation ran // into when trying to fetch key material, and makes it possible to surface the // error to the user. A non-nil error value passed here causes distributor's -// KeyMaterial() method to return nil key material. +// KeyMaterialReader() method to return nil key material. func (d *Distributor) Set(km *KeyMaterial, err error) { d.mu.Lock() d.km = km @@ -103,8 +106,13 @@ func (d *Distributor) keyMaterial() (*KeyMaterial, error) { return d.km, d.pErr } -// Stop turns down the distributor, releases allocated resources and fails any -// active KeyMaterial() call waiting for new key material. -func (d *Distributor) Stop() { +// Close turns down the distributor, releases allocated resources and fails any +// active KeyMaterialReader() call waiting for new key material. +func (d *Distributor) Close() { d.closed.Fire() } + +// Done returns a channel which will be closed when the distributor is closed. +func (d *Distributor) Done() <-chan struct{} { + return d.closed.Done() +} diff --git a/credentials/tls/certprovider/distributor_test.go b/credentials/tls/certprovider/distributor_test.go index e6c41564c293..a2bd72033eaf 100644 --- a/credentials/tls/certprovider/distributor_test.go +++ b/credentials/tls/certprovider/distributor_test.go @@ -36,10 +36,8 @@ func (s) TestDistributor(t *testing.T) { dist := NewDistributor() // Read cert/key files from testdata. - km, err := loadKeyMaterials() - if err != nil { - t.Fatal(err) - } + km := loadKeyMaterials(t, "x509/server1_cert.pem", "x509/server1_key.pem", "x509/client_ca_cert.pem") + // wantKM1 has both local and root certs. wantKM1 := *km // wantKM2 has only local certs. Roots are nil-ed out. @@ -56,7 +54,7 @@ func (s) TestDistributor(t *testing.T) { go func() { defer wg.Done() - // The first call to KeyMaterial() should timeout because no key + // The first call to KeyMaterialReader() should timeout because no key // material has been set on the distributor as yet. ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout/2) defer cancel() @@ -66,7 +64,7 @@ func (s) TestDistributor(t *testing.T) { } proceedCh <- struct{}{} - // This call to KeyMaterial() should return the key material with both + // This call to KeyMaterialReader() should return the key material with both // the local certs and the root certs. ctx, cancel = context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() @@ -76,11 +74,11 @@ func (s) TestDistributor(t *testing.T) { return } if !reflect.DeepEqual(gotKM, &wantKM1) { - errCh <- fmt.Errorf("provider.KeyMaterial() = %+v, want %+v", gotKM, wantKM1) + errCh <- fmt.Errorf("provider.KeyMaterialReader() = %+v, want %+v", gotKM, wantKM1) } proceedCh <- struct{}{} - // This call to KeyMaterial() should eventually return key material with + // This call to KeyMaterialReader() should eventually return key material with // only the local certs. ctx, cancel = context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() @@ -96,7 +94,7 @@ func (s) TestDistributor(t *testing.T) { } proceedCh <- struct{}{} - // This call to KeyMaterial() should return nil key material and a + // This call to KeyMaterialReader() should return nil key material and a // non-nil error. ctx, cancel = context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() @@ -114,7 +112,7 @@ func (s) TestDistributor(t *testing.T) { } proceedCh <- struct{}{} - // This call to KeyMaterial() should eventually return errProviderClosed + // This call to KeyMaterialReader() should eventually return errProviderClosed // error. ctx, cancel = context.WithTimeout(context.Background(), defaultTestTimeout) defer cancel() @@ -139,9 +137,8 @@ func (s) TestDistributor(t *testing.T) { }) waitAndDo(t, proceedCh, errCh, func() { - dist.Stop() + dist.Close() }) - } func waitAndDo(t *testing.T, proceedCh chan struct{}, errCh chan error, do func()) { diff --git a/credentials/tls/certprovider/provider.go b/credentials/tls/certprovider/provider.go index 58c93c1e731f..a082cd3eacbc 100644 --- a/credentials/tls/certprovider/provider.go +++ b/credentials/tls/certprovider/provider.go @@ -32,7 +32,7 @@ import ( ) var ( - // errProviderClosed is returned by Distributor.KeyMaterial when it is + // errProviderClosed is returned by Distributor.KeyMaterialReader when it is // closed. errProviderClosed = errors.New("provider instance is closed") @@ -87,14 +87,27 @@ type StableConfig interface { // the latest secrets, and free to share any state between different // instantiations as they deem fit. type Provider interface { - // KeyMaterial returns the key material sourced by the provider. - // Callers are expected to use the returned value as read-only. - KeyMaterial(ctx context.Context) (*KeyMaterial, error) + // KeyMaterialReader returns a reader to read key material sourced by the + // provider. Callers are expected to call the Close() method on the returned + // reader when they are no longer interested in the underlying key material. + KeyMaterialReader(opts KeyMaterialOptions) (KeyMaterialReader, error) // Close cleans up resources allocated by the provider. Close() } +// KeyMaterialReader is used to read key material at handshake time. +// Implementations must be thread-safe. +type KeyMaterialReader interface { + // Returns the key material when they are ready. + // Implementations must honor any deadline set in the context. + KeyMaterial(ctx context.Context) (*KeyMaterial, error) + + // Close provides a way for callers to indicate that they are no longer + // interested in the underlying key material. + Close() +} + // KeyMaterial wraps the certificates and keys returned by a provider instance. type KeyMaterial struct { // Certs contains a slice of cert/key pairs used to prove local identity. @@ -102,3 +115,15 @@ type KeyMaterial struct { // Roots contains the set of trusted roots to validate the peer's identity. Roots *x509.CertPool } + +// KeyMaterialOptions wraps options passed to the KeyMaterialReader() method. +type KeyMaterialOptions struct { + // CertName holds the certificate name, whose key material is of interest to + // the caller. + CertName string + // WantRoot indicates if the caller is interested in the root certificate. + WantRoot bool + // WantIdentity indicates if the caller is interested in the identity + // certificate. + WantIdentity bool +} diff --git a/credentials/tls/certprovider/store.go b/credentials/tls/certprovider/store.go index 0d2b29b675b1..95c7925225e9 100644 --- a/credentials/tls/certprovider/store.go +++ b/credentials/tls/certprovider/store.go @@ -25,49 +25,53 @@ import ( // provStore is the global singleton certificate provider store. var provStore = &store{ - providers: make(map[storeKey]*wrappedProvider), + providers: make(map[providerKey]*wrappedProvider), + distributors: make(map[distributorKey]*wrappedDistributor), } -// storeKey acts as the key to the map of providers maintained by the store. A +// providerKey acts as the key to a map of providers maintained by the store. A // combination of provider name and configuration is used to uniquely identify // every provider instance in the store. Go maps need to be indexed by // comparable types, so the provider configuration is converted from // `interface{}` to string using the ParseConfig method while creating this key. -type storeKey struct { +type providerKey struct { // name of the certificate provider. name string // configuration of the certificate provider in string form. config string } -// wrappedProvider wraps a provider instance with a reference count. -type wrappedProvider struct { - Provider - refCount int - - // A reference to the key and store are also kept here to override the - // Close method on the provider. - storeKey storeKey - store *store +// distributorKey acts as the key to map of distributors maintained by the +// store. These distributor instances are returned by the provider +// implementation's GetKeyMaterial() method. +type distributorKey struct { + // name of the certificate provider. + name string + // configuration of the certificate provider in string form. + config string + // options for reading key material. + options KeyMaterialOptions } // store is a collection of provider instances, safe for concurrent access. type store struct { - mu sync.Mutex - providers map[storeKey]*wrappedProvider + mu sync.Mutex + providers map[providerKey]*wrappedProvider + distributors map[distributorKey]*wrappedDistributor } -// GetProvider returns a provider instance corresponding to name and config. -// name is the registered name of the provider and config is the -// provider-specific configuration. Implementations of the Builder interface -// should clearly document the type of configuration accepted by them. +// GetKeyMaterialReader returns a reader to read key materials from. // -// If a provider exists for the (name+config) combination, its reference count -// is incremented before returning. If no provider exists for the (name+config) -// combination, a new one is created using the registered builder. If no -// registered builder is found, or the provider configuration is rejected by it, -// a non-nil error is returned. -func GetProvider(name string, config interface{}) (Provider, error) { +// name is the registered name of the provider, config is the provider-specific +// configuration, and opts contains key material specific options. +// +// If no provider exists for the (name+config) combination in the store, a new +// one is created using the registered builder. If not, the existing provider is +// used. If no registered builder is found, or the provider configuration is +// rejected by it, a non-nil error is returned. If no distributor exists for the +// (name+config+opts) combo in the store, a new one is created by calling the +// KeyMaterialReader() method on the provider. +func GetKeyMaterialReader(name string, config interface{}, opts KeyMaterialOptions) (KeyMaterialReader, error) { provStore.mu.Lock() defer provStore.mu.Unlock() @@ -79,42 +83,107 @@ func GetProvider(name string, config interface{}) (Provider, error) { if err != nil { return nil, err } - - sk := storeKey{ + pk := providerKey{ name: name, config: string(stableConfig.Canonical()), } - if wp, ok := provStore.providers[sk]; ok { - wp.refCount++ - return wp, nil + + // Create a new provider for the (name+config) combo or use an existing one. + var wp *wrappedProvider + wp, ok := provStore.providers[pk] + if !ok { + provider := builder.Build(stableConfig) + if provider == nil { + return nil, fmt.Errorf("certprovider.Build(%v) failed", pk) + } + wp = &wrappedProvider{ + prov: provider, + storeKey: pk, + store: provStore, + } + provStore.providers[pk] = wp } - provider := builder.Build(stableConfig) - if provider == nil { - return nil, fmt.Errorf("certprovider.Build(%v) failed", sk) + // Create a new distributor for the (name+config+opts) combo or use an + // existing one. The reference count for the provider is incremented only + // when creating a new distributor. + dk := distributorKey{ + name: name, + config: string(stableConfig.Canonical()), + options: opts, } - wp := &wrappedProvider{ - Provider: provider, - refCount: 1, - storeKey: sk, - store: provStore, + if wd, ok := provStore.distributors[dk]; ok { + wd.refCount++ + return wd, nil } - provStore.providers[sk] = wp - return wp, nil + distributor, err := wp.prov.KeyMaterialReader(opts) + if err != nil { + return nil, err + } + wd := &wrappedDistributor{ + KeyMaterialReader: distributor, + refCount: 1, + wp: wp, + storeKey: dk, + store: provStore, + } + provStore.distributors[dk] = wd + wp.refCount++ + return wd, nil } -// Close overrides the Close method of the embedded provider. It releases the -// reference held by the caller on the underlying provider and if the -// provider's reference count reaches zero, it is removed from the store, and -// its Close method is also invoked. -func (wp *wrappedProvider) Close() { - ps := wp.store - ps.mu.Lock() - defer ps.mu.Unlock() +// wrappedProvider wraps a provider instance with a reference count. +type wrappedProvider struct { + prov Provider + // refcount tracks the number of KeyMaterialReader instances (or + // distributors) doled out by this provider. + refCount int + // A reference to the key and store are also kept here to invoke the + // Close method on the provider, and to remove it from the store map. + storeKey providerKey + store *store +} +// close releases the reference held by the caller on the underlying provider +// and if the provider's reference count reaches zero, it is removed from the +// store, and its Close method is also invoked. +// +// Caller must hold store mutex. +func (wp *wrappedProvider) close() { + ps := wp.store wp.refCount-- if wp.refCount == 0 { - wp.Provider.Close() + wp.prov.Close() delete(ps.providers, wp.storeKey) } } + +// wrappedDistributor wraps a distributor instance with a reference count. +type wrappedDistributor struct { + KeyMaterialReader + refCount int + // The underlying provider which is the source for key material stored in + // this distributor. + wp *wrappedProvider + // A reference to the key and store are also kept here to invoke the + // Close method on the distributor, and to remove it from the store map. + storeKey distributorKey + store *store +} + +// Close overrides the underlying distributor's Close() method. It updates the +// reference count, and when it reaches zero, calls Close() on the underlying +// distributor, removes it from the store map, and also calls close() on the +// associated wrapped provider to update its reference count. +func (wd *wrappedDistributor) Close() { + ps := wd.store + ps.mu.Lock() + defer ps.mu.Unlock() + + wd.refCount-- + if wd.refCount == 0 { + wd.KeyMaterialReader.Close() + delete(ps.distributors, wd.storeKey) + wd.wp.close() + } +} diff --git a/credentials/tls/certprovider/store_test.go b/credentials/tls/certprovider/store_test.go index 33135d41c854..7d8919cf0490 100644 --- a/credentials/tls/certprovider/store_test.go +++ b/credentials/tls/certprovider/store_test.go @@ -1,3 +1,5 @@ +// +build go1.13 + /* * * Copyright 2020 gRPC authors. @@ -22,13 +24,19 @@ import ( "context" "crypto/tls" "crypto/x509" + "errors" "fmt" "io/ioutil" - "reflect" + "math/big" + "sync" "testing" "time" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "google.golang.org/grpc/internal/grpctest" + "google.golang.org/grpc/internal/testutils" "google.golang.org/grpc/testdata" ) @@ -36,12 +44,23 @@ const ( fakeProvider1Name = "fake-certificate-provider-1" fakeProvider2Name = "fake-certificate-provider-2" fakeConfig = "my fake config" + providerChanSize = 2 defaultTestTimeout = 1 * time.Second ) +var fpb1, fpb2 *fakeProviderBuilder + func init() { - Register(&fakeProviderBuilder{name: fakeProvider1Name}) - Register(&fakeProviderBuilder{name: fakeProvider2Name}) + fpb1 = &fakeProviderBuilder{ + name: fakeProvider1Name, + providerChan: testutils.NewChannelWithSize(providerChanSize), + } + fpb2 = &fakeProviderBuilder{ + name: fakeProvider2Name, + providerChan: testutils.NewChannelWithSize(providerChanSize), + } + Register(fpb1) + Register(fpb2) } type s struct { @@ -55,25 +74,20 @@ func Test(t *testing.T) { // fakeProviderBuilder builds new instances of fakeProvider and interprets the // config provided to it as a string. type fakeProviderBuilder struct { - name string + name string + providerChan *testutils.Channel } func (b *fakeProviderBuilder) Build(StableConfig) Provider { - ctx, cancel := context.WithCancel(context.Background()) - p := &fakeProvider{ - Distributor: NewDistributor(), - cancel: cancel, - done: make(chan struct{}), - kmCh: make(chan *KeyMaterial, 2), - } - go p.run(ctx) + p := &fakeProvider{distributors: make(map[KeyMaterialOptions]*Distributor)} + b.providerChan.Send(p) return p } func (b *fakeProviderBuilder) ParseConfig(config interface{}) (StableConfig, error) { s, ok := config.(string) if !ok { - return nil, fmt.Errorf("provider %s received bad config %v", b.name, config) + return nil, fmt.Errorf("providerBuilder %s received config of type %T, want string", b.name, config) } return &fakeStableConfig{config: s}, nil } @@ -90,262 +104,301 @@ func (c *fakeStableConfig) Canonical() []byte { return []byte(c.config) } -// fakeProvider is an implementation of the Provider interface which embeds a -// Distributor and exposes two channels for the user: -// 1. to be notified when the provider is closed -// 2. to push new key material into the provider +// fakeProvider is an implementation of the Provider interface which provides a +// method for tests to invoke to push new key materials. type fakeProvider struct { - *Distributor - - // Used to cancel the run goroutine when the provider is closed. - cancel context.CancelFunc - // This channel is closed when the provider is closed. Tests should block on - // this to make sure the provider is closed. - done chan struct{} - // Tests can push new key material on this channel, and the provider will - // return this on subsequent calls to KeyMaterial(). - kmCh chan *KeyMaterial + mu sync.Mutex + distributors map[KeyMaterialOptions]*Distributor } -func (p *fakeProvider) run(ctx context.Context) { - for { - select { - case <-ctx.Done(): - return - case km := <-p.kmCh: - p.Distributor.Set(km, nil) - } +// newKeyMaterial allows tests to push new key material to the fake provider +// which will be made available to users of this provider. +func (p *fakeProvider) newKeyMaterial(opts KeyMaterialOptions, km *KeyMaterial, err error) { + p.mu.Lock() + dist, ok := p.distributors[opts] + if !ok { + dist = NewDistributor() + p.distributors[opts] = dist } + dist.Set(km, err) + p.mu.Unlock() } +// KeyMaterialReader helps implement the Provider interface. +func (p *fakeProvider) KeyMaterialReader(opts KeyMaterialOptions) (KeyMaterialReader, error) { + p.mu.Lock() + defer p.mu.Unlock() + + dist, ok := p.distributors[opts] + if !ok { + dist = NewDistributor() + p.distributors[opts] = dist + } + return dist, nil +} + +// Close helps implement the Provider interface. func (p *fakeProvider) Close() { - p.cancel() - p.Distributor.Stop() + p.mu.Lock() + for _, d := range p.distributors { + d.Close() + } + p.mu.Unlock() } // loadKeyMaterials is a helper to read cert/key files from testdata and convert -// them into a KeyMaterial struct. -func loadKeyMaterials() (*KeyMaterial, error) { - certs, err := tls.LoadX509KeyPair(testdata.Path("x509/server1_cert.pem"), testdata.Path("x509/server1_key.pem")) +// them into a KeyMaterialReader struct. +func loadKeyMaterials(t *testing.T, cert, key, ca string) *KeyMaterial { + t.Helper() + + certs, err := tls.LoadX509KeyPair(testdata.Path(cert), testdata.Path(key)) if err != nil { - return nil, err + t.Fatalf("Failed to load keyPair: %v", err) } - pemData, err := ioutil.ReadFile(testdata.Path("x509/client_ca_cert.pem")) + pemData, err := ioutil.ReadFile(testdata.Path(ca)) if err != nil { - return nil, err + t.Fatal(err) } roots := x509.NewCertPool() roots.AppendCertsFromPEM(pemData) - return &KeyMaterial{Certs: []tls.Certificate{certs}, Roots: roots}, nil + return &KeyMaterial{Certs: []tls.Certificate{certs}, Roots: roots} } -func makeProvider(t *testing.T, name, config string) (Provider, *fakeProvider) { - t.Helper() +// readAndVerifyKeyMaterial attempts to read key material from the provider +// reader and compares it against the expected key material. +func readAndVerifyKeyMaterial(kmr KeyMaterialReader, wantKM *KeyMaterial) error { + ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) + defer cancel() - prov, err := GetProvider(name, config) + gotKM, err := kmr.KeyMaterial(ctx) if err != nil { - t.Fatal(err) + return fmt.Errorf("KeyMaterial(ctx) failed: %v", err) } - - // The store returns a wrappedProvider, which holds a reference to the - // actual provider, which in our case in the fakeProvider. - wp := prov.(*wrappedProvider) - fp := wp.Provider.(*fakeProvider) - return prov, fp + if !cmp.Equal(gotKM, wantKM, cmpopts.IgnoreUnexported(big.Int{}), cmpopts.IgnoreUnexported(x509.CertPool{}), cmpopts.EquateEmpty()) { + return fmt.Errorf("KeyMaterial(ctx) returned %+v, want %+v", gotKM, wantKM) + } + return nil } -// TestStoreWithSingleProvider creates a single provider through the store and -// calls methods on it. -func (s) TestStoreWithSingleProvider(t *testing.T) { - prov, fp := makeProvider(t, fakeProvider1Name, fakeConfig) - - // Push key materials into the provider. - wantKM, err := loadKeyMaterials() +// TestStoreSingleProviderSingleReader creates a single provider and single +// reader through the store and calls methods on them. +func (s) TestStoreSingleProviderSingleReader(t *testing.T) { + // Create a KeyMaterialReader through the store. This will internally create + // a provider and a distributor. + kmOpts := KeyMaterialOptions{CertName: "default"} + kmReader, err := GetKeyMaterialReader(fakeProvider1Name, fakeConfig, kmOpts) if err != nil { - t.Fatal(err) + t.Fatalf("GetKeyMaterialReader(%s, %s, %v) failed: %v", fakeProvider1Name, fakeConfig, kmOpts, err) } - fp.kmCh <- wantKM + defer kmReader.Close() - // Get key materials from the provider and compare it to the ones we pushed - // above. - ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) - defer cancel() - gotKM, err := prov.KeyMaterial(ctx) + // Our fakeProviderBuilder pushes newly created providers on a channel. Grab + // the fake provider from that channel. + p, err := fpb1.providerChan.Receive() if err != nil { - t.Fatalf("provider.KeyMaterial() = %v", err) + t.Fatalf("Timeout when expecting certProvider %q to be created", fakeProvider1Name) } - // TODO(easwars): Remove all references to reflect.DeepEqual and use - // cmp.Equal instead. Currently, the later panics because x509.Certificate - // type defines an Equal method, but does not check for nil. This has been - // fixed in - // https://github.com/golang/go/commit/89865f8ba64ccb27f439cce6daaa37c9aa38f351, - // but this is only available starting go1.14. So, once we remove support - // for go1.13, we can make the switch. - if !reflect.DeepEqual(gotKM, wantKM) { - t.Fatalf("provider.KeyMaterial() = %+v, want %+v", gotKM, wantKM) + prov := p.(*fakeProvider) + + // Attempt to read from key material from the reader returned by the store. + // This will fail because we have not pushed any key material into our fake + // provider. + if err := readAndVerifyKeyMaterial(kmReader, nil); errors.Is(err, context.DeadlineExceeded) { + t.Fatal(err) } - // Close the provider and retry the KeyMaterial() call, and expect it to - // fail with a known error. - prov.Close() - if _, err := prov.KeyMaterial(ctx); err != errProviderClosed { - t.Fatalf("provider.KeyMaterial() = %v, wantErr: %v", err, errProviderClosed) + // Load key material from testdata directory, push it into out fakeProvider + // and attempt to read from the reader returned by the store. + testKM1 := loadKeyMaterials(t, "x509/server1_cert.pem", "x509/server1_key.pem", "x509/client_ca_cert.pem") + prov.newKeyMaterial(kmOpts, testKM1, nil) + if err := readAndVerifyKeyMaterial(kmReader, testKM1); err != nil { + t.Fatal(err) } -} -// TestStoreWithSingleProviderWithSharing creates multiple instances of the same -// type of provider through the store (and expects the store's sharing mechanism -// to kick in) and calls methods on it. -func (s) TestStoreWithSingleProviderWithSharing(t *testing.T) { - prov1, fp1 := makeProvider(t, fakeProvider1Name, fakeConfig) - prov2, _ := makeProvider(t, fakeProvider1Name, fakeConfig) + // Push new key material and read from the reader. This should returned + // updated key material. + testKM2 := loadKeyMaterials(t, "x509/server2_cert.pem", "x509/server2_key.pem", "x509/client_ca_cert.pem") + prov.newKeyMaterial(kmOpts, testKM2, nil) + if err := readAndVerifyKeyMaterial(kmReader, testKM2); err != nil { + t.Fatal(err) + } +} - // Push key materials into the fake provider1. - wantKM, err := loadKeyMaterials() +// TestStoreSingleProviderMultipleReaders creates key material readers on a +// single provider through the store (and expects the store's sharing mechanism +// to kick in) and calls methods on them. +func (s) TestStoreSingleProviderMultipleReaders(t *testing.T) { + // Create three readers on the same fake provider. Two of these readers use + // certName `foo`, while the third one uses certName `bar`. + optsFoo := KeyMaterialOptions{CertName: "foo"} + optsBar := KeyMaterialOptions{CertName: "bar"} + readerFoo1, err := GetKeyMaterialReader(fakeProvider1Name, fakeConfig, optsFoo) if err != nil { - t.Fatal(err) + t.Fatalf("GetKeyMaterialReader(%s, %s, %v) failed: %v", fakeProvider1Name, fakeConfig, optsFoo, err) } - fp1.kmCh <- wantKM + defer readerFoo1.Close() + readerFoo2, err := GetKeyMaterialReader(fakeProvider1Name, fakeConfig, optsFoo) + if err != nil { + t.Fatalf("GetKeyMaterialReader(%s, %s, %v) failed: %v", fakeProvider1Name, fakeConfig, optsFoo, err) + } + defer readerFoo2.Close() + readerBar1, err := GetKeyMaterialReader(fakeProvider1Name, fakeConfig, optsBar) + if err != nil { + t.Fatalf("GetKeyMaterialReader(%s, %s, %v) failed: %v", fakeProvider1Name, fakeConfig, optsBar, err) + } + defer readerBar1.Close() - // Get key materials from the fake provider2 and compare it to the ones we - // pushed above. - ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) - defer cancel() - gotKM, err := prov2.KeyMaterial(ctx) + // Our fakeProviderBuilder pushes newly created providers on a channel. Grab + // the fake provider from that channel. + p, err := fpb1.providerChan.Receive() if err != nil { - t.Fatalf("provider.KeyMaterial() = %v", err) + t.Fatalf("Timeout when expecting certProvider %q to be created", fakeProvider1Name) } - if !reflect.DeepEqual(gotKM, wantKM) { - t.Fatalf("provider.KeyMaterial() = %+v, want %+v", gotKM, wantKM) + prov := p.(*fakeProvider) + + // Make sure the store does not create multiple providers here. + if _, err := fpb1.providerChan.Receive(); err != testutils.ErrRecvTimeout { + t.Fatal("Multiple certProviders created when only one was expected") } - // Close the provider1 and retry the KeyMaterial() call on prov2, and expect - // it to succeed. - prov1.Close() - if _, err := prov2.KeyMaterial(ctx); err != nil { - t.Fatalf("provider.KeyMaterial() = %v", err) + // Push key material for optsFoo, and make sure the foo readers return + // appropriate key material and the bar reader times out. + fooKM := loadKeyMaterials(t, "x509/server1_cert.pem", "x509/server1_key.pem", "x509/client_ca_cert.pem") + prov.newKeyMaterial(optsFoo, fooKM, nil) + if err := readAndVerifyKeyMaterial(readerFoo1, fooKM); err != nil { + t.Fatal(err) + } + if err := readAndVerifyKeyMaterial(readerFoo2, fooKM); err != nil { + t.Fatal(err) + } + if err := readAndVerifyKeyMaterial(readerBar1, nil); errors.Is(err, context.DeadlineExceeded) { + t.Fatal(err) } - prov2.Close() - if _, err := prov2.KeyMaterial(ctx); err != errProviderClosed { - t.Fatalf("provider.KeyMaterial() = %v, wantErr: %v", err, errProviderClosed) + // Push key material for optsBar, and make sure the bar reader returns appropriate key material. + barKM := loadKeyMaterials(t, "x509/server2_cert.pem", "x509/server2_key.pem", "x509/client_ca_cert.pem") + prov.newKeyMaterial(optsBar, barKM, nil) + if err := readAndVerifyKeyMaterial(readerBar1, barKM); err != nil { + t.Fatal(err) + } + + // Make sure the above push of new key material does not affect foo readers. + if err := readAndVerifyKeyMaterial(readerFoo1, fooKM); err != nil { + t.Fatal(err) } } -// TestStoreWithSingleProviderWithoutSharing creates multiple instances of the -// same type of provider through the store with different configs. The store +// TestStoreMultipleProviderInstancesOfSameType creates multiple instances of +// the same type of provider through the store with different configs. The store // would end up creating different provider instances for these and no sharing // would take place. -func (s) TestStoreWithSingleProviderWithoutSharing(t *testing.T) { - prov1, fp1 := makeProvider(t, fakeProvider1Name, fakeConfig+"1111") - prov2, fp2 := makeProvider(t, fakeProvider1Name, fakeConfig+"2222") - - // Push the same key materials into the two providers. - wantKM, err := loadKeyMaterials() +func (s) TestStoreMultipleProviderInstancesOfSameType(t *testing.T) { + // Create three readers on the same fake provider. Two of these readers use + // certName `foo`, while the third one uses certName `bar`. + opts := KeyMaterialOptions{CertName: "foo"} + cfg1 := fakeConfig + "1111" + cfg2 := fakeConfig + "2222" + reader1, err := GetKeyMaterialReader(fakeProvider1Name, cfg1, opts) if err != nil { - t.Fatal(err) + t.Fatalf("GetKeyMaterialReader(%s, %s, %v) failed: %v", fakeProvider1Name, cfg1, opts, err) } - fp1.kmCh <- wantKM - fp2.kmCh <- wantKM - - // Get key materials from the fake provider1 and compare it to the ones we - // pushed above. - ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) - defer cancel() - gotKM, err := prov1.KeyMaterial(ctx) + defer reader1.Close() + reader2, err := GetKeyMaterialReader(fakeProvider1Name, cfg2, opts) if err != nil { - t.Fatalf("provider.KeyMaterial() = %v", err) - } - if !reflect.DeepEqual(gotKM, wantKM) { - t.Fatalf("provider.KeyMaterial() = %+v, want %+v", gotKM, wantKM) + t.Fatalf("GetKeyMaterialReader(%s, %s, %v) failed: %v", fakeProvider1Name, cfg2, opts, err) } + defer reader2.Close() - // Get key materials from the fake provider2 and compare it to the ones we - // pushed above. - gotKM, err = prov2.KeyMaterial(ctx) + // Our fakeProviderBuilder pushes newly created providers on a channel. Grab + // the fake provider from that channel. + p1, err := fpb1.providerChan.Receive() if err != nil { - t.Fatalf("provider.KeyMaterial() = %v", err) - } - if !reflect.DeepEqual(gotKM, wantKM) { - t.Fatalf("provider.KeyMaterial() = %+v, want %+v", gotKM, wantKM) + t.Fatalf("Timeout when expecting certProvider %q to be created", fakeProvider1Name) } + prov1 := p1.(*fakeProvider) - // Update the key materials used by provider1, and make sure provider2 is - // not affected. - newKM, err := loadKeyMaterials() + p2, err := fpb1.providerChan.Receive() if err != nil { - t.Fatal(err) + t.Fatalf("Timeout when expecting certProvider %q to be created", fakeProvider1Name) } - newKM.Roots = nil - fp1.kmCh <- newKM + prov2 := p2.(*fakeProvider) - gotKM, err = prov2.KeyMaterial(ctx) - if err != nil { - t.Fatalf("provider.KeyMaterial() = %v", err) + // Push the same key material into both providers and verify that the + // readers return the appropriate key material. + km1 := loadKeyMaterials(t, "x509/server1_cert.pem", "x509/server1_key.pem", "x509/client_ca_cert.pem") + prov1.newKeyMaterial(opts, km1, nil) + prov2.newKeyMaterial(opts, km1, nil) + if err := readAndVerifyKeyMaterial(reader1, km1); err != nil { + t.Fatal(err) } - if !reflect.DeepEqual(gotKM, wantKM) { - t.Fatalf("provider.KeyMaterial() = %+v, want %+v", gotKM, wantKM) + if err := readAndVerifyKeyMaterial(reader2, km1); err != nil { + t.Fatal(err) } - // Close the provider1 and retry the KeyMaterial() call on prov2, and expect - // it to succeed. - prov1.Close() - if _, err := prov2.KeyMaterial(ctx); err != nil { - t.Fatalf("provider.KeyMaterial() = %v", err) + // Push new key material into only one of the providers and and verify that + // the readers return the appropriate key material. + km2 := loadKeyMaterials(t, "x509/server2_cert.pem", "x509/server2_key.pem", "x509/client_ca_cert.pem") + prov2.newKeyMaterial(opts, km2, nil) + if err := readAndVerifyKeyMaterial(reader1, km1); err != nil { + t.Fatal(err) } - - prov2.Close() - if _, err := prov2.KeyMaterial(ctx); err != errProviderClosed { - t.Fatalf("provider.KeyMaterial() = %v, wantErr: %v", err, errProviderClosed) + if err := readAndVerifyKeyMaterial(reader2, km2); err != nil { + t.Fatal(err) } -} - -// TestStoreWithMultipleProviders creates multiple providers of different types -// and make sure closing of one does not affect the other. -func (s) TestStoreWithMultipleProviders(t *testing.T) { - prov1, fp1 := makeProvider(t, fakeProvider1Name, fakeConfig) - prov2, fp2 := makeProvider(t, fakeProvider2Name, fakeConfig) - // Push key materials into the fake providers. - wantKM, err := loadKeyMaterials() - if err != nil { + // Close one of the readers and verify that the other one is not affected. + reader1.Close() + if err := readAndVerifyKeyMaterial(reader2, km2); err != nil { t.Fatal(err) } - fp1.kmCh <- wantKM - fp2.kmCh <- wantKM +} - // Get key materials from the fake provider1 and compare it to the ones we - // pushed above. - ctx, cancel := context.WithTimeout(context.Background(), defaultTestTimeout) - defer cancel() - gotKM, err := prov1.KeyMaterial(ctx) +// TestStoreMultipleProviders creates providers of different types and makes +// sure closing of one does not affect the other. +func (s) TestStoreMultipleProviders(t *testing.T) { + opts := KeyMaterialOptions{CertName: "foo"} + reader1, err := GetKeyMaterialReader(fakeProvider1Name, fakeConfig, opts) if err != nil { - t.Fatalf("provider.KeyMaterial() = %v", err) + t.Fatalf("GetKeyMaterialReader(%s, %s, %v) failed: %v", fakeProvider1Name, fakeConfig, opts, err) } - if !reflect.DeepEqual(gotKM, wantKM) { - t.Fatalf("provider.KeyMaterial() = %+v, want %+v", gotKM, wantKM) + defer reader1.Close() + reader2, err := GetKeyMaterialReader(fakeProvider2Name, fakeConfig, opts) + if err != nil { + t.Fatalf("GetKeyMaterialReader(%s, %s, %v) failed: %v", fakeProvider1Name, fakeConfig, opts, err) } + defer reader2.Close() - // Get key materials from the fake provider2 and compare it to the ones we - // pushed above. - gotKM, err = prov2.KeyMaterial(ctx) + // Our fakeProviderBuilder pushes newly created providers on a channel. Grab + // the fake provider from that channel. + p1, err := fpb1.providerChan.Receive() if err != nil { - t.Fatalf("provider.KeyMaterial() = %v", err) + t.Fatalf("Timeout when expecting certProvider %q to be created", fakeProvider1Name) } - if !reflect.DeepEqual(gotKM, wantKM) { - t.Fatalf("provider.KeyMaterial() = %+v, want %+v", gotKM, wantKM) + prov1 := p1.(*fakeProvider) + + p2, err := fpb2.providerChan.Receive() + if err != nil { + t.Fatalf("Timeout when expecting certProvider %q to be created", fakeProvider2Name) } + prov2 := p2.(*fakeProvider) - // Close the provider1 and retry the KeyMaterial() call on prov2, and expect - // it to succeed. - prov1.Close() - if _, err := prov2.KeyMaterial(ctx); err != nil { - t.Fatalf("provider.KeyMaterial() = %v", err) + // Push the key material into both providers and verify that the + // readers return the appropriate key material. + km1 := loadKeyMaterials(t, "x509/server1_cert.pem", "x509/server1_key.pem", "x509/client_ca_cert.pem") + prov1.newKeyMaterial(opts, km1, nil) + km2 := loadKeyMaterials(t, "x509/server2_cert.pem", "x509/server2_key.pem", "x509/client_ca_cert.pem") + prov2.newKeyMaterial(opts, km2, nil) + if err := readAndVerifyKeyMaterial(reader1, km1); err != nil { + t.Fatal(err) + } + if err := readAndVerifyKeyMaterial(reader2, km2); err != nil { + t.Fatal(err) } - prov2.Close() - if _, err := prov2.KeyMaterial(ctx); err != errProviderClosed { - t.Fatalf("provider.KeyMaterial() = %v, wantErr: %v", err, errProviderClosed) + // Close one of the readers and verify that the other one is not affected. + reader1.Close() + if err := readAndVerifyKeyMaterial(reader2, km2); err != nil { + t.Fatal(err) } }