From 312a6cdde443d204974c93c1059eaf96ff822a60 Mon Sep 17 00:00:00 2001 From: Andrey Ermolov Date: Thu, 2 Nov 2023 01:02:52 +0000 Subject: [PATCH 1/4] Add initial scan as a part of FWCP creation --- security/advancedtls/crl_provider.go | 7 ++++--- security/advancedtls/crl_provider_test.go | 6 ++---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/security/advancedtls/crl_provider.go b/security/advancedtls/crl_provider.go index 590906169f3..069c405314a 100644 --- a/security/advancedtls/crl_provider.go +++ b/security/advancedtls/crl_provider.go @@ -109,8 +109,9 @@ type FileWatcherCRLProvider struct { // NewFileWatcherCRLProvider returns a new instance of the // FileWatcherCRLProvider. It uses FileWatcherOptions to validate and apply -// configuration required for creating a new instance. Users should call Close -// to stop the background refresh of CRLDirectory. +// configuration required for creating a new instance. The initial scan of +// CRLDirectory is performed inside this function. Users should call Close to +// stop the background refresh of CRLDirectory. func NewFileWatcherCRLProvider(o FileWatcherOptions) (*FileWatcherCRLProvider, error) { if err := o.validate(); err != nil { return nil, err @@ -121,6 +122,7 @@ func NewFileWatcherCRLProvider(o FileWatcherOptions) (*FileWatcherCRLProvider, e stop: make(chan struct{}), done: make(chan struct{}), } + provider.scanCRLDirectory() go provider.run() return provider, nil } @@ -149,7 +151,6 @@ func (p *FileWatcherCRLProvider) run() { defer close(p.done) ticker := time.NewTicker(p.opts.RefreshDuration) defer ticker.Stop() - p.scanCRLDirectory() for { select { diff --git a/security/advancedtls/crl_provider_test.go b/security/advancedtls/crl_provider_test.go index 5245f58895a..bd200475373 100644 --- a/security/advancedtls/crl_provider_test.go +++ b/security/advancedtls/crl_provider_test.go @@ -45,6 +45,7 @@ func (s) TestStaticCRLProvider(t *testing.T) { rawCRLs = append(rawCRLs, rawCRL) } p := NewStaticCRLProvider(rawCRLs) + // Each test data entry contains a description of a certificate chain, // certificate chain itself, and if CRL is not expected to be found. tests := []struct { @@ -154,10 +155,6 @@ func (s) TestFileWatcherCRLProvider(t *testing.T) { t.Fatal("Unexpected error while creating FileWatcherCRLProvider:", err) } - // We need to make sure that initial CRLDirectory scan is completed before - // querying the internal map. - p.Close() - // Each test data entry contains a description of a certificate chain, // certificate chain itself, and if CRL is not expected to be found. tests := []struct { @@ -197,6 +194,7 @@ func (s) TestFileWatcherCRLProvider(t *testing.T) { } }) } + p.Close() if diff := cmp.Diff(len(nonCRLFilesSet), nonCRLFilesUnderCRLDirectory); diff != "" { t.Errorf("Unexpected number Number of callback executions\ndiff (-got +want):\n%s", diff) } From d97b6a9d8a92dfa26a4e302cfd9019fc03bc90db Mon Sep 17 00:00:00 2001 From: Andrey Ermolov Date: Thu, 2 Nov 2023 01:22:38 +0000 Subject: [PATCH 2/4] Add comment about default value for RefreshDuration --- security/advancedtls/crl_provider.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/security/advancedtls/crl_provider.go b/security/advancedtls/crl_provider.go index 069c405314a..6777be0c555 100644 --- a/security/advancedtls/crl_provider.go +++ b/security/advancedtls/crl_provider.go @@ -91,7 +91,7 @@ func (p *StaticCRLProvider) CRL(cert *x509.Certificate) (*CRL, error) { // FileWatcherCRLProvider. type FileWatcherOptions struct { CRLDirectory string // Path of the directory containing CRL files - RefreshDuration time.Duration // Time interval between CRLDirectory scans, can't be smaller than 1 minute + RefreshDuration time.Duration // Time interval (default value is 1 hour) between CRLDirectory scans, can't be smaller than 1 minute CRLReloadingFailedCallback func(err error) // Custom callback executed when a CRL file can’t be processed } From 465ebacc5ccbf673f6e4476ae3e757f20c3d7058 Mon Sep 17 00:00:00 2001 From: Andrey Ermolov Date: Mon, 6 Nov 2023 16:40:58 +0000 Subject: [PATCH 3/4] Promote Close() to the interface level --- security/advancedtls/crl_provider.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/security/advancedtls/crl_provider.go b/security/advancedtls/crl_provider.go index 6777be0c555..e30f81f5c18 100644 --- a/security/advancedtls/crl_provider.go +++ b/security/advancedtls/crl_provider.go @@ -52,6 +52,9 @@ type CRLProvider interface { // // [RFC5280 - Undetermined]: https://datatracker.ietf.org/doc/html/rfc5280#section-6.3.3 CRL(cert *x509.Certificate) (*CRL, error) + + // Close cleans up resources allocated by the provider. + Close() } // StaticCRLProvider implements CRLProvider interface by accepting raw content @@ -87,6 +90,9 @@ func (p *StaticCRLProvider) CRL(cert *x509.Certificate) (*CRL, error) { return p.crls[cert.Issuer.ToRDNSequence().String()], nil } +// Close is a no-op. +func (p *StaticCRLProvider) Close() {} + // FileWatcherOptions represents a data structure holding a configuration for // FileWatcherCRLProvider. type FileWatcherOptions struct { From 6101672030da142f780f83d68e69b7dfef10511c Mon Sep 17 00:00:00 2001 From: Andrey Ermolov Date: Wed, 8 Nov 2023 17:49:49 +0000 Subject: [PATCH 4/4] Revert "Promote Close() to the interface level" This reverts commit 465ebacc5ccbf673f6e4476ae3e757f20c3d7058. --- security/advancedtls/crl_provider.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/security/advancedtls/crl_provider.go b/security/advancedtls/crl_provider.go index e30f81f5c18..6777be0c555 100644 --- a/security/advancedtls/crl_provider.go +++ b/security/advancedtls/crl_provider.go @@ -52,9 +52,6 @@ type CRLProvider interface { // // [RFC5280 - Undetermined]: https://datatracker.ietf.org/doc/html/rfc5280#section-6.3.3 CRL(cert *x509.Certificate) (*CRL, error) - - // Close cleans up resources allocated by the provider. - Close() } // StaticCRLProvider implements CRLProvider interface by accepting raw content @@ -90,9 +87,6 @@ func (p *StaticCRLProvider) CRL(cert *x509.Certificate) (*CRL, error) { return p.crls[cert.Issuer.ToRDNSequence().String()], nil } -// Close is a no-op. -func (p *StaticCRLProvider) Close() {} - // FileWatcherOptions represents a data structure holding a configuration for // FileWatcherCRLProvider. type FileWatcherOptions struct {