From 9413123c0f95990e50e66684704111f51a23e26c Mon Sep 17 00:00:00 2001 From: Caleb Doxsey Date: Fri, 11 Nov 2022 14:14:30 -0700 Subject: [PATCH] config: generate cookie secret if not set in all-in-one mode (#3742) * config: generate cookie secret if not set in all-in-one mode * fix tests * config: add warning about cookie_secret * breakup lines --- authenticate/authenticate_test.go | 3 --- config/envoyconfig/protocols.go | 2 +- config/options.go | 11 +++++++- config/options_test.go | 36 +++++++++++++++++++++----- internal/log/warnings.go | 42 +++++++++++++++++++++++++++++++ internal/syncutil/syncutil.go | 27 ++++++++++++++++++++ pkg/cryptutil/tls.go | 4 +-- proxy/proxy_test.go | 3 --- 8 files changed, 111 insertions(+), 17 deletions(-) create mode 100644 internal/log/warnings.go create mode 100644 internal/syncutil/syncutil.go diff --git a/authenticate/authenticate_test.go b/authenticate/authenticate_test.go index 255d6725a..124e8c2a4 100644 --- a/authenticate/authenticate_test.go +++ b/authenticate/authenticate_test.go @@ -32,8 +32,6 @@ func TestOptions_Validate(t *testing.T) { emptyClientID.ClientID = "" emptyClientSecret := newTestOptions(t) emptyClientSecret.ClientSecret = "" - emptyCookieSecret := newTestOptions(t) - emptyCookieSecret.CookieSecret = "" invalidCookieSecret := newTestOptions(t) invalidCookieSecret.CookieSecret = "OromP1gurwGWjQPYb1nNgSxtbVB5NnLzX6z5WOKr0Yw^" shortCookieLength := newTestOptions(t) @@ -53,7 +51,6 @@ func TestOptions_Validate(t *testing.T) { }{ {"minimum options", good, false}, {"nil options", &config.Options{}, true}, - {"no cookie secret", emptyCookieSecret, true}, {"invalid cookie secret", invalidCookieSecret, true}, {"short cookie secret", shortCookieLength, true}, {"no shared secret", badSharedKey, true}, diff --git a/config/envoyconfig/protocols.go b/config/envoyconfig/protocols.go index c286ba76c..b10dda6f0 100644 --- a/config/envoyconfig/protocols.go +++ b/config/envoyconfig/protocols.go @@ -100,7 +100,7 @@ func getUpstreamProtocolForPolicy(ctx context.Context, policy *config.Policy) up upstreamProtocol := upstreamProtocolAuto if policy.AllowWebsockets { // #2388, force http/1 when using web sockets - log.Info(ctx).Msg("envoyconfig: forcing http/1.1 due to web socket support") + log.WarnWebSocketHTTP1_1(getClusterID(policy)) upstreamProtocol = upstreamProtocolHTTP1 } return upstreamProtocol diff --git a/config/options.go b/config/options.go index 1f708e1fb..02dabcc46 100644 --- a/config/options.go +++ b/config/options.go @@ -985,7 +985,7 @@ func (o *Options) GetSharedKey() ([]byte, error) { sharedKey = string(bs) } // mutual auth between services on the same host can be generated at runtime - if IsAll(o.Services) && o.SharedKey == "" && o.DataBrokerStorageType == StorageInMemoryName { + if IsAll(o.Services) && sharedKey == "" { sharedKey = randomSharedKey } if sharedKey == "" { @@ -1188,6 +1188,15 @@ func (o *Options) GetCookieSecret() ([]byte, error) { } cookieSecret = string(bs) } + + if IsAll(o.Services) && cookieSecret == "" { + log.WarnCookieSecret() + cookieSecret = randomSharedKey + } + if cookieSecret == "" { + return nil, errors.New("empty cookie secret") + } + return base64.StdEncoding.DecodeString(cookieSecret) } diff --git a/config/options_test.go b/config/options_test.go index 1dc20bd3a..2397d562f 100644 --- a/config/options_test.go +++ b/config/options_test.go @@ -53,11 +53,6 @@ func Test_Validate(t *testing.T) { badSignoutRedirectURL := testOptions() badSignoutRedirectURL.SignOutRedirectURLString = "--" - missingSharedSecretWithPersistence := testOptions() - missingSharedSecretWithPersistence.SharedKey = "" - missingSharedSecretWithPersistence.DataBrokerStorageType = StorageRedisName - missingSharedSecretWithPersistence.DataBrokerStorageConnectionString = "redis://somehost:6379" - tests := []struct { name string testOpts *Options @@ -71,7 +66,6 @@ func Test_Validate(t *testing.T) { {"invalid databroker storage type", invalidStorageType, true}, {"missing databroker storage dsn", missingStorageDSN, true}, {"invalid signout redirect url", badSignoutRedirectURL, true}, - {"no shared key with databroker persistence", missingSharedSecretWithPersistence, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -776,6 +770,36 @@ func TestOptions_GetSetResponseHeaders(t *testing.T) { }) } +func TestOptions_GetSharedKey(t *testing.T) { + t.Run("default", func(t *testing.T) { + o := NewDefaultOptions() + bs, err := o.GetSharedKey() + assert.NoError(t, err) + assert.Equal(t, randomSharedKey, base64.StdEncoding.EncodeToString(bs)) + }) + t.Run("missing", func(t *testing.T) { + o := NewDefaultOptions() + o.Services = ServiceProxy + _, err := o.GetSharedKey() + assert.Error(t, err) + }) +} + +func TestOptions_GetCookieSecret(t *testing.T) { + t.Run("default", func(t *testing.T) { + o := NewDefaultOptions() + bs, err := o.GetCookieSecret() + assert.NoError(t, err) + assert.Equal(t, randomSharedKey, base64.StdEncoding.EncodeToString(bs)) + }) + t.Run("missing", func(t *testing.T) { + o := NewDefaultOptions() + o.Services = ServiceProxy + _, err := o.GetCookieSecret() + assert.Error(t, err) + }) +} + func encodeCert(cert *tls.Certificate) []byte { return pem.EncodeToMemory(&pem.Block{Type: "CERTIFICATE", Bytes: cert.Certificate[0]}) } diff --git a/internal/log/warnings.go b/internal/log/warnings.go new file mode 100644 index 000000000..75085d2db --- /dev/null +++ b/internal/log/warnings.go @@ -0,0 +1,42 @@ +package log + +import ( + "context" + "sync" + + "github.com/pomerium/pomerium/internal/syncutil" +) + +var warnCookieSecretOnce sync.Once + +// WarnCookieSecret warns about the cookie secret. +func WarnCookieSecret() { + warnCookieSecretOnce.Do(func() { + Warn(context.Background()). + Msg("using a generated COOKIE_SECRET. " + + "Set the COOKIE_SECRET to avoid users being logged out on restart. " + + "https://www.pomerium.com/docs/reference/cookie-secret") + }) +} + +var warnNoTLSCertificateOnce syncutil.OnceMap[string] + +// WarnNoTLSCertificate warns about no TLS certificate. +func WarnNoTLSCertificate(domain string) { + warnNoTLSCertificateOnce.Do(domain, func() { + Warn(context.Background()). + Str("domain", domain). + Msg("no TLS certificate found for domain, using a self-signed certificate") + }) +} + +var warnWebSocketHTTP1_1Once syncutil.OnceMap[string] + +// WarnWebSocketHTTP1_1 warns about falling back to http 1.1 due to web socket support. +func WarnWebSocketHTTP1_1(clusterID string) { + warnWebSocketHTTP1_1Once.Do(clusterID, func() { + Warn(context.Background()). + Str("cluster-id", clusterID). + Msg("forcing http/1.1 due to web socket support") + }) +} diff --git a/internal/syncutil/syncutil.go b/internal/syncutil/syncutil.go new file mode 100644 index 000000000..1bdf28ef7 --- /dev/null +++ b/internal/syncutil/syncutil.go @@ -0,0 +1,27 @@ +// Package syncutil contains methods for working with sync code. +package syncutil + +import ( + "sync" +) + +// A OnceMap is a collection sync.Onces accessible by a key. The zero value is usable. +type OnceMap[T comparable] struct { + mu sync.Mutex + m map[T]*sync.Once +} + +// Do runs f once. +func (o *OnceMap[T]) Do(key T, f func()) { + o.mu.Lock() + if o.m == nil { + o.m = make(map[T]*sync.Once) + } + oo, ok := o.m[key] + if !ok { + oo = new(sync.Once) + o.m[key] = oo + } + o.mu.Unlock() + oo.Do(f) +} diff --git a/pkg/cryptutil/tls.go b/pkg/cryptutil/tls.go index 3531bce7b..93f0e073e 100644 --- a/pkg/cryptutil/tls.go +++ b/pkg/cryptutil/tls.go @@ -55,9 +55,7 @@ func GetCertificateForDomain(certificates []tls.Certificate, domain string) (*tl } } - log.Error(context.Background()). - Str("domain", domain). - Msg("cryptutil: no TLS certificate found for domain, using self-signed certificate") + log.WarnNoTLSCertificate(domain) // finally fall back to a generated, self-signed certificate return GenerateSelfSignedCertificate(domain) diff --git a/proxy/proxy_test.go b/proxy/proxy_test.go index 44d3c0a11..8ec634442 100644 --- a/proxy/proxy_test.go +++ b/proxy/proxy_test.go @@ -41,8 +41,6 @@ func TestOptions_Validate(t *testing.T) { badAuthURL.AuthenticateURLString = "BAD_URL" authenticateBadScheme := testOptions(t) authenticateBadScheme.AuthenticateURLString = "authenticate.corp.beyondperimeter.com" - emptyCookieSecret := testOptions(t) - emptyCookieSecret.CookieSecret = "" invalidCookieSecret := testOptions(t) invalidCookieSecret.CookieSecret = "OromP1gurwGWjQPYb1nNgSxtbVB5NnLzX6z5WOKr0Yw^" shortCookieLength := testOptions(t) @@ -62,7 +60,6 @@ func TestOptions_Validate(t *testing.T) { }{ {"good - minimum options", good, false}, {"nil options", &config.Options{}, true}, - {"no cookie secret", emptyCookieSecret, true}, {"invalid cookie secret", invalidCookieSecret, true}, {"short cookie secret", shortCookieLength, true}, {"no shared secret", badSharedKey, true},