diff --git a/internal/app/bootstrap/bootstrap.go b/internal/app/bootstrap/bootstrap.go index 3138614ff..34a3451ba 100644 --- a/internal/app/bootstrap/bootstrap.go +++ b/internal/app/bootstrap/bootstrap.go @@ -18,6 +18,7 @@ package bootstrap import ( "context" "crypto/rand" + "crypto/subtle" "encoding/hex" "fmt" "log" @@ -294,11 +295,12 @@ func (s *Service) Claim(ctx context.Context, token, address, password, firstName remaining, _ := s.cache.PTTL(ctx, setupTokenKey).Result() stored, gerr := s.cache.GetDel(ctx, setupTokenKey).Result() - if gerr != nil || stored == "" || stored != hashToken(token) { + matches := subtle.ConstantTimeCompare([]byte(stored), []byte(hashToken(token))) == 1 + if gerr != nil || stored == "" || !matches { // Put a valid-but-losing token back only when the value did not match, // so a typo does not burn the real one, and only for the time it had // left. - if gerr == nil && stored != "" && stored != hashToken(token) && remaining > 0 { + if gerr == nil && stored != "" && !matches && remaining > 0 { _ = s.cache.SetEx(ctx, setupTokenKey, stored, remaining).Err() } return nil, errx.ErrSetupToken diff --git a/internal/app/instancecheck/checks_security.go b/internal/app/instancecheck/checks_security.go index 6b48ea210..c31374587 100644 --- a/internal/app/instancecheck/checks_security.go +++ b/internal/app/instancecheck/checks_security.go @@ -87,7 +87,7 @@ func checkCredentialsKeyUnset(ctx context.Context, d Deps, in Input) *Finding { return nil } return result(CategorySecurity, SeverityError, "Mailbox credentials are not sealed", - "CREDENTIALS_ENCRYPTION_KEY is not set, so mailbox SMTP and IMAP passwords are stored without being sealed. "+ + "CREDENTIALS_ENCRYPTION_KEY is not set, so mailbox credentials and webhook signing secrets cannot be stored and those connections are refused. "+ "Set a 64 hex character key (`openssl rand -hex 32`) before connecting any mailbox. "+ "Back it up: losing it makes connected mailboxes unrecoverable.", docsEncryption) diff --git a/internal/client/smtpimap/imap/client.go b/internal/client/smtpimap/imap/client.go index bef9e2826..078777d73 100644 --- a/internal/client/smtpimap/imap/client.go +++ b/internal/client/smtpimap/imap/client.go @@ -173,6 +173,7 @@ func (c *Client) connectLocked() *errx.MailError { tlsConf := &tls.Config{ ServerName: host, + MinVersion: tls.VersionTLS12, InsecureSkipVerify: netbind.InsecureTLS(), //nolint:gosec // MAIL_TLS_INSECURE, local dev only } diff --git a/internal/repository/pg_oauth.go b/internal/repository/pg_oauth.go index f7c672766..c1d6f2553 100644 --- a/internal/repository/pg_oauth.go +++ b/internal/repository/pg_oauth.go @@ -85,9 +85,12 @@ func NewOAuthRepositorySealed(db *pgxpool.Pool, enc *encrypt.Encrypter) OAuthRep const webhookSecretPrefix = "whsec_" func (r *oauthRepository) sealWebhookSecret(plain string) (string, error) { - if r.enc == nil || plain == "" { + if plain == "" { return plain, nil } + if r.enc == nil { + return "", errNoCredentialKey + } return r.enc.Encrypt(plain) } diff --git a/internal/repository/pg_webhook.go b/internal/repository/pg_webhook.go index 11ccdfc13..99c1bca84 100644 --- a/internal/repository/pg_webhook.go +++ b/internal/repository/pg_webhook.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "strings" "time" "github.com/google/uuid" @@ -103,12 +104,17 @@ func NewWebhookRepositorySealed(db *pgxpool.Pool, enc *encrypt.Encrypter) Webhoo return &webhookRepository{db: db, enc: enc} } -// sealSecret encrypts a signing secret for storage. Without an encrypter it -// stores what it was given, matching the behaviour before sealing existed. +// errNoCredentialKey refuses to store a signing secret that could not be sealed. +var errNoCredentialKey = errors.New("CREDENTIALS_ENCRYPTION_KEY is not set, so a signing secret cannot be stored") + +// sealSecret encrypts a signing secret for storage, and refuses without the instance key. func (r *webhookRepository) sealSecret(plain string) (string, error) { - if r.enc == nil || plain == "" { + if plain == "" { return plain, nil } + if r.enc == nil { + return "", errNoCredentialKey + } return r.enc.Encrypt(plain) } @@ -126,7 +132,11 @@ func (r *webhookRepository) openSecret(stored string) (string, bool) { if plain, err := r.enc.Decrypt(stored); err == nil { return plain, false } - return stored, true + // Only a plaintext token is legacy; anything else is ciphertext under another key. + if strings.HasPrefix(stored, "whsec_") { + return stored, true + } + return "", false } // endpointCols is the shared column projection so every read scans identically. @@ -271,6 +281,9 @@ func (r *webhookRepository) GetEndpointSecret(ctx context.Context, endpointID uu } plain, legacy := r.openSecret(secret) + if plain == "" && secret != "" { + return "", errors.New("webhook signing secret is unreadable under this instance's key; rotate it") + } if legacy { // Re-seal on first read, the same way mailbox credentials convert, so // the plaintext window closes on its own rather than waiting for the diff --git a/internal/repository/webhook_event_filter_live_test.go b/internal/repository/webhook_event_filter_live_test.go index 59a778208..2304ce5ba 100644 --- a/internal/repository/webhook_event_filter_live_test.go +++ b/internal/repository/webhook_event_filter_live_test.go @@ -5,6 +5,7 @@ import ( "testing" "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/pkg/encrypt" ) // An omitted event filter is the documented "every non-firehose event", which @@ -17,7 +18,11 @@ import ( func TestLiveWebhookEndpointEmptyEventFilter(t *testing.T) { _, pool := liveContactDB(t) f := newSharedOrgFixture(t, pool) - repo := NewWebhookRepository(pool) + enc, err := encrypt.NewEncrypter(make([]byte, 32)) + if err != nil { + t.Fatal(err) + } + repo := NewWebhookRepositorySealed(pool, enc) ctx := context.Background() endpoint := &models.WebhookEndpoint{ diff --git a/site/public/install.sh b/site/public/install.sh index a7d73ed63..e72578192 100644 --- a/site/public/install.sh +++ b/site/public/install.sh @@ -1598,6 +1598,16 @@ render_caddyfile() { } } +# Customer-owned domains get the same headers minus HSTS: a pin sent from a +# customer's apex would cover every subdomain they run elsewhere. +(customer_headers) { + header { + X-Content-Type-Options "nosniff" + Referrer-Policy "strict-origin-when-cross-origin" + -Server + } +} + $H_APP { import warmbly_headers reverse_proxy web:80 @@ -1655,7 +1665,7 @@ render_caddy_custom_domains() { # obtains the certificate on the first request, after /tls/authorize confirms # this instance has verified the name. https:// { - import warmbly_headers + import customer_headers tls { on_demand } diff --git a/site/public/install.sh.sha256 b/site/public/install.sh.sha256 index 40928fe5c..02ecf7c18 100644 --- a/site/public/install.sh.sha256 +++ b/site/public/install.sh.sha256 @@ -1 +1 @@ -3bb7282129a03b70a4c73351656ffa85b9b998da2f8fb7b85127c90d8bb78ace install.sh +098221eed28b874d84250518a001bbf38e3e74520c00bc48d3cc1328d2107f56 install.sh