Skip to content

Commit

Permalink
Fix some possible race conditions
Browse files Browse the repository at this point in the history
When launching multiple reva instance for a single process we need to
make sure that the sharedconfig global variable is initialized exactly
once. Otherwise we might run into race conditions. Similarly make sure
to initialize the user cache for the auth interceptor exactly once.
  • Loading branch information
rhafer committed Oct 19, 2022
1 parent df0a189 commit 4371fa6
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 24 deletions.
6 changes: 6 additions & 0 deletions changelog/unreleased/fix-racecond.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Bugfix: Fix possible race conditions

We fixed two potential race condition when initializing the shared config
structure and when setting up caches for the http authentication interceptors.

https://github.com/cs3org/reva/pull/3xxx
11 changes: 9 additions & 2 deletions internal/http/interceptors/auth/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"fmt"
"net/http"
"strings"
"sync"
"time"

"github.com/bluele/gcache"
Expand Down Expand Up @@ -55,7 +56,10 @@ import (
// name is the Tracer name used to identify this instrumentation library.
const tracerName = "auth"

var userGroupsCache gcache.Cache
var (
cacheOnce sync.Once
userGroupsCache gcache.Cache
)

type config struct {
Priority int `mapstructure:"priority"`
Expand Down Expand Up @@ -116,7 +120,10 @@ func New(m map[string]interface{}, unprotected []string, tp trace.TracerProvider
if conf.UserGroupsCacheSize == 0 {
conf.UserGroupsCacheSize = 5000
}
userGroupsCache = gcache.New(conf.UserGroupsCacheSize).LFU().Build()

cacheOnce.Do(func() {
userGroupsCache = gcache.New(conf.UserGroupsCacheSize).LFU().Build()
})

credChain := map[string]auth.CredentialStrategy{}
for i, key := range conf.CredentialChain {
Expand Down
51 changes: 29 additions & 22 deletions pkg/sharedconf/sharedconf.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,15 @@ package sharedconf
import (
"fmt"
"os"
"sync"

"github.com/mitchellh/mapstructure"
)

var sharedConf = &conf{}
var (
sharedConf = &conf{}
sharedConfOnce sync.Once
)

// ClientOptions represent additional options (e.g. tls settings) for the grpc clients
type ClientOptions struct {
Expand All @@ -43,32 +47,35 @@ type conf struct {

// Decode decodes the configuration.
func Decode(v interface{}) error {
if err := mapstructure.Decode(v, sharedConf); err != nil {
return err
}
var err error

// add some defaults
if sharedConf.GatewaySVC == "" {
sharedConf.GatewaySVC = "0.0.0.0:19000"
}
sharedConfOnce.Do(func() {
if err = mapstructure.Decode(v, sharedConf); err != nil {
return
}
// add some defaults
if sharedConf.GatewaySVC == "" {
sharedConf.GatewaySVC = "0.0.0.0:19000"
}

// this is the default address we use for the data gateway HTTP service
if sharedConf.DataGateway == "" {
host, err := os.Hostname()
if err != nil || host == "" {
sharedConf.DataGateway = "http://0.0.0.0:19001/datagateway"
} else {
sharedConf.DataGateway = fmt.Sprintf("http://%s:19001/datagateway", host)
// this is the default address we use for the data gateway HTTP service
if sharedConf.DataGateway == "" {
host, err := os.Hostname()
if err != nil || host == "" {
sharedConf.DataGateway = "http://0.0.0.0:19001/datagateway"
} else {
sharedConf.DataGateway = fmt.Sprintf("http://%s:19001/datagateway", host)
}
}
}

// TODO(labkode): would be cool to autogenerate one secret and print
// it on init time.
if sharedConf.JWTSecret == "" {
sharedConf.JWTSecret = "changemeplease"
}
// TODO(labkode): would be cool to autogenerate one secret and print
// it on init time.
if sharedConf.JWTSecret == "" {
sharedConf.JWTSecret = "changemeplease"
}
})

return nil
return err
}

// GetJWTSecret returns the package level configured jwt secret if not overwritten.
Expand Down

0 comments on commit 4371fa6

Please sign in to comment.