Rule selection rules

This makes the following changes: - riles are identified by an ID - include / exclude list now work - rules are selected based on these lists - blacklist rules are broken out into methods - rule constructors now take the config map - config file can be used to select rules - CLI options embelish config selection options
holyspectral · Aug 11, 2016 · 713949f · 713949f
1 parent 235308f
commit 713949f
Show file tree

Hide file tree

Showing 31 changed files with 208 additions and 178 deletions.
diff --git a/main.go b/main.go
@@ -22,6 +22,7 @@ import (
 	"log"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 
 	gas "github.com/HewlettPackard/gas/core"
@@ -61,15 +62,20 @@ 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...)
+func extendConfList(conf map[string]interface{}, name string, inputStr string) {
+	if inputStr == "" {
+		conf[name] = []string{}
+	} else {
+		input := strings.Split(inputStr, ",")
+		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 {
-			logger.Fatal("Config item must be a string list: ", name)
+			conf[name] = input
 		}
-	} else {
-		conf[name] = []string{}
 	}
 }
 
@@ -86,8 +92,8 @@ func buildConfig(incRules string, excRules string) map[string]interface{} {
 	}
 
 	// add in CLI include and exclude data
-	extendConfList(config, "include", strings.Split(incRules, ","))
-	extendConfList(config, "exclude", strings.Split(excRules, ","))
+	extendConfList(config, "include", incRules)
+	extendConfList(config, "exclude", excRules)
 
 	// override ignoreNosec if given on CLI
 	if flagIgnoreNoSec != nil {
@@ -108,6 +114,20 @@ func usage() {
 	fmt.Fprintln(os.Stderr, usageText)
 	fmt.Fprint(os.Stderr, "OPTIONS:\n\n")
 	flag.PrintDefaults()
+	fmt.Fprint(os.Stderr, "\n\nRULES:\n\n")
+
+	// sorted rule list for eas of reading
+	rl := GetFullRuleList()
+	keys := make([]string, 0, len(rl))
+	for key := range rl {
+		keys = append(keys, key)
+	}
+	sort.Strings(keys)
+	for _, k := range keys {
+		v := rl[k]
+		fmt.Fprintf(os.Stderr, "\t%s: %s\n", k, v.description)
+	}
+	fmt.Fprint(os.Stderr, "\n")
 }
 
 func main() {
@@ -119,15 +139,11 @@ func main() {
 	var excluded filelist = []string{"*_test.go"}
 	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")
+	flag.StringVar(&incRules, "include", "", "comma sperated list of rules IDs to include, see rule list")
 
 	excRules := ""
-	flag.StringVar(&excRules, "exclude", "", "comma sperated list of rules to exclude")
+	flag.StringVar(&excRules, "exclude", "", "comma sperated list of rules IDs to exclude, see rule list")
 
 	// Custom commands / utilities to run instead of default analyzer
 	tools := newUtils()
@@ -155,12 +171,8 @@ func main() {
 
 	// Setup analyzer
 	config := buildConfig(incRules, excRules)
-
 	analyzer := gas.NewAnalyzer(config, logger)
-	if !rules.overwritten {
-		rules.useDefaults()
-	}
-	rules.apply(&analyzer)
+	AddRules(&analyzer, config)
 
 	// Traverse directory structure if './...'
 	if flag.NArg() == 1 && flag.Arg(0) == "./..." {

diff --git a/rulelist.go b/rulelist.go
@@ -17,85 +17,77 @@ package main
 import (
 	"fmt"
 	"go/ast"
-	"strings"
 
 	gas "github.com/HewlettPackard/gas/core"
 	"github.com/HewlettPackard/gas/rules"
 )
 
-type ruleMaker func() (gas.Rule, ast.Node)
-type ruleConfig struct {
-	enabled      bool
-	constructors []ruleMaker
+type RuleInfo struct {
+	description string
+	build       func(map[string]interface{}) (gas.Rule, ast.Node)
 }
 
-type rulelist struct {
-	rules       map[string]*ruleConfig
-	overwritten bool
-}
+// GetFullRuleList get the full list of all rules available to GAS
+func GetFullRuleList() map[string]RuleInfo {
+	return map[string]RuleInfo{
+		// misc
+		"G101": RuleInfo{"hardcoded credentials", rules.NewHardcodedCredentials},
+		"G102": RuleInfo{"bind to all interfaces", rules.NewBindsToAllNetworkInterfaces},
+		"G103": RuleInfo{"use of unsafe block", rules.NewUsingUnsafe},
+		"G104": RuleInfo{"errors not checked", rules.NewTemplateCheck},
 
-func newRulelist() rulelist {
-	var rs rulelist
-	rs.rules = make(map[string]*ruleConfig)
-	rs.overwritten = false
-	rs.register("sql", rules.NewSqlStrConcat, rules.NewSqlStrFormat)
-	rs.register("crypto", rules.NewUsesWeakCryptography)
-	rs.register("hardcoded", rules.NewHardcodedCredentials)
-	rs.register("perms", rules.NewMkdirPerms, rules.NewChmodPerms)
-	rs.register("tempfile", rules.NewBadTempFile)
-	rs.register("tls_good", rules.NewModernTlsCheck)
-	rs.register("tls_ok", rules.NewIntermediateTlsCheck)
-	rs.register("tls_old", rules.NewCompatTlsCheck)
-	rs.register("bind", rules.NewBindsToAllNetworkInterfaces)
-	rs.register("unsafe", rules.NewUsingUnsafe)
-	rs.register("rsa", rules.NewWeakKeyStrength)
-	rs.register("templates", rules.NewTemplateCheck)
-	rs.register("exec", rules.NewSubproc)
-	rs.register("errors", rules.NewNoErrorCheck)
-	rs.register("rand", rules.NewWeakRandCheck)
-	rs.register("blacklist_imports", rules.NewBlacklistImports)
-	return rs
-}
+		// injection
+		"G201": RuleInfo{"sql string format", rules.NewSqlStrFormat},
+		"G202": RuleInfo{"sql string concat", rules.NewSqlStrConcat},
+		"G203": RuleInfo{"unescaped templates", rules.NewTemplateCheck},
+		"G204": RuleInfo{"use of exec", rules.NewSubproc},
 
-func (r *rulelist) register(name string, cons ...ruleMaker) {
-	r.rules[name] = &ruleConfig{false, cons}
-}
+		// filesystem
+		"G301": RuleInfo{"poor mkdir permissions", rules.NewMkdirPerms},
+		"G302": RuleInfo{"poor chmod permisions", rules.NewChmodPerms},
+		"G303": RuleInfo{"predicatable tempfile", rules.NewBadTempFile},
 
-func (r *rulelist) useDefaults() {
-	for k := range r.rules {
-		r.rules[k].enabled = true
-	}
-}
+		// crypto
+		"G401": RuleInfo{"weak crypto", rules.NewUsesWeakCryptography},
+		"G402": RuleInfo{"bad TLS options", rules.NewIntermediateTlsCheck},
+		"G403": RuleInfo{"bad RSA key length", rules.NewWeakKeyStrength},
+		"G404": RuleInfo{"poor random source (rand)", rules.NewWeakRandCheck},
 
-func (r *rulelist) list() []string {
-	i := 0
-	keys := make([]string, len(r.rules))
-	for k := range r.rules {
-		keys[i] = k
-		i++
+		// blacklist
+		"G501": RuleInfo{"blacklist: crypto/md5", rules.NewBlacklist_crypto_md5},
+		"G502": RuleInfo{"blacklist: crypto/des", rules.NewBlacklist_crypto_des},
+		"G503": RuleInfo{"blacklist: crypto/rc4", rules.NewBlacklist_crypto_rc4},
+		"G504": RuleInfo{"blacklist: net/http/cgi", rules.NewBlacklist_net_http_cgi},
 	}
-	return keys
 }
 
-func (r *rulelist) apply(g *gas.Analyzer) {
-	for _, v := range r.rules {
-		if v.enabled {
-			for _, ctor := range v.constructors {
-				g.AddRule(ctor())
+func AddRules(analyzer *gas.Analyzer, conf map[string]interface{}) {
+	var all map[string]RuleInfo
+
+	inc := conf["include"].([]string)
+	exc := conf["exclude"].([]string)
+
+	fmt.Println(len(inc))
+
+	// add included rules
+	if len(inc) == 0 {
+		all = GetFullRuleList()
+	} else {
+		all = map[string]RuleInfo{}
+		tmp := GetFullRuleList()
+		for _, v := range inc {
+			if val, ok := tmp[v]; ok {
+				all[v] = val
 			}
 		}
 	}
-}
 
-func (r *rulelist) String() string {
-	return strings.Join(r.list(), ", ")
-}
+	// remove excluded rules
+	for _, v := range exc {
+		delete(all, v)
+	}
 
-func (r *rulelist) Set(opt string) error {
-	r.overwritten = true
-	if x, ok := r.rules[opt]; ok {
-		x.enabled = true
-		return nil
+	for _, v := range all {
+		analyzer.AddRule(v.build(conf))
 	}
-	return fmt.Errorf("Valid rules are: %s", r)
 }
diff --git a/rules/bind.go b/rules/bind.go
@@ -15,9 +15,10 @@
 package rules
 
 import (
-	gas "github.com/HewlettPackard/gas/core"
 	"go/ast"
 	"regexp"
+
+	gas "github.com/HewlettPackard/gas/core"
 )
 
 // Looks for net.Listen("0.0.0.0") or net.Listen(":8080")
@@ -38,7 +39,7 @@ func (r *BindsToAllNetworkInterfaces) Match(n ast.Node, c *gas.Context) (gi *gas
 	return
 }
 
-func NewBindsToAllNetworkInterfaces() (r gas.Rule, n ast.Node) {
+func NewBindsToAllNetworkInterfaces(conf map[string]interface{}) (r gas.Rule, n ast.Node) {
 	r = &BindsToAllNetworkInterfaces{
 		call:    regexp.MustCompile(`^net\.Listen$`),
 		pattern: regexp.MustCompile(`^(0.0.0.0|:).*$`),

diff --git a/rules/bind_test.go b/rules/bind_test.go
@@ -23,7 +23,7 @@ import (
 func TestBind0000(t *testing.T) {
 	config := map[string]interface{}{"ignoreNosec": false}
 	analyzer := gas.NewAnalyzer(config, nil)
-	analyzer.AddRule(NewBindsToAllNetworkInterfaces())
+	analyzer.AddRule(NewBindsToAllNetworkInterfaces(config))
 
 	issues := gasTestRunner(`
 		package main
@@ -45,7 +45,7 @@ func TestBind0000(t *testing.T) {
 func TestBindEmptyHost(t *testing.T) {
 	config := map[string]interface{}{"ignoreNosec": false}
 	analyzer := gas.NewAnalyzer(config, nil)
-	analyzer.AddRule(NewBindsToAllNetworkInterfaces())
+	analyzer.AddRule(NewBindsToAllNetworkInterfaces(config))
 
 	issues := gasTestRunner(`
 		package main

diff --git a/rules/blacklist.go b/rules/blacklist.go
@@ -20,47 +20,68 @@ import (
 	gas "github.com/HewlettPackard/gas/core"
 )
 
-type BlacklistImports struct {
-	BlacklistSet map[string]gas.MetaData
+type BlacklistImport struct {
+	gas.MetaData
+	Path string
 }
 
-func (r *BlacklistImports) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) {
+func (r *BlacklistImport) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) {
 	if node, ok := n.(*ast.ImportSpec); ok {
-		if data, ok := r.BlacklistSet[node.Path.Value]; ok {
-			return gas.NewIssue(c, n, data.What, data.Severity, data.Confidence), nil
+		if r.Path == node.Path.Value {
+			return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil
 		}
 	}
 	return nil, nil
 }
 
-func NewBlacklistImports() (r gas.Rule, n ast.Node) {
-	// TODO(tkelsey): make this configurable
-	// TODO(tkelsey): make it so each item can be selected/excluded individually
-	r = &BlacklistImports{
-		BlacklistSet: map[string]gas.MetaData{
-			`"crypto/md5"`: gas.MetaData{
-				Severity:   gas.High,
-				Confidence: gas.High,
-				What:       "Use of weak cryptographic primitive",
-			},
-			`"crypto/des"`: gas.MetaData{
-				Severity:   gas.High,
-				Confidence: gas.High,
-				What:       "Use of weak cryptographic primitive",
-			},
-			`"crypto/rc4"`: gas.MetaData{
-				Severity:   gas.High,
-				Confidence: gas.High,
-				What:       "Use of weak cryptographic primitive",
-			},
-			`"net/http/cgi"`: gas.MetaData{
-				Severity:   gas.High,
-				Confidence: gas.Low,
-				What:       "Go code running under CGI is vulnerable to Httpoxy attack. (CVE-2016-5386)",
-			},
+func NewBlacklist_crypto_md5(conf map[string]interface{}) (r gas.Rule, n ast.Node) {
+	r = &BlacklistImport{
+		MetaData: gas.MetaData{
+			Severity:   gas.High,
+			Confidence: gas.High,
+			What:       "Use of weak cryptographic primitive",
 		},
+		Path: `"crypto/md5"`,
 	}
+	n = (*ast.ImportSpec)(nil)
+	return
+}
+
+func NewBlacklist_crypto_des(conf map[string]interface{}) (r gas.Rule, n ast.Node) {
+	r = &BlacklistImport{
+		MetaData: gas.MetaData{
+			Severity:   gas.High,
+			Confidence: gas.High,
+			What:       "Use of weak cryptographic primitive",
+		},
+		Path: `"crypto/des"`,
+	}
+	n = (*ast.ImportSpec)(nil)
+	return
+}
 
+func NewBlacklist_crypto_rc4(conf map[string]interface{}) (r gas.Rule, n ast.Node) {
+	r = &BlacklistImport{
+		MetaData: gas.MetaData{
+			Severity:   gas.High,
+			Confidence: gas.High,
+			What:       "Use of weak cryptographic primitive",
+		},
+		Path: `"crypto/rc4"`,
+	}
+	n = (*ast.ImportSpec)(nil)
+	return
+}
+
+func NewBlacklist_net_http_cgi(conf map[string]interface{}) (r gas.Rule, n ast.Node) {
+	r = &BlacklistImport{
+		MetaData: gas.MetaData{
+			Severity:   gas.High,
+			Confidence: gas.High,
+			What:       "Go code running under CGI is vulnerable to Httpoxy attack. (CVE-2016-5386)",
+		},
+		Path: `"net/http/cgi"`,
+	}
 	n = (*ast.ImportSpec)(nil)
 	return
 }
diff --git a/rules/errors.go b/rules/errors.go
@@ -50,7 +50,7 @@ func (r *NoErrorCheck) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err err
 	return nil, nil
 }
 
-func NewNoErrorCheck() (r gas.Rule, n ast.Node) {
+func NewNoErrorCheck(conf map[string]interface{}) (r gas.Rule, n ast.Node) {
 	r = &NoErrorCheck{
 		MetaData: gas.MetaData{
 			Severity:   gas.Low,