From d9e8402dc27bd11137e9301cf14eaf01d2786efd Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Mon, 23 Nov 2020 19:18:24 +0530 Subject: [PATCH] MM-30319: Fix race in ensureAsymmetricSigningKey (#16354) A cluster changed listener always runs in a separate goroutine. This causes a race condition when a new member gets added in the cluster. We fix it by accessing the value atomically. I tried to write a test for this, but it wasn't triggering at all. The best way is to just start the server with cluster enabled in race mode, and verify that the race doesn't happen. https://mattermost.atlassian.net/browse/MM-30319 ```release-notes NONE ``` --- app/config.go | 11 +++++++---- app/server.go | 9 ++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/app/config.go b/app/config.go index 599a32039d..7861516537 100644 --- a/app/config.go +++ b/app/config.go @@ -164,7 +164,7 @@ func (s *Server) ensurePostActionCookieSecret() error { // ensureAsymmetricSigningKey ensures that an asymmetric signing key exists and future calls to // AsymmetricSigningKey will always return a valid signing key. func (s *Server) ensureAsymmetricSigningKey() error { - if s.asymmetricSigningKey != nil { + if s.AsymmetricSigningKey() != nil { return nil } @@ -227,14 +227,14 @@ func (s *Server) ensureAsymmetricSigningKey() error { default: return fmt.Errorf("unknown curve: " + key.ECDSAKey.Curve) } - s.asymmetricSigningKey = &ecdsa.PrivateKey{ + s.asymmetricSigningKey.Store(&ecdsa.PrivateKey{ PublicKey: ecdsa.PublicKey{ Curve: curve, X: key.ECDSAKey.X, Y: key.ECDSAKey.Y, }, D: key.ECDSAKey.D, - } + }) s.regenerateClientConfig() return nil } @@ -279,7 +279,10 @@ func (s *Server) ensureFirstServerRunTimestamp() error { // AsymmetricSigningKey will return a private key that can be used for asymmetric signing. func (s *Server) AsymmetricSigningKey() *ecdsa.PrivateKey { - return s.asymmetricSigningKey + if key := s.asymmetricSigningKey.Load(); key != nil { + return key.(*ecdsa.PrivateKey) + } + return nil } func (a *App) AsymmetricSigningKey() *ecdsa.PrivateKey { diff --git a/app/server.go b/app/server.go index fb3fc8434d..71850c34ae 100644 --- a/app/server.go +++ b/app/server.go @@ -5,7 +5,6 @@ package app import ( "context" - "crypto/ecdsa" "crypto/tls" "fmt" "hash/maphash" @@ -129,7 +128,6 @@ type Server struct { searchLicenseListenerId string loggerLicenseListenerId string configStore *config.Store - asymmetricSigningKey *ecdsa.PrivateKey postActionCookieSecret []byte advancedLogListenerCleanup func() @@ -137,9 +135,10 @@ type Server struct { pluginCommands []*PluginCommand pluginCommandsLock sync.RWMutex - clientConfig atomic.Value - clientConfigHash atomic.Value - limitedClientConfig atomic.Value + asymmetricSigningKey atomic.Value + clientConfig atomic.Value + clientConfigHash atomic.Value + limitedClientConfig atomic.Value telemetryService *telemetry.TelemetryService