From 6a1590bc97c0a513a38abdbff46825408cdf22ac Mon Sep 17 00:00:00 2001 From: Javier Marcos <1271349+javuto@users.noreply.github.com> Date: Fri, 31 Jan 2025 20:00:35 +0100 Subject: [PATCH 1/3] Prevent nil pointer when loading any JSON configuration --- admin/jwt.go | 6 +++++- admin/main.go | 2 +- admin/oauth.go | 5 +++++ admin/oidc.go | 6 +++++- admin/saml.go | 3 +++ api/jwt.go | 9 +++++++-- backend/backend.go | 3 +++ cache/cache.go | 7 +++++-- carves/s3.go | 3 +++ cli/api.go | 2 +- logging/elastic.go | 3 +++ logging/graylog.go | 7 +++++-- logging/kinesis.go | 3 +++ logging/logstash.go | 3 +++ logging/s3.go | 4 ++++ logging/splunk.go | 4 ++++ metrics/metrics.go | 3 +++ 17 files changed, 63 insertions(+), 10 deletions(-) diff --git a/admin/jwt.go b/admin/jwt.go index fddbb800..986c4e0d 100644 --- a/admin/jwt.go +++ b/admin/jwt.go @@ -3,12 +3,13 @@ package main import ( "crypto/rsa" "crypto/tls" + "fmt" "github.com/golang-jwt/jwt/v4" "github.com/jmpsec/osctrl/settings" "github.com/jmpsec/osctrl/types" - "github.com/spf13/viper" "github.com/rs/zerolog/log" + "github.com/spf13/viper" ) // Function to load the configuration file @@ -22,6 +23,9 @@ func loadJWTConfiguration(file string) (types.JSONConfigurationJWT, error) { } // JWT values jwtRaw := viper.Sub(settings.AuthJWT) + if jwtRaw == nil { + return cfg, fmt.Errorf("JSON key %s not found in file %s", settings.AuthJWT, file) + } if err := jwtRaw.Unmarshal(&cfg); err != nil { return cfg, err } diff --git a/admin/main.go b/admin/main.go index be3f78d5..6b23b019 100644 --- a/admin/main.go +++ b/admin/main.go @@ -206,7 +206,7 @@ func loadConfiguration(file, service string) (types.JSONConfigurationAdmin, erro // Admin values adminRaw := viper.Sub(service) if adminRaw == nil { - return cfg, fmt.Errorf("JSON key %s not found in %s", service, file) + return cfg, fmt.Errorf("JSON key %s not found in file %s", service, file) } if err := adminRaw.Unmarshal(&cfg); err != nil { return cfg, err diff --git a/admin/oauth.go b/admin/oauth.go index 7902b828..716d0fb7 100644 --- a/admin/oauth.go +++ b/admin/oauth.go @@ -1,6 +1,8 @@ package main import ( + "fmt" + "github.com/jmpsec/osctrl/settings" "github.com/rs/zerolog/log" "github.com/spf13/viper" @@ -26,6 +28,9 @@ func loadOAuth(file string) (JSONConfigurationOAuth, error) { } // OAuth values oauthRaw := viper.Sub(settings.AuthOAuth) + if oauthRaw == nil { + return cfg, fmt.Errorf("JSON key %s not found in %s", settings.AuthOAuth, file) + } if err := oauthRaw.Unmarshal(&cfg); err != nil { return cfg, err } diff --git a/admin/oidc.go b/admin/oidc.go index c02cd7d4..a778e160 100644 --- a/admin/oidc.go +++ b/admin/oidc.go @@ -1,10 +1,11 @@ package main import ( + "fmt" "github.com/jmpsec/osctrl/settings" - "github.com/spf13/viper" "github.com/rs/zerolog/log" + "github.com/spf13/viper" ) // JSONConfigurationOIDC to keep all OIDC details for auth @@ -30,6 +31,9 @@ func loadOIDC(file string) (JSONConfigurationOIDC, error) { } // OAuth values oauthRaw := viper.Sub(settings.AuthOIDC) + if oauthRaw == nil { + return cfg, fmt.Errorf("JSON key %s not found in %s", settings.AuthOIDC, file) + } if err := oauthRaw.Unmarshal(&cfg); err != nil { return cfg, err } diff --git a/admin/saml.go b/admin/saml.go index 2c6cda83..9ce6aa77 100644 --- a/admin/saml.go +++ b/admin/saml.go @@ -45,6 +45,9 @@ func loadSAML(file string) (JSONConfigurationSAML, error) { } // SAML values samlRaw := viper.Sub(settings.AuthSAML) + if samlRaw == nil { + return cfg, fmt.Errorf("JSON key %s not found in %s", settings.AuthSAML, file) + } if err := samlRaw.Unmarshal(&cfg); err != nil { return cfg, err } diff --git a/api/jwt.go b/api/jwt.go index 38223482..4e8776ff 100644 --- a/api/jwt.go +++ b/api/jwt.go @@ -1,6 +1,8 @@ package main import ( + "fmt" + "github.com/jmpsec/osctrl/settings" "github.com/jmpsec/osctrl/types" "github.com/rs/zerolog/log" @@ -17,8 +19,11 @@ func loadJWTConfiguration(file string) (types.JSONConfigurationJWT, error) { return cfg, err } // JWT values - headersRaw := viper.Sub(settings.AuthJWT) - if err := headersRaw.Unmarshal(&cfg); err != nil { + jwtRaw := viper.Sub(settings.AuthJWT) + if jwtRaw == nil { + return cfg, fmt.Errorf("JSON key %s not found in %s", settings.AuthJWT, file) + } + if err := jwtRaw.Unmarshal(&cfg); err != nil { return cfg, err } // No errors! diff --git a/backend/backend.go b/backend/backend.go index f9fd9c51..c965c90f 100644 --- a/backend/backend.go +++ b/backend/backend.go @@ -48,6 +48,9 @@ func LoadConfiguration(file, key string) (JSONConfigurationDB, error) { } // Backend values dbRaw := viper.Sub(key) + if dbRaw == nil { + return config, fmt.Errorf("JSON key %s not found in %s", key, file) + } if err := dbRaw.Unmarshal(&config); err != nil { return config, err } diff --git a/cache/cache.go b/cache/cache.go index 0d7115c6..3aa03276 100644 --- a/cache/cache.go +++ b/cache/cache.go @@ -46,8 +46,11 @@ func LoadConfiguration(file, key string) (JSONConfigurationRedis, error) { return config, err } // Backend values - dbRaw := viper.Sub(key) - if err := dbRaw.Unmarshal(&config); err != nil { + redisRaw := viper.Sub(key) + if redisRaw == nil { + return config, fmt.Errorf("JSON key %s not found in %s", key, file) + } + if err := redisRaw.Unmarshal(&config); err != nil { return config, err } // No errors! diff --git a/carves/s3.go b/carves/s3.go index 222c6de4..f8540d04 100644 --- a/carves/s3.go +++ b/carves/s3.go @@ -85,6 +85,9 @@ func LoadS3(file string) (types.S3Configuration, error) { return _s3Cfg, err } cfgRaw := viper.Sub(settings.LoggingS3) + if cfgRaw == nil { + return _s3Cfg, fmt.Errorf("JSON key %s not found in %s", settings.LoggingS3, file) + } if err := cfgRaw.Unmarshal(&_s3Cfg); err != nil { return _s3Cfg, err } diff --git a/cli/api.go b/cli/api.go index c7838608..f97cfc4a 100644 --- a/cli/api.go +++ b/cli/api.go @@ -68,7 +68,7 @@ func loadAPIConfiguration(file string) (JSONConfigurationAPI, error) { // API values apiRaw := viper.Sub(projectName) if apiRaw == nil { - return config, fmt.Errorf("could not find key %s", projectName) + return config, fmt.Errorf("JSON key %s not found in %s", projectName, file) } if err := apiRaw.Unmarshal(&config); err != nil { return config, err diff --git a/logging/elastic.go b/logging/elastic.go index c548cab1..3a2848c8 100644 --- a/logging/elastic.go +++ b/logging/elastic.go @@ -73,6 +73,9 @@ func LoadElastic(file string) (ElasticConfiguration, error) { return _elasticCfg, err } cfgRaw := viper.Sub(settings.LoggingElastic) + if cfgRaw == nil { + return _elasticCfg, fmt.Errorf("JSON key %s not found in %s", settings.LoggingElastic, file) + } if err := cfgRaw.Unmarshal(&_elasticCfg); err != nil { return _elasticCfg, err } diff --git a/logging/graylog.go b/logging/graylog.go index a1acc61e..477a8c9a 100644 --- a/logging/graylog.go +++ b/logging/graylog.go @@ -2,6 +2,7 @@ package logging import ( "encoding/json" + "fmt" "strings" "time" @@ -32,8 +33,10 @@ func LoadGraylog(file string) (GraylogConfiguration, error) { return _graylogCfg, err } cfgRaw := viper.Sub(settings.LoggingGraylog) - err = cfgRaw.Unmarshal(&_graylogCfg) - if err != nil { + if cfgRaw == nil { + return _graylogCfg, fmt.Errorf("JSON key %s not found in %s", settings.LoggingGraylog, file) + } + if err := cfgRaw.Unmarshal(&_graylogCfg); err != nil { return _graylogCfg, err } // No errors! diff --git a/logging/kinesis.go b/logging/kinesis.go index 5de8dd11..f6277ad5 100644 --- a/logging/kinesis.go +++ b/logging/kinesis.go @@ -65,6 +65,9 @@ func LoadKinesis(file string) (KinesisConfiguration, error) { return _kinesisCfg, err } cfgRaw := viper.Sub(settings.LoggingSplunk) + if cfgRaw == nil { + return _kinesisCfg, fmt.Errorf("JSON key %s not found in %s", settings.LoggingSplunk, file) + } if err := cfgRaw.Unmarshal(&_kinesisCfg); err != nil { return _kinesisCfg, err } diff --git a/logging/logstash.go b/logging/logstash.go index f9b7ebe8..f22dd940 100644 --- a/logging/logstash.go +++ b/logging/logstash.go @@ -61,6 +61,9 @@ func LoadLogstash(file string) (LogstashConfiguration, error) { return _logstashCfg, err } cfgRaw := viper.Sub(settings.LoggingLogstash) + if cfgRaw == nil { + return _logstashCfg, fmt.Errorf("JSON key %s not found in %s", settings.LoggingLogstash, file) + } if err := cfgRaw.Unmarshal(&_logstashCfg); err != nil { return _logstashCfg, err } diff --git a/logging/s3.go b/logging/s3.go index a9b924aa..750b21df 100644 --- a/logging/s3.go +++ b/logging/s3.go @@ -3,6 +3,7 @@ package logging import ( "bytes" "context" + "fmt" "net/http" "strconv" "time" @@ -72,6 +73,9 @@ func LoadS3(file string) (types.S3Configuration, error) { return _s3Cfg, err } cfgRaw := viper.Sub(settings.LoggingS3) + if cfgRaw == nil { + return _s3Cfg, fmt.Errorf("JSON key %s not found in %s", settings.LoggingS3, file) + } if err := cfgRaw.Unmarshal(&_s3Cfg); err != nil { return _s3Cfg, err } diff --git a/logging/splunk.go b/logging/splunk.go index 5c6e8bb8..e811b219 100644 --- a/logging/splunk.go +++ b/logging/splunk.go @@ -2,6 +2,7 @@ package logging import ( "encoding/json" + "fmt" "strings" "time" @@ -54,6 +55,9 @@ func LoadSplunk(file string) (SlunkConfiguration, error) { return _splunkCfg, err } cfgRaw := viper.Sub(settings.LoggingSplunk) + if cfgRaw == nil { + return _splunkCfg, fmt.Errorf("JSON key %s not found in %s", settings.LoggingSplunk, file) + } if err := cfgRaw.Unmarshal(&_splunkCfg); err != nil { return _splunkCfg, err } diff --git a/metrics/metrics.go b/metrics/metrics.go index 0b33c028..c462e189 100644 --- a/metrics/metrics.go +++ b/metrics/metrics.go @@ -35,6 +35,9 @@ func LoadConfiguration() (Configuration, error) { return _metricsCfg, err } cfgRaw := viper.Sub(metricsName) + if cfgRaw == nil { + return _metricsCfg, fmt.Errorf("JSON key %s not found in %s", metricsName, metricsConfigFile) + } if err := cfgRaw.Unmarshal(&_metricsCfg); err != nil { return _metricsCfg, err } From f61e1a6b9411f88dfdacfa82d17a32f18af515bc Mon Sep 17 00:00:00 2001 From: Javier Marcos <1271349+javuto@users.noreply.github.com> Date: Fri, 31 Jan 2025 21:18:15 +0100 Subject: [PATCH 2/3] Specify DB logger from config or file --- admin/handlers/handlers.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/admin/handlers/handlers.go b/admin/handlers/handlers.go index 95e8d1fa..2530a65a 100644 --- a/admin/handlers/handlers.go +++ b/admin/handlers/handlers.go @@ -181,7 +181,7 @@ func WithDBLogger(dbfile string, config *backend.JSONConfigurationDB) HandlersOp } logger, err := logging.CreateLoggerDBConfig(*config) if err != nil { - log.Err(err).Msg("error creating DB logger") + log.Err(err).Msg("error creating DB logger (config)") logger.Enabled = false logger.Database = nil } @@ -190,7 +190,7 @@ func WithDBLogger(dbfile string, config *backend.JSONConfigurationDB) HandlersOp } logger, err := logging.CreateLoggerDBFile(dbfile) if err != nil { - log.Err(err).Msg("error creating DB logger") + log.Err(err).Msg("error creating DB logger (file)") logger.Enabled = false logger.Database = nil } From 82f74414e16ca79be3fc062cd7e50a67846d1850 Mon Sep 17 00:00:00 2001 From: Javier Marcos <1271349+javuto@users.noreply.github.com> Date: Fri, 31 Jan 2025 21:21:34 +0100 Subject: [PATCH 3/3] Avoid nil pointer in logger --- admin/handlers/handlers.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/admin/handlers/handlers.go b/admin/handlers/handlers.go index 2530a65a..15c7ce1f 100644 --- a/admin/handlers/handlers.go +++ b/admin/handlers/handlers.go @@ -182,8 +182,10 @@ func WithDBLogger(dbfile string, config *backend.JSONConfigurationDB) HandlersOp logger, err := logging.CreateLoggerDBConfig(*config) if err != nil { log.Err(err).Msg("error creating DB logger (config)") - logger.Enabled = false - logger.Database = nil + logger = &logging.LoggerDB{ + Enabled: false, + Database: nil, + } } h.DBLogger = logger return @@ -191,8 +193,10 @@ func WithDBLogger(dbfile string, config *backend.JSONConfigurationDB) HandlersOp logger, err := logging.CreateLoggerDBFile(dbfile) if err != nil { log.Err(err).Msg("error creating DB logger (file)") - logger.Enabled = false - logger.Database = nil + logger = &logging.LoggerDB{ + Enabled: false, + Database: nil, + } } h.DBLogger = logger }