From e3b1d33b954aee6f32590b739f13ea929634c233 Mon Sep 17 00:00:00 2001 From: Tim Kelsey Date: Fri, 5 Aug 2016 14:27:21 +0100 Subject: [PATCH] Configuration This re-works the way that CLI options are passed through to the analyzer so that they can act as overrides for config options. If not given on the CLI, options will come from a config file. If no file is used then a default value is chosen. Two lists are also populated with tests to include or exclude. These lists are not used for now but will eventually replace the way we select test to run in a future patch to follow. --- core/analyzer.go | 16 ++------ main.go | 61 +++++++++++++++++++++++++++-- rules/bind_test.go | 6 ++- rules/errors_test.go | 9 +++-- rules/fileperms_test.go | 6 ++- rules/hardcoded_credentials_test.go | 3 +- rules/httpoxy_test.go | 3 +- rules/nosec_test.go | 25 +++++++++++- rules/rand_test.go | 6 ++- rules/rsa_test.go | 3 +- rules/sql_test.go | 18 ++++++--- rules/subproc_test.go | 12 ++++-- rules/tempfiles_test.go | 3 +- rules/templates_test.go | 12 ++++-- rules/tls_test.go | 12 ++++-- rules/unsafe_test.go | 3 +- rules/weakcrypto_test.go | 9 +++-- 17 files changed, 154 insertions(+), 53 deletions(-) diff --git a/core/analyzer.go b/core/analyzer.go index 89e6852..57f4cc2 100644 --- a/core/analyzer.go +++ b/core/analyzer.go @@ -15,13 +15,11 @@ package core import ( - "encoding/json" "go/ast" "go/importer" "go/parser" "go/token" "go/types" - "io/ioutil" "log" "os" "reflect" @@ -59,27 +57,19 @@ type Analyzer struct { Stats Metrics `json:"metrics"` } -func NewAnalyzer(ignoreNosec bool, conf *string, logger *log.Logger) Analyzer { +func NewAnalyzer(conf map[string]interface{}, logger *log.Logger) Analyzer { if logger == nil { logger = log.New(os.Stdout, "[gas]", 0) } a := Analyzer{ - ignoreNosec: ignoreNosec, + ignoreNosec: conf["ignoreNosec"].(bool), ruleset: make(RuleSet), Issues: make([]Issue, 0), context: Context{token.NewFileSet(), nil, nil, nil, nil, nil}, logger: logger, } - if conf != nil && *conf != "" { // if we have a config - if data, err := ioutil.ReadFile(*conf); err == nil { - if err := json.Unmarshal(data, &(a.context.Config)); err != nil { - logger.Fatal("Could not parse JSON config: ", *conf, ": ", err) - } - } else { - logger.Fatal("Could not read config file: ", *conf) - } - } + // TODO(tkelsey): use the inc/exc lists return a } diff --git a/main.go b/main.go index 97fd4c2..4b2141a 100644 --- a/main.go +++ b/main.go @@ -15,8 +15,10 @@ package main import ( + "encoding/json" "flag" "fmt" + "io/ioutil" "log" "os" "path/filepath" @@ -57,6 +59,51 @@ USAGE: ` +var logger *log.Logger + +func extendConfList(conf map[string]interface{}, name string, input []string) { + if val, ok := conf[name]; ok { + if data, ok := val.(*[]string); ok { + conf[name] = append(*data, input...) + } else { + logger.Fatal("Config item must be a string list: ", name) + } + } else { + conf[name] = []string{} + } +} + +func buildConfig(incRules string, excRules string) map[string]interface{} { + config := make(map[string]interface{}) + if flagConfig != nil && *flagConfig != "" { // parse config if we have one + if data, err := ioutil.ReadFile(*flagConfig); err == nil { + if err := json.Unmarshal(data, &(config)); err != nil { + logger.Fatal("Could not parse JSON config: ", *flagConfig, ": ", err) + } + } else { + logger.Fatal("Could not read config file: ", *flagConfig) + } + } + + // add in CLI include and exclude data + extendConfList(config, "include", strings.Split(incRules, ",")) + extendConfList(config, "exclude", strings.Split(excRules, ",")) + + // override ignoreNosec if given on CLI + if flagIgnoreNoSec != nil { + config["ignoreNosec"] = *flagIgnoreNoSec + } else { + val, ok := config["ignoreNosec"] + if !ok { + config["ignoreNosec"] = false + } else if _, ok := val.(bool); !ok { + logger.Fatal("Config value must be a bool: 'ignoreNosec'") + } + } + + return config +} + func usage() { fmt.Fprintln(os.Stderr, usageText) fmt.Fprint(os.Stderr, "OPTIONS:\n\n") @@ -70,12 +117,18 @@ func main() { // Exclude files var excluded filelist = []string{"*_test.go"} - flag.Var(&excluded, "exclude", "File pattern to exclude from scan") + flag.Var(&excluded, "skip", "File pattern to exclude from scan") // Rule configuration rules := newRulelist() flag.Var(&rules, "rule", "GAS rules enabled when performing a scan") + incRules := "" + flag.StringVar(&incRules, "include", "", "comma sperated list of rules to include") + + excRules := "" + flag.StringVar(&excRules, "exclude", "", "comma sperated list of rules to exclude") + // Custom commands / utilities to run instead of default analyzer tools := newUtils() flag.Var(tools, "tool", "GAS utilities to assist with rule development") @@ -84,7 +137,7 @@ func main() { flag.Parse() // Setup logging - logger := log.New(os.Stderr, "[gas]", log.LstdFlags) + logger = log.New(os.Stderr, "[gas]", log.LstdFlags) // Ensure at least one file was specified if flag.NArg() == 0 { @@ -101,7 +154,9 @@ func main() { } // Setup analyzer - analyzer := gas.NewAnalyzer(*flagIgnoreNoSec, flagConfig, logger) + config := buildConfig(incRules, excRules) + + analyzer := gas.NewAnalyzer(config, logger) if !rules.overwritten { rules.useDefaults() } diff --git a/rules/bind_test.go b/rules/bind_test.go index 512e08f..41787e4 100644 --- a/rules/bind_test.go +++ b/rules/bind_test.go @@ -21,7 +21,8 @@ import ( ) func TestBind0000(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewBindsToAllNetworkInterfaces()) issues := gasTestRunner(` @@ -42,7 +43,8 @@ func TestBind0000(t *testing.T) { } func TestBindEmptyHost(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewBindsToAllNetworkInterfaces()) issues := gasTestRunner(` diff --git a/rules/errors_test.go b/rules/errors_test.go index 4689ed4..c644ad0 100644 --- a/rules/errors_test.go +++ b/rules/errors_test.go @@ -21,7 +21,8 @@ import ( ) func TestErrorsMulti(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewNoErrorCheck()) issues := gasTestRunner( @@ -43,7 +44,8 @@ func TestErrorsMulti(t *testing.T) { } func TestErrorsSingle(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewNoErrorCheck()) issues := gasTestRunner( @@ -65,7 +67,8 @@ func TestErrorsSingle(t *testing.T) { } func TestErrorsGood(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewNoErrorCheck()) issues := gasTestRunner( diff --git a/rules/fileperms_test.go b/rules/fileperms_test.go index 9a5eaa5..601f570 100644 --- a/rules/fileperms_test.go +++ b/rules/fileperms_test.go @@ -21,7 +21,8 @@ import ( ) func TestChmod(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewChmodPerms()) issues := gasTestRunner(` @@ -36,7 +37,8 @@ func TestChmod(t *testing.T) { } func TestMkdir(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewMkdirPerms()) issues := gasTestRunner(` diff --git a/rules/hardcoded_credentials_test.go b/rules/hardcoded_credentials_test.go index bbcf2be..1d512a7 100644 --- a/rules/hardcoded_credentials_test.go +++ b/rules/hardcoded_credentials_test.go @@ -21,7 +21,8 @@ import ( ) func TestHardcoded(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewHardcodedCredentials()) issues := gasTestRunner( diff --git a/rules/httpoxy_test.go b/rules/httpoxy_test.go index 400f7b7..cdaefe8 100644 --- a/rules/httpoxy_test.go +++ b/rules/httpoxy_test.go @@ -21,7 +21,8 @@ import ( ) func TestHttpoxy(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewBlacklistImports()) issues := gasTestRunner(` diff --git a/rules/nosec_test.go b/rules/nosec_test.go index b7bbc10..d0f7103 100644 --- a/rules/nosec_test.go +++ b/rules/nosec_test.go @@ -21,7 +21,8 @@ import ( ) func TestNosec(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewSubproc()) issues := gasTestRunner( @@ -39,7 +40,8 @@ func TestNosec(t *testing.T) { } func TestNosecBlock(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewSubproc()) issues := gasTestRunner( @@ -58,3 +60,22 @@ func TestNosecBlock(t *testing.T) { checkTestResults(t, issues, 0, "None") } + +func TestNosecIgnore(t *testing.T) { + config := map[string]interface{}{"ignoreNosec": true} + analyzer := gas.NewAnalyzer(config, nil) + analyzer.AddRule(NewSubproc()) + + issues := gasTestRunner( + `package main + + import ( + "fmt" + ) + + func main() { + cmd := exec.Command("sh", "-c", config.Command) // #nosec + }`, analyzer) + + checkTestResults(t, issues, 1, "Subprocess launching with variable.") +} diff --git a/rules/rand_test.go b/rules/rand_test.go index faab035..670b22c 100644 --- a/rules/rand_test.go +++ b/rules/rand_test.go @@ -21,7 +21,8 @@ import ( ) func TestRandOk(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewWeakRandCheck()) issues := gasTestRunner( @@ -38,7 +39,8 @@ func TestRandOk(t *testing.T) { } func TestRandBad(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewWeakRandCheck()) issues := gasTestRunner( diff --git a/rules/rsa_test.go b/rules/rsa_test.go index 9ec2a51..7988562 100644 --- a/rules/rsa_test.go +++ b/rules/rsa_test.go @@ -21,7 +21,8 @@ import ( ) func TestRSAKeys(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewWeakKeyStrength()) issues := gasTestRunner( diff --git a/rules/sql_test.go b/rules/sql_test.go index df8031c..2590230 100644 --- a/rules/sql_test.go +++ b/rules/sql_test.go @@ -21,7 +21,8 @@ import ( ) func TestSQLInjectionViaConcatenation(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewSqlStrConcat()) source := ` @@ -48,7 +49,8 @@ func TestSQLInjectionViaConcatenation(t *testing.T) { } func TestSQLInjectionViaIntepolation(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewSqlStrFormat()) source := ` @@ -77,7 +79,8 @@ func TestSQLInjectionViaIntepolation(t *testing.T) { } func TestSQLInjectionFalsePositiveA(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewSqlStrConcat()) analyzer.AddRule(NewSqlStrFormat()) @@ -112,7 +115,8 @@ func TestSQLInjectionFalsePositiveA(t *testing.T) { } func TestSQLInjectionFalsePositiveB(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewSqlStrConcat()) analyzer.AddRule(NewSqlStrFormat()) @@ -147,7 +151,8 @@ func TestSQLInjectionFalsePositiveB(t *testing.T) { } func TestSQLInjectionFalsePositiveC(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewSqlStrConcat()) analyzer.AddRule(NewSqlStrFormat()) @@ -182,7 +187,8 @@ func TestSQLInjectionFalsePositiveC(t *testing.T) { } func TestSQLInjectionFalsePositiveD(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewSqlStrConcat()) analyzer.AddRule(NewSqlStrFormat()) diff --git a/rules/subproc_test.go b/rules/subproc_test.go index d71ef7b..d8199d4 100644 --- a/rules/subproc_test.go +++ b/rules/subproc_test.go @@ -21,7 +21,8 @@ import ( ) func TestSubprocess(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewSubproc()) issues := gasTestRunner(` @@ -48,7 +49,8 @@ func TestSubprocess(t *testing.T) { } func TestSubprocessVar(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewSubproc()) issues := gasTestRunner(` @@ -75,7 +77,8 @@ func TestSubprocessVar(t *testing.T) { } func TestSubprocessPath(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewSubproc()) issues := gasTestRunner(` @@ -101,7 +104,8 @@ func TestSubprocessPath(t *testing.T) { } func TestSubprocessSyscall(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewSubproc()) issues := gasTestRunner(` diff --git a/rules/tempfiles_test.go b/rules/tempfiles_test.go index 1a5fc68..d3c019c 100644 --- a/rules/tempfiles_test.go +++ b/rules/tempfiles_test.go @@ -21,7 +21,8 @@ import ( ) func TestTempfiles(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewBadTempFile()) source := ` diff --git a/rules/templates_test.go b/rules/templates_test.go index e1f84ed..0185ef7 100644 --- a/rules/templates_test.go +++ b/rules/templates_test.go @@ -21,7 +21,8 @@ import ( ) func TestTemplateCheckSafe(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewTemplateCheck()) source := ` @@ -48,7 +49,8 @@ func TestTemplateCheckSafe(t *testing.T) { } func TestTemplateCheckBadHTML(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewTemplateCheck()) source := ` @@ -76,7 +78,8 @@ func TestTemplateCheckBadHTML(t *testing.T) { } func TestTemplateCheckBadJS(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewTemplateCheck()) source := ` @@ -104,7 +107,8 @@ func TestTemplateCheckBadJS(t *testing.T) { } func TestTemplateCheckBadURL(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewTemplateCheck()) source := ` diff --git a/rules/tls_test.go b/rules/tls_test.go index 6ead5d2..2f9c797 100644 --- a/rules/tls_test.go +++ b/rules/tls_test.go @@ -21,7 +21,8 @@ import ( ) func TestInsecureSkipVerify(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewModernTlsCheck()) issues := gasTestRunner(` @@ -49,7 +50,8 @@ func TestInsecureSkipVerify(t *testing.T) { } func TestInsecureMinVersion(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewModernTlsCheck()) issues := gasTestRunner(` @@ -77,7 +79,8 @@ func TestInsecureMinVersion(t *testing.T) { } func TestInsecureMaxVersion(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewModernTlsCheck()) issues := gasTestRunner(` @@ -105,7 +108,8 @@ func TestInsecureMaxVersion(t *testing.T) { } func TestInsecureCipherSuite(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewModernTlsCheck()) issues := gasTestRunner(` diff --git a/rules/unsafe_test.go b/rules/unsafe_test.go index 2a35649..1dcbb3e 100644 --- a/rules/unsafe_test.go +++ b/rules/unsafe_test.go @@ -21,7 +21,8 @@ import ( ) func TestUnsafe(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewUsingUnsafe()) issues := gasTestRunner(` diff --git a/rules/weakcrypto_test.go b/rules/weakcrypto_test.go index 390f0ec..35070c6 100644 --- a/rules/weakcrypto_test.go +++ b/rules/weakcrypto_test.go @@ -21,7 +21,8 @@ import ( ) func TestMD5(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewBlacklistImports()) analyzer.AddRule(NewUsesWeakCryptography()) @@ -42,7 +43,8 @@ func TestMD5(t *testing.T) { } func TestDES(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewBlacklistImports()) analyzer.AddRule(NewUsesWeakCryptography()) @@ -81,7 +83,8 @@ func TestDES(t *testing.T) { } func TestRC4(t *testing.T) { - analyzer := gas.NewAnalyzer(false, nil, nil) + config := map[string]interface{}{"ignoreNosec": false} + analyzer := gas.NewAnalyzer(config, nil) analyzer.AddRule(NewBlacklistImports()) analyzer.AddRule(NewUsesWeakCryptography())