From f637481c67241151dc6d6fe2b12852e2ad8d70c2 Mon Sep 17 00:00:00 2001 From: Wang Yan Date: Mon, 15 Nov 2021 14:57:22 +0800 Subject: [PATCH] fix go check issues 1, Fix GoSec G404: Use of weak random number generator (math/rand instead of crypto/rand) 2, Fix Static check: ST1019: package "github.com/sirupsen/logrus" is being imported more than once Signed-off-by: Wang Yan --- contrib/token-server/main.go | 12 ++++++++++-- registry/handlers/app.go | 16 +++++++++++----- registry/registry.go | 35 +++++++++++++++++------------------ 3 files changed, 38 insertions(+), 25 deletions(-) diff --git a/contrib/token-server/main.go b/contrib/token-server/main.go index e722cd8e5..4566a8de2 100644 --- a/contrib/token-server/main.go +++ b/contrib/token-server/main.go @@ -2,9 +2,10 @@ package main import ( "context" + "crypto/rand" "encoding/json" "flag" - "math/rand" + "math/big" "net/http" "strconv" "strings" @@ -141,8 +142,15 @@ const refreshTokenLength = 15 func newRefreshToken() string { s := make([]rune, refreshTokenLength) + max := int64(len(refreshCharacters)) for i := range s { - s[i] = refreshCharacters[rand.Intn(len(refreshCharacters))] + randInt, err := rand.Int(rand.Reader, big.NewInt(max)) + // let '0' serves the failure case + if err != nil { + logrus.Infof("Error on making refersh token: %v", err) + randInt = big.NewInt(0) + } + s[i] = refreshCharacters[randInt.Int64()] } return string(s) } diff --git a/registry/handlers/app.go b/registry/handlers/app.go index 212c79a70..0514ed22b 100644 --- a/registry/handlers/app.go +++ b/registry/handlers/app.go @@ -2,10 +2,11 @@ package handlers import ( "context" - cryptorand "crypto/rand" + "crypto/rand" "expvar" "fmt" - "math/rand" + "math" + "math/big" "net" "net/http" "net/url" @@ -612,7 +613,7 @@ func (app *App) configureLogHook(configuration *configuration.Configuration) { func (app *App) configureSecret(configuration *configuration.Configuration) { if configuration.HTTP.Secret == "" { var secretBytes [randomSecretSize]byte - if _, err := cryptorand.Read(secretBytes[:]); err != nil { + if _, err := rand.Read(secretBytes[:]); err != nil { panic(fmt.Sprintf("could not generate random bytes for HTTP secret: %v", err)) } configuration.HTTP.Secret = string(secretBytes[:]) @@ -1062,8 +1063,13 @@ func startUploadPurger(ctx context.Context, storageDriver storagedriver.StorageD } go func() { - rand.Seed(time.Now().Unix()) - jitter := time.Duration(rand.Int()%60) * time.Minute + randInt, err := rand.Int(rand.Reader, new(big.Int).SetInt64(math.MaxInt64)) + if err != nil { + log.Infof("Failed to generate random jitter: %v", err) + // sleep 30min for failure case + randInt = big.NewInt(30) + } + jitter := time.Duration(randInt.Int64()%60) * time.Minute log.Infof("Starting upload purge in %s", jitter) time.Sleep(jitter) diff --git a/registry/registry.go b/registry/registry.go index 7d12f760a..229d13396 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -20,7 +20,6 @@ import ( "github.com/docker/go-metrics" gorhandlers "github.com/gorilla/handlers" "github.com/sirupsen/logrus" - log "github.com/sirupsen/logrus" "github.com/spf13/cobra" "github.com/yvasiyarov/gorelic" "golang.org/x/crypto/acme" @@ -111,16 +110,16 @@ var ServeCmd = &cobra.Command{ if config.HTTP.Debug.Addr != "" { go func(addr string) { - log.Infof("debug server listening %v", addr) + logrus.Infof("debug server listening %v", addr) if err := http.ListenAndServe(addr, nil); err != nil { - log.Fatalf("error listening on debug interface: %v", err) + logrus.Fatalf("error listening on debug interface: %v", err) } }(config.HTTP.Debug.Addr) } registry, err := NewRegistry(ctx, config) if err != nil { - log.Fatalln(err) + logrus.Fatalln(err) } if config.HTTP.Debug.Prometheus.Enabled { @@ -128,12 +127,12 @@ var ServeCmd = &cobra.Command{ if path == "" { path = "/metrics" } - log.Info("providing prometheus metrics on ", path) + logrus.Info("providing prometheus metrics on ", path) http.Handle(path, metrics.Handler()) } if err = registry.ListenAndServe(); err != nil { - log.Fatalln(err) + logrus.Fatalln(err) } }, } @@ -344,7 +343,7 @@ func configureReporting(app *handlers.App) http.Handler { // configureLogging prepares the context with a logger using the // configuration. func configureLogging(ctx context.Context, config *configuration.Configuration) (context.Context, error) { - log.SetLevel(logLevel(config.Log.Level)) + logrus.SetLevel(logLevel(config.Log.Level)) formatter := config.Log.Formatter if formatter == "" { @@ -353,16 +352,16 @@ func configureLogging(ctx context.Context, config *configuration.Configuration) switch formatter { case "json": - log.SetFormatter(&log.JSONFormatter{ + logrus.SetFormatter(&logrus.JSONFormatter{ TimestampFormat: time.RFC3339Nano, DisableHTMLEscape: true, }) case "text": - log.SetFormatter(&log.TextFormatter{ + logrus.SetFormatter(&logrus.TextFormatter{ TimestampFormat: time.RFC3339Nano, }) case "logstash": - log.SetFormatter(&logstash.LogstashFormatter{ + logrus.SetFormatter(&logstash.LogstashFormatter{ Formatter: &logrus.JSONFormatter{TimestampFormat: time.RFC3339Nano}, }) default: @@ -373,7 +372,7 @@ func configureLogging(ctx context.Context, config *configuration.Configuration) } if config.Log.Formatter != "" { - log.Debugf("using %q logging formatter", config.Log.Formatter) + logrus.Debugf("using %q logging formatter", config.Log.Formatter) } if len(config.Log.Fields) > 0 { @@ -391,11 +390,11 @@ func configureLogging(ctx context.Context, config *configuration.Configuration) return ctx, nil } -func logLevel(level configuration.Loglevel) log.Level { - l, err := log.ParseLevel(string(level)) +func logLevel(level configuration.Loglevel) logrus.Level { + l, err := logrus.ParseLevel(string(level)) if err != nil { - l = log.InfoLevel - log.Warnf("error parsing level %q: %v, using %q ", level, err, l) + l = logrus.InfoLevel + logrus.Warnf("error parsing level %q: %v, using %q ", level, err, l) } return l @@ -421,10 +420,10 @@ func configureBugsnag(config *configuration.Configuration) { // configure logrus bugsnag hook hook, err := logrus_bugsnag.NewBugsnagHook() if err != nil { - log.Fatalln(err) + logrus.Fatalln(err) } - log.AddHook(hook) + logrus.AddHook(hook) } // panicHandler add an HTTP handler to web app. The handler recover the happening @@ -434,7 +433,7 @@ func panicHandler(handler http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { defer func() { if err := recover(); err != nil { - log.Panic(fmt.Sprintf("%v", err)) + logrus.Panic(fmt.Sprintf("%v", err)) } }() handler.ServeHTTP(w, r)