From b8cdc32174800d132a17e36aa69d3592d754af4d Mon Sep 17 00:00:00 2001 From: Delon Wong Her Laang Date: Fri, 26 Jan 2018 11:16:49 +0800 Subject: [PATCH 01/29] Working version of xml result format. --- output/formatter.go | 76 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/output/formatter.go b/output/formatter.go index 39955c0..1659fdc 100644 --- a/output/formatter.go +++ b/output/formatter.go @@ -17,6 +17,7 @@ package output import ( "encoding/csv" "encoding/json" + "encoding/xml" htmlTemplate "html/template" "io" plainTemplate "text/template" @@ -36,6 +37,9 @@ const ( // ReportCSV set the output format to csv ReportCSV // CSV format + + // ReportXML set the output format to junit xml + ReportXML // JUnit XML format ) var text = `Results: @@ -57,6 +61,30 @@ type reportInfo struct { Stats *gas.Metrics } +type XMLReport struct { + XMLName xml.Name `xml:"testsuites"` + Testsuites []Testsuite `xml:"testsuite"` +} + +type Testsuite struct { + XMLName xml.Name `xml:"testsuite"` + Name string `xml:"name,attr"` + Tests int `xml:"tests,attr"` + Testcases []Testcase `xml:"testcase"` +} + +type Testcase struct { + XMLName xml.Name `xml:"testcase"` + Name string `xml:"name,attr"` + Failure Failure `xml:"failure"` +} + +type Failure struct { + XMLName xml.Name `xml:"failure"` + Message string `xml:"message,attr"` + Text string `xml:",innerxml"` +} + // CreateReport generates a report based for the supplied issues and metrics given // the specified format. The formats currently accepted are: json, csv, html and text. func CreateReport(w io.Writer, format string, issues []*gas.Issue, metrics *gas.Metrics) error { @@ -70,6 +98,8 @@ func CreateReport(w io.Writer, format string, issues []*gas.Issue, metrics *gas. err = reportJSON(w, data) case "csv": err = reportCSV(w, data) + case "xml": + err = reportXML(w, data) case "html": err = reportFromHTMLTemplate(w, html, data) case "text": @@ -112,6 +142,52 @@ func reportCSV(w io.Writer, data *reportInfo) error { return nil } +func reportXML(w io.Writer, data *reportInfo) error { + testsuites := make(map[string][]Testcase) + for _, issue := range data.Issues { + stacktrace, err := json.MarshalIndent(issue, "", "\t") + if err != nil { + panic(err) + } + testcase := Testcase{ + Name: issue.File, + Failure: Failure{ + Message: "Found 1 vulnerability. See stacktrace for details.", + Text: string(stacktrace), + }, + } + if _, ok := testsuites[issue.What]; ok { + testsuites[issue.What] = append(testsuites[issue.What], testcase) + } else { + testsuites[issue.What] = []Testcase{testcase} + } + } + + var xmlReport XMLReport + for what, testcases := range testsuites { + testsuite := Testsuite{ + Name: what, + Tests: len(testcases), + } + for _, testcase := range testcases { + testsuite.Testcases = append(testsuite.Testcases, testcase) + } + xmlReport.Testsuites = append(xmlReport.Testsuites, testsuite) + } + + raw, err := xml.Marshal(xmlReport) + if err != nil { + panic(err) + } + + _, err = w.Write(raw) + if err != nil { + panic(err) + } + + return err +} + func reportFromPlaintextTemplate(w io.Writer, reportTemplate string, data *reportInfo) error { t, e := plainTemplate.New("gas").Parse(reportTemplate) if e != nil { From 7539b3735fe331b36ece2ff80560d5f29d8f5f4f Mon Sep 17 00:00:00 2001 From: Wong Her Laang Date: Sat, 27 Jan 2018 11:49:58 +0800 Subject: [PATCH 02/29] Added xml header format. --- output/formatter.go | 1 + 1 file changed, 1 insertion(+) diff --git a/output/formatter.go b/output/formatter.go index 1659fdc..49fa232 100644 --- a/output/formatter.go +++ b/output/formatter.go @@ -180,6 +180,7 @@ func reportXML(w io.Writer, data *reportInfo) error { panic(err) } + raw = append([]byte("\n"), raw...) _, err = w.Write(raw) if err != nil { panic(err) From 2c1a0b87329fb7b17d6e1a7cc038d56b1ef5a766 Mon Sep 17 00:00:00 2001 From: Wong Her Laang Date: Sat, 27 Jan 2018 12:14:35 +0800 Subject: [PATCH 03/29] Refactored code. --- output/formatter.go | 66 ++++------------------------------- output/junit_xml_format.go | 70 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 59 deletions(-) create mode 100644 output/junit_xml_format.go diff --git a/output/formatter.go b/output/formatter.go index 49fa232..c9e4597 100644 --- a/output/formatter.go +++ b/output/formatter.go @@ -38,8 +38,8 @@ const ( // ReportCSV set the output format to csv ReportCSV // CSV format - // ReportXML set the output format to junit xml - ReportXML // JUnit XML format + // ReportJUnitXML set the output format to junit xml + ReportJUnitXML // JUnit XML format ) var text = `Results: @@ -61,30 +61,6 @@ type reportInfo struct { Stats *gas.Metrics } -type XMLReport struct { - XMLName xml.Name `xml:"testsuites"` - Testsuites []Testsuite `xml:"testsuite"` -} - -type Testsuite struct { - XMLName xml.Name `xml:"testsuite"` - Name string `xml:"name,attr"` - Tests int `xml:"tests,attr"` - Testcases []Testcase `xml:"testcase"` -} - -type Testcase struct { - XMLName xml.Name `xml:"testcase"` - Name string `xml:"name,attr"` - Failure Failure `xml:"failure"` -} - -type Failure struct { - XMLName xml.Name `xml:"failure"` - Message string `xml:"message,attr"` - Text string `xml:",innerxml"` -} - // CreateReport generates a report based for the supplied issues and metrics given // the specified format. The formats currently accepted are: json, csv, html and text. func CreateReport(w io.Writer, format string, issues []*gas.Issue, metrics *gas.Metrics) error { @@ -143,44 +119,16 @@ func reportCSV(w io.Writer, data *reportInfo) error { } func reportXML(w io.Writer, data *reportInfo) error { - testsuites := make(map[string][]Testcase) - for _, issue := range data.Issues { - stacktrace, err := json.MarshalIndent(issue, "", "\t") - if err != nil { - panic(err) - } - testcase := Testcase{ - Name: issue.File, - Failure: Failure{ - Message: "Found 1 vulnerability. See stacktrace for details.", - Text: string(stacktrace), - }, - } - if _, ok := testsuites[issue.What]; ok { - testsuites[issue.What] = append(testsuites[issue.What], testcase) - } else { - testsuites[issue.What] = []Testcase{testcase} - } - } + groupedData := groupDataByRules(data) + junitXMLStruct := createJUnitXMLStruct(groupedData) - var xmlReport XMLReport - for what, testcases := range testsuites { - testsuite := Testsuite{ - Name: what, - Tests: len(testcases), - } - for _, testcase := range testcases { - testsuite.Testcases = append(testsuite.Testcases, testcase) - } - xmlReport.Testsuites = append(xmlReport.Testsuites, testsuite) - } - - raw, err := xml.Marshal(xmlReport) + raw, err := xml.Marshal(junitXMLStruct) if err != nil { panic(err) } - raw = append([]byte("\n"), raw...) + xmlHeader := []byte("\n") + raw = append(xmlHeader, raw...) _, err = w.Write(raw) if err != nil { panic(err) diff --git a/output/junit_xml_format.go b/output/junit_xml_format.go new file mode 100644 index 0000000..ca7b323 --- /dev/null +++ b/output/junit_xml_format.go @@ -0,0 +1,70 @@ +package output + +import ( + "encoding/json" + "encoding/xml" + + "github.com/GoASTScanner/gas" +) + +type JUnitXMLReport struct { + XMLName xml.Name `xml:"testsuites"` + Testsuites []Testsuite `xml:"testsuite"` +} + +type Testsuite struct { + XMLName xml.Name `xml:"testsuite"` + Name string `xml:"name,attr"` + Tests int `xml:"tests,attr"` + Testcases []Testcase `xml:"testcase"` +} + +type Testcase struct { + XMLName xml.Name `xml:"testcase"` + Name string `xml:"name,attr"` + Failure Failure `xml:"failure"` +} + +type Failure struct { + XMLName xml.Name `xml:"failure"` + Message string `xml:"message,attr"` + Text string `xml:",innerxml"` +} + +func groupDataByRules(data *reportInfo) map[string][]*gas.Issue { + groupedData := make(map[string][]*gas.Issue) + for _, issue := range data.Issues { + if _, ok := groupedData[issue.What]; ok { + groupedData[issue.What] = append(groupedData[issue.What], issue) + } else { + groupedData[issue.What] = []*gas.Issue{issue} + } + } + return groupedData +} + +func createJUnitXMLStruct(groupedData map[string][]*gas.Issue) JUnitXMLReport { + var xmlReport JUnitXMLReport + for what, issues := range groupedData { + testsuite := Testsuite{ + Name: what, + Tests: len(issues), + } + for _, issue := range issues { + stacktrace, err := json.MarshalIndent(issue, "", "\t") + if err != nil { + panic(err) + } + testcase := Testcase{ + Name: issue.File, + Failure: Failure{ + Message: "Found 1 vulnerability. See stacktrace for details.", + Text: string(stacktrace), + }, + } + testsuite.Testcases = append(testsuite.Testcases, testcase) + } + xmlReport.Testsuites = append(xmlReport.Testsuites, testsuite) + } + return xmlReport +} From 1346bd37ca856bd77a1901a7402e2ee5c0b784f5 Mon Sep 17 00:00:00 2001 From: Wong Her Laang Date: Sat, 27 Jan 2018 12:19:38 +0800 Subject: [PATCH 04/29] Edited README and help text. --- README.md | 2 +- cmd/gas/main.go | 2 +- output/formatter.go | 6 +++--- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 9c6c140..cc3b160 100644 --- a/README.md +++ b/README.md @@ -104,7 +104,7 @@ $ gas -nosec=true ./... ### Output formats -Gas currently supports text, json and csv output formats. By default +Gas currently supports text, json, csv and JUnit XML output formats. By default results will be reported to stdout, but can also be written to an output file. The output format is controlled by the '-fmt' flag, and the output file is controlled by the '-out' flag as follows: diff --git a/cmd/gas/main.go b/cmd/gas/main.go index 68a6277..deef06e 100644 --- a/cmd/gas/main.go +++ b/cmd/gas/main.go @@ -59,7 +59,7 @@ var ( flagIgnoreNoSec = flag.Bool("nosec", false, "Ignores #nosec comments when set") // format output - flagFormat = flag.String("fmt", "text", "Set output format. Valid options are: json, csv, html, or text") + flagFormat = flag.String("fmt", "text", "Set output format. Valid options are: json, csv, junit-xml, html, or text") // output file flagOutput = flag.String("out", "", "Set output file for results") diff --git a/output/formatter.go b/output/formatter.go index c9e4597..da6eb7e 100644 --- a/output/formatter.go +++ b/output/formatter.go @@ -74,8 +74,8 @@ func CreateReport(w io.Writer, format string, issues []*gas.Issue, metrics *gas. err = reportJSON(w, data) case "csv": err = reportCSV(w, data) - case "xml": - err = reportXML(w, data) + case "junit-xml": + err = reportJUnitXML(w, data) case "html": err = reportFromHTMLTemplate(w, html, data) case "text": @@ -118,7 +118,7 @@ func reportCSV(w io.Writer, data *reportInfo) error { return nil } -func reportXML(w io.Writer, data *reportInfo) error { +func reportJUnitXML(w io.Writer, data *reportInfo) error { groupedData := groupDataByRules(data) junitXMLStruct := createJUnitXMLStruct(groupedData) From 4059facfb933c48d65d4fb9fb3f1f03615f19447 Mon Sep 17 00:00:00 2001 From: Wong Her Laang Date: Sat, 27 Jan 2018 12:25:54 +0800 Subject: [PATCH 05/29] Pretty print xml result for better viewing. --- output/formatter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/output/formatter.go b/output/formatter.go index da6eb7e..572e807 100644 --- a/output/formatter.go +++ b/output/formatter.go @@ -122,7 +122,7 @@ func reportJUnitXML(w io.Writer, data *reportInfo) error { groupedData := groupDataByRules(data) junitXMLStruct := createJUnitXMLStruct(groupedData) - raw, err := xml.Marshal(junitXMLStruct) + raw, err := xml.MarshalIndent(junitXMLStruct, "", "\t") if err != nil { panic(err) } From fdc78c0c4739ab2fe02e97c450f971b6ae973f33 Mon Sep 17 00:00:00 2001 From: Wong Her Laang Date: Sat, 27 Jan 2018 12:43:08 +0800 Subject: [PATCH 06/29] Changed failure text from json to plaintext. --- output/junit_xml_format.go | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/output/junit_xml_format.go b/output/junit_xml_format.go index ca7b323..290be81 100644 --- a/output/junit_xml_format.go +++ b/output/junit_xml_format.go @@ -1,8 +1,8 @@ package output import ( - "encoding/json" "encoding/xml" + "strconv" "github.com/GoASTScanner/gas" ) @@ -51,15 +51,17 @@ func createJUnitXMLStruct(groupedData map[string][]*gas.Issue) JUnitXMLReport { Tests: len(issues), } for _, issue := range issues { - stacktrace, err := json.MarshalIndent(issue, "", "\t") - if err != nil { - panic(err) - } + text := "Results:\n" + text += "[" + issue.File + ":" + issue.Line + "] - " + + issue.What + " (Confidence: " + strconv.Itoa(int(issue.Confidence)) + + ", Severity: " + strconv.Itoa(int(issue.Severity)) + ")\n" + text += "> " + issue.Code + testcase := Testcase{ Name: issue.File, Failure: Failure{ Message: "Found 1 vulnerability. See stacktrace for details.", - Text: string(stacktrace), + Text: text, }, } testsuite.Testcases = append(testsuite.Testcases, testcase) From 5b91afec367cc4fcc04a7bc81b7bdff859cce4aa Mon Sep 17 00:00:00 2001 From: Wong Her Laang Date: Sat, 27 Jan 2018 14:45:04 +0800 Subject: [PATCH 07/29] Unexport junit xml structs and some further refactoring. --- output/junit_xml_format.go | 39 +++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/output/junit_xml_format.go b/output/junit_xml_format.go index 290be81..1af54d3 100644 --- a/output/junit_xml_format.go +++ b/output/junit_xml_format.go @@ -7,30 +7,37 @@ import ( "github.com/GoASTScanner/gas" ) -type JUnitXMLReport struct { +type junitXMLReport struct { XMLName xml.Name `xml:"testsuites"` - Testsuites []Testsuite `xml:"testsuite"` + Testsuites []testsuite `xml:"testsuite"` } -type Testsuite struct { +type testsuite struct { XMLName xml.Name `xml:"testsuite"` Name string `xml:"name,attr"` Tests int `xml:"tests,attr"` - Testcases []Testcase `xml:"testcase"` + Testcases []testcase `xml:"testcase"` } -type Testcase struct { +type testcase struct { XMLName xml.Name `xml:"testcase"` Name string `xml:"name,attr"` - Failure Failure `xml:"failure"` + Failure failure `xml:"failure"` } -type Failure struct { +type failure struct { XMLName xml.Name `xml:"failure"` Message string `xml:"message,attr"` Text string `xml:",innerxml"` } +func genereratePlaintext(issue *gas.Issue) string { + return "Results:\n" + + "[" + issue.File + ":" + issue.Line + "] - " + + issue.What + " (Confidence: " + strconv.Itoa(int(issue.Confidence)) + + ", Severity: " + strconv.Itoa(int(issue.Severity)) + ")\n" + "> " + issue.Code +} + func groupDataByRules(data *reportInfo) map[string][]*gas.Issue { groupedData := make(map[string][]*gas.Issue) for _, issue := range data.Issues { @@ -43,25 +50,19 @@ func groupDataByRules(data *reportInfo) map[string][]*gas.Issue { return groupedData } -func createJUnitXMLStruct(groupedData map[string][]*gas.Issue) JUnitXMLReport { - var xmlReport JUnitXMLReport +func createJUnitXMLStruct(groupedData map[string][]*gas.Issue) junitXMLReport { + var xmlReport junitXMLReport for what, issues := range groupedData { - testsuite := Testsuite{ + testsuite := testsuite{ Name: what, Tests: len(issues), } for _, issue := range issues { - text := "Results:\n" - text += "[" + issue.File + ":" + issue.Line + "] - " + - issue.What + " (Confidence: " + strconv.Itoa(int(issue.Confidence)) + - ", Severity: " + strconv.Itoa(int(issue.Severity)) + ")\n" - text += "> " + issue.Code - - testcase := Testcase{ + testcase := testcase{ Name: issue.File, - Failure: Failure{ + Failure: failure{ Message: "Found 1 vulnerability. See stacktrace for details.", - Text: text, + Text: genereratePlaintext(issue), }, } testsuite.Testcases = append(testsuite.Testcases, testcase) From 143df04ede480a1aa78d2f2d55166187d5a2b6ba Mon Sep 17 00:00:00 2001 From: Wong Her Laang Date: Sat, 27 Jan 2018 22:23:07 +0800 Subject: [PATCH 08/29] Fixed typo. --- output/junit_xml_format.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/output/junit_xml_format.go b/output/junit_xml_format.go index 1af54d3..9796079 100644 --- a/output/junit_xml_format.go +++ b/output/junit_xml_format.go @@ -31,7 +31,7 @@ type failure struct { Text string `xml:",innerxml"` } -func genereratePlaintext(issue *gas.Issue) string { +func generatePlaintext(issue *gas.Issue) string { return "Results:\n" + "[" + issue.File + ":" + issue.Line + "] - " + issue.What + " (Confidence: " + strconv.Itoa(int(issue.Confidence)) + @@ -62,7 +62,7 @@ func createJUnitXMLStruct(groupedData map[string][]*gas.Issue) junitXMLReport { Name: issue.File, Failure: failure{ Message: "Found 1 vulnerability. See stacktrace for details.", - Text: genereratePlaintext(issue), + Text: generatePlaintext(issue), }, } testsuite.Testcases = append(testsuite.Testcases, testcase) From f111d5de2c5e8255c9a74184fa8481c580de55fd Mon Sep 17 00:00:00 2001 From: Jon McClintock Date: Mon, 29 Jan 2018 18:33:48 +0000 Subject: [PATCH 09/29] [Issue 159] Allow loader errors so that processing continues if there's a package loading problem. --- analyzer.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/analyzer.go b/analyzer.go index f050ab7..e336869 100644 --- a/analyzer.go +++ b/analyzer.go @@ -96,7 +96,11 @@ func (gas *Analyzer) LoadRules(ruleDefinitions ...RuleBuilder) { // Process kicks off the analysis process for a given package func (gas *Analyzer) Process(packagePaths ...string) error { - packageConfig := loader.Config{Build: &build.Default, ParserMode: parser.ParseComments} + packageConfig := loader.Config{ + Build: &build.Default, + ParserMode: parser.ParseComments, + AllowErrors: true, + } for _, packagePath := range packagePaths { abspath, _ := filepath.Abs(packagePath) gas.logger.Println("Searching directory:", abspath) From b49fef79a510c51f759b5bdb595986a782887058 Mon Sep 17 00:00:00 2001 From: Grant Murphy Date: Tue, 30 Jan 2018 09:27:55 +1000 Subject: [PATCH 10/29] Using godep not glide for dependency management --- glide.lock | 25 ------------------------- 1 file changed, 25 deletions(-) delete mode 100644 glide.lock diff --git a/glide.lock b/glide.lock deleted file mode 100644 index d76519f..0000000 --- a/glide.lock +++ /dev/null @@ -1,25 +0,0 @@ -hash: fb4cbcb4f806804f30683cd298d9316522f1d7678498039eccdb29f020de1c7f -updated: 2017-05-09T21:54:08.9517391-07:00 -imports: -- name: github.com/kisielk/gotool - version: 0de1eaf82fa3f583ce21fde859f1e7e0c5e9b220 -- name: github.com/nbutton23/zxcvbn-go - version: a22cb81b2ecdde8b68e9ffb8824731cbf88e1de4 - subpackages: - - adjacency - - data - - entropy - - frequency - - match - - matching - - scoring - - utils/math -- name: github.com/ryanuber/go-glob - version: 572520ed46dbddaed19ea3d9541bdd0494163693 -- name: golang.org/x/tools - version: 1dbffd0798679c0c6b466e620725135944cfddea - subpackages: - - go/ast/astutil - - go/buildutil - - go/loader -testImports: [] From 7c7fe752b6c35ba97859bb8d586795c8f9f19353 Mon Sep 17 00:00:00 2001 From: Grant Murphy Date: Tue, 30 Jan 2018 09:32:04 +1000 Subject: [PATCH 11/29] Fix go vet errors in tests --- analyzer_test.go | 4 +--- import_tracker_test.go | 3 ++- rules/rules_test.go | 3 +-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/analyzer_test.go b/analyzer_test.go index cff3415..3422b5c 100644 --- a/analyzer_test.go +++ b/analyzer_test.go @@ -1,7 +1,6 @@ package gas_test import ( - "bytes" "io/ioutil" "log" "os" @@ -20,10 +19,9 @@ var _ = Describe("Analyzer", func() { var ( analyzer *gas.Analyzer logger *log.Logger - output *bytes.Buffer ) BeforeEach(func() { - logger, output = testutils.NewLogger() + logger, _ = testutils.NewLogger() analyzer = gas.NewAnalyzer(nil, logger) }) diff --git a/import_tracker_test.go b/import_tracker_test.go index abde8b7..492933c 100644 --- a/import_tracker_test.go +++ b/import_tracker_test.go @@ -2,7 +2,7 @@ package gas_test import ( . "github.com/onsi/ginkgo" - //. "github.com/onsi/gomega" + . "github.com/onsi/gomega" ) var _ = Describe("ImportTracker", func() { @@ -15,6 +15,7 @@ var _ = Describe("ImportTracker", func() { }) Context("when I have a valid go package", func() { It("should record all import specs", func() { + Expect(source).To(Equal(source)) Skip("Not implemented") }) diff --git a/rules/rules_test.go b/rules/rules_test.go index 6bc0d7d..6b0f884 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -17,14 +17,13 @@ var _ = Describe("gas rules", func() { var ( logger *log.Logger - output *bytes.Buffer config gas.Config analyzer *gas.Analyzer runner func(string, []testutils.CodeSample) ) BeforeEach(func() { - logger, output = testutils.NewLogger() + logger, _ = testutils.NewLogger() config = gas.NewConfig() analyzer = gas.NewAnalyzer(config, logger) runner = func(rule string, samples []testutils.CodeSample) { From a97a196160e6d081dc87900f9d3ab7f0f93da5bb Mon Sep 17 00:00:00 2001 From: Grant Murphy Date: Tue, 30 Jan 2018 09:35:35 +1000 Subject: [PATCH 12/29] Unused import --- rules/rules_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/rules/rules_test.go b/rules/rules_test.go index 6b0f884..c42902f 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -1,7 +1,6 @@ package rules_test import ( - "bytes" "fmt" "log" From 862295cb7d907671c4c27abda5549c32e0a16cd6 Mon Sep 17 00:00:00 2001 From: Delon Wong Her Laang Date: Tue, 30 Jan 2018 09:54:30 +0800 Subject: [PATCH 13/29] Return err instead of panic. --- output/formatter.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/output/formatter.go b/output/formatter.go index 572e807..d829c53 100644 --- a/output/formatter.go +++ b/output/formatter.go @@ -124,17 +124,17 @@ func reportJUnitXML(w io.Writer, data *reportInfo) error { raw, err := xml.MarshalIndent(junitXMLStruct, "", "\t") if err != nil { - panic(err) + return err } xmlHeader := []byte("\n") raw = append(xmlHeader, raw...) _, err = w.Write(raw) if err != nil { - panic(err) + return err } - return err + return nil } func reportFromPlaintextTemplate(w io.Writer, reportTemplate string, data *reportInfo) error { From 33fff9514f211abc35f9723c87723f5b49fa0faa Mon Sep 17 00:00:00 2001 From: Delon Wong Her Laang Date: Thu, 1 Feb 2018 12:30:47 +0800 Subject: [PATCH 14/29] Excape html string for junit output. --- output/junit_xml_format.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/output/junit_xml_format.go b/output/junit_xml_format.go index 9796079..2fd5c39 100644 --- a/output/junit_xml_format.go +++ b/output/junit_xml_format.go @@ -2,6 +2,7 @@ package output import ( "encoding/xml" + htmlLib "html" "strconv" "github.com/GoASTScanner/gas" @@ -35,7 +36,7 @@ func generatePlaintext(issue *gas.Issue) string { return "Results:\n" + "[" + issue.File + ":" + issue.Line + "] - " + issue.What + " (Confidence: " + strconv.Itoa(int(issue.Confidence)) + - ", Severity: " + strconv.Itoa(int(issue.Severity)) + ")\n" + "> " + issue.Code + ", Severity: " + strconv.Itoa(int(issue.Severity)) + ")\n" + "> " + htmlLib.EscapeString(issue.Code) } func groupDataByRules(data *reportInfo) map[string][]*gas.Issue { From d3c3cd641921d6da8199acd1c662a578e751d9d6 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Tue, 6 Feb 2018 16:56:26 +0100 Subject: [PATCH 15/29] Add a rule to detect the usage of ssh InsecureIgnoreHostKey function --- rules/rulelist.go | 1 + rules/rules_test.go | 4 ++++ rules/ssh.go | 47 +++++++++++++++++++++++++++++++++++++++++++++ testutils/source.go | 9 +++++++++ 4 files changed, 61 insertions(+) create mode 100644 rules/ssh.go diff --git a/rules/rulelist.go b/rules/rulelist.go index 833b742..6cc3ee6 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -65,6 +65,7 @@ func Generate(filters ...RuleFilter) RuleList { "G103": RuleDefinition{"Audit the use of unsafe block", NewUsingUnsafe}, "G104": RuleDefinition{"Audit errors not checked", NewNoErrorCheck}, "G105": RuleDefinition{"Audit the use of big.Exp function", NewUsingBigExp}, + "G106": RuleDefinition{"Audit the use of ssh.InsecureIgnoreHostKey function", NewSSHHostKey}, // injection "G201": RuleDefinition{"SQL query construction using format string", NewSQLStrFormat}, diff --git a/rules/rules_test.go b/rules/rules_test.go index c42902f..a7dca95 100644 --- a/rules/rules_test.go +++ b/rules/rules_test.go @@ -65,6 +65,10 @@ var _ = Describe("gas rules", func() { runner("G105", testutils.SampleCodeG105) }) + It("should detect of ssh.InsecureIgnoreHostKey function", func() { + runner("G106", testutils.SampleCodeG106) + }) + It("should detect sql injection via format strings", func() { runner("G201", testutils.SampleCodeG201) }) diff --git a/rules/ssh.go b/rules/ssh.go new file mode 100644 index 0000000..857ac46 --- /dev/null +++ b/rules/ssh.go @@ -0,0 +1,47 @@ +// (c) Copyright 2016 Hewlett Packard Enterprise Development LP +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package rules + +import ( + "go/ast" + + "github.com/GoASTScanner/gas" +) + +type sshHostKey struct { + gas.MetaData + pkg string + calls []string +} + +func (r *sshHostKey) Match(n ast.Node, c *gas.Context) (gi *gas.Issue, err error) { + if _, matches := gas.MatchCallByPackage(n, c, r.pkg, r.calls...); matches { + return gas.NewIssue(c, n, r.What, r.Severity, r.Confidence), nil + } + return nil, nil +} + +// NewSSHHostKey rule detects the use of insecure ssh HostKeyCallback. +func NewSSHHostKey(conf gas.Config) (gas.Rule, []ast.Node) { + return &sshHostKey{ + pkg: "golang.org/x/crypto/ssh", + calls: []string{"InsecureIgnoreHostKey"}, + MetaData: gas.MetaData{ + What: "Use of ssh InsecureIgnoreHostKey should be audited", + Severity: gas.Medium, + Confidence: gas.High, + }, + }, []ast.Node{(*ast.CallExpr)(nil)} +} diff --git a/testutils/source.go b/testutils/source.go index f606e4a..6338763 100644 --- a/testutils/source.go +++ b/testutils/source.go @@ -183,6 +183,15 @@ func main() { z = z.Exp(x, y, m) }`, 1}} + // SampleCodeG106 - ssh InsecureIgnoreHostKey + SampleCodeG106 = []CodeSample{{` +package main +import ( + "golang.org/x/crypto/ssh" +) +func main() { + _ := ssh.InsecureIgnoreHostKey() +}`, 1}} // SampleCodeG201 - SQL injection via format string SampleCodeG201 = []CodeSample{ {` From f1b903f060cb8f139c6671e0b0c31dfadddbdc52 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Tue, 6 Feb 2018 16:59:00 +0100 Subject: [PATCH 16/29] Update README --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index cc3b160..1c9a1e4 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,7 @@ or to specify a set of rules to explicitly exclude using the '-exclude=' flag. - G103: Audit the use of unsafe block - G104: Audit errors not checked - G105: Audit the use of math/big.Int.Exp + - G106: Audit the use of ssh.InsecureIgnoreHostKey - G201: SQL query construction using format string - G202: SQL query construction using string concatenation - G203: Use of unescaped data in HTML templates From 179c178924936e79118de810689c4199c17ae35a Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Wed, 7 Feb 2018 09:23:52 +0100 Subject: [PATCH 17/29] Add some review fixes --- rules/ssh.go | 14 -------------- testutils/source.go | 2 +- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/rules/ssh.go b/rules/ssh.go index 857ac46..99b7e27 100644 --- a/rules/ssh.go +++ b/rules/ssh.go @@ -1,17 +1,3 @@ -// (c) Copyright 2016 Hewlett Packard Enterprise Development LP -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - package rules import ( diff --git a/testutils/source.go b/testutils/source.go index 6338763..eb4a826 100644 --- a/testutils/source.go +++ b/testutils/source.go @@ -190,7 +190,7 @@ import ( "golang.org/x/crypto/ssh" ) func main() { - _ := ssh.InsecureIgnoreHostKey() + _ = ssh.InsecureIgnoreHostKey() }`, 1}} // SampleCodeG201 - SQL injection via format string SampleCodeG201 = []CodeSample{ From a7cdd9cd8d41a49373e35d9d69cd9668e99b6c0a Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Wed, 7 Feb 2018 10:10:34 +0100 Subject: [PATCH 18/29] Add ssh package to the build The ssh package is not part of the standard library in go 1.5. --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index d144b31..37924cd 100644 --- a/.travis.yml +++ b/.travis.yml @@ -7,6 +7,7 @@ go: install: - go get -v github.com/onsi/ginkgo/ginkgo - go get -v github.com/onsi/gomega + - go get -v golang.org/x/crypto/ssh - go get -v -t ./... - export PATH=$PATH:$HOME/gopath/bin From c2c21553a31363ede1236799c29625f60e6b3f15 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Wed, 7 Feb 2018 14:07:24 +0100 Subject: [PATCH 19/29] Fix some gas warnings --- cmd/gas/main.go | 14 ++++++++++---- testutils/pkg.go | 3 ++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/cmd/gas/main.go b/cmd/gas/main.go index deef06e..80208ea 100644 --- a/cmd/gas/main.go +++ b/cmd/gas/main.go @@ -149,9 +149,15 @@ func saveOutput(filename, format string, issues []*gas.Issue, metrics *gas.Metri return err } defer outfile.Close() - output.CreateReport(outfile, format, issues, metrics) + err = output.CreateReport(outfile, format, issues, metrics) + if err != nil { + return err + } } else { - output.CreateReport(os.Stdout, format, issues, metrics) + err := output.CreateReport(os.Stdout, format, issues, metrics) + if err != nil { + return err + } } return nil } @@ -166,7 +172,7 @@ func main() { // Ensure at least one file was specified if flag.NArg() == 0 { - fmt.Fprintf(os.Stderr, "\nError: FILE [FILE...] or './...' expected\n") + fmt.Fprintf(os.Stderr, "\nError: FILE [FILE...] or './...' expected\n") // #nosec flag.Usage() os.Exit(1) } @@ -231,7 +237,7 @@ func main() { } // Finialize logging - logWriter.Close() + logWriter.Close() // #nosec // Do we have an issue? If so exit 1 if issuesFound { diff --git a/testutils/pkg.go b/testutils/pkg.go index 34404e7..569cd80 100644 --- a/testutils/pkg.go +++ b/testutils/pkg.go @@ -128,6 +128,7 @@ func (p *TestPackage) CreateContext(filename string) *gas.Context { // Close will delete the package and all files in that directory func (p *TestPackage) Close() { if p.ondisk { - os.RemoveAll(p.Path) + err := os.RemoveAll(p.Path) + log.Println(err) } } From 6cd7a6d7fe4c1102c03f6f48e102b12fde2644cc Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Wed, 7 Feb 2018 14:13:17 +0100 Subject: [PATCH 20/29] Add Fprint, Fprintf, Fprintln to NoErrorCheck whitelist --- rules/errors.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/errors.go b/rules/errors.go index cd06ed7..cda2087 100644 --- a/rules/errors.go +++ b/rules/errors.go @@ -77,7 +77,7 @@ func NewNoErrorCheck(conf gas.Config) (gas.Rule, []ast.Node) { // black list instead. whitelist := gas.NewCallList() whitelist.AddAll("bytes.Buffer", "Write", "WriteByte", "WriteRune", "WriteString") - whitelist.AddAll("fmt", "Print", "Printf", "Println") + whitelist.AddAll("fmt", "Print", "Printf", "Println", "Fprint", "Fprintf", "Fprintln") whitelist.Add("io.PipeWriter", "CloseWithError") if configured, ok := conf["G104"]; ok { From d4ebb032a9e3abd0af263b15b6355c636d1db363 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Thu, 8 Feb 2018 12:08:05 +0100 Subject: [PATCH 21/29] Sort the issues by severity in descending order before creating the report --- cmd/gas/main.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cmd/gas/main.go b/cmd/gas/main.go index 80208ea..bfbe676 100644 --- a/cmd/gas/main.go +++ b/cmd/gas/main.go @@ -79,6 +79,9 @@ var ( // log to file or stderr flagLogfile = flag.String("log", "", "Log messages to file rather than stderr") + // sort the issues by severity + flagSortIssues = flag.Bool("sort", true, "Sort issues by severity") + logger *log.Logger ) @@ -231,6 +234,11 @@ func main() { os.Exit(0) } + // Sort the issue by severity + if *flagSortIssues { + sort.Slice(issues, func(i, j int) bool { return (issues[i].Severity > issues[j].Severity) }) + } + // Create output report if err := saveOutput(*flagOutput, *flagFormat, issues, metrics); err != nil { logger.Fatal(err) From 84bfbbfd8c8867536218ab5b096d8790cbb36a88 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Sat, 10 Feb 2018 19:45:04 +0100 Subject: [PATCH 22/29] Switch to sort Interface to be backward compatible with older go versions --- cmd/gas/main.go | 2 +- cmd/gas/sort_issues.go | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) create mode 100644 cmd/gas/sort_issues.go diff --git a/cmd/gas/main.go b/cmd/gas/main.go index bfbe676..03dc30c 100644 --- a/cmd/gas/main.go +++ b/cmd/gas/main.go @@ -236,7 +236,7 @@ func main() { // Sort the issue by severity if *flagSortIssues { - sort.Slice(issues, func(i, j int) bool { return (issues[i].Severity > issues[j].Severity) }) + sortIssues(issues) } // Create output report diff --git a/cmd/gas/sort_issues.go b/cmd/gas/sort_issues.go new file mode 100644 index 0000000..5557f96 --- /dev/null +++ b/cmd/gas/sort_issues.go @@ -0,0 +1,20 @@ +package main + +import ( + "sort" + + "github.com/GoASTScanner/gas" +) + +type sortBySeverity []*gas.Issue + +func (s sortBySeverity) Len() int { return len(s) } + +func (s sortBySeverity) Less(i, j int) bool { return s[i].Severity > s[i].Severity } + +func (s sortBySeverity) Swap(i, j int) { s[i], s[j] = s[j], s[i] } + +// sortIssues sorts the issues by severity in descending order +func sortIssues(issues []*gas.Issue) { + sort.Sort(sortBySeverity(issues)) +} From e15c057349dd08da4de85dac639f30c682bcb14d Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Sat, 10 Feb 2018 19:46:39 +0100 Subject: [PATCH 23/29] Update the build file to validate gas from go version 1.7 onward --- .travis.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 37924cd..d14b1cf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,7 +2,9 @@ language: go before_script: - go vet $(go list ./... | grep -v /vendor/) go: - - 1.5 + - 1.7 + - 1.8 + - 1.9 - tip install: - go get -v github.com/onsi/ginkgo/ginkgo From e385ab872f78950708452976444af3411cd42491 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Sat, 10 Feb 2018 19:59:27 +0100 Subject: [PATCH 24/29] Update the build file with more checks Validate the tool from go version 1.7 onward --- .travis.yml | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 37924cd..15dee58 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,15 +1,25 @@ language: go -before_script: - - go vet $(go list ./... | grep -v /vendor/) + go: - - 1.5 + - 1.7 + - 1.8 + - 1.9 - tip + install: + - go get -u github.com/golang/lint/golint - go get -v github.com/onsi/ginkgo/ginkgo - go get -v github.com/onsi/gomega - go get -v golang.org/x/crypto/ssh + - go get github.com/GoASTScanner/gas/cmd/gas/... - go get -v -t ./... - export PATH=$PATH:$HOME/gopath/bin +before_script: + - test -z "$(gofmt -s -l -w $(find . -type f -name '*.go' -not -path './vendor/*') | tee /dev/stderr)" + - test -z "$(golint . | tee /dev/stderr)" + - go vet $(go list ./... | grep -v /vendor/) + - gas ./... + script: ginkgo -r From 230d286f4ea2a8c74642763e09ef58e73ee6294d Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Sat, 10 Feb 2018 20:04:58 +0100 Subject: [PATCH 25/29] Fix gofmt formatting --- import_tracker_test.go | 2 +- rules/rulelist.go | 42 +++++++++++++++++++++--------------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/import_tracker_test.go b/import_tracker_test.go index 492933c..a1a46f1 100644 --- a/import_tracker_test.go +++ b/import_tracker_test.go @@ -15,7 +15,7 @@ var _ = Describe("ImportTracker", func() { }) Context("when I have a valid go package", func() { It("should record all import specs", func() { - Expect(source).To(Equal(source)) + Expect(source).To(Equal(source)) Skip("Not implemented") }) diff --git a/rules/rulelist.go b/rules/rulelist.go index 6cc3ee6..2846368 100644 --- a/rules/rulelist.go +++ b/rules/rulelist.go @@ -60,35 +60,35 @@ func NewRuleFilter(action bool, ruleIDs ...string) RuleFilter { func Generate(filters ...RuleFilter) RuleList { rules := map[string]RuleDefinition{ // misc - "G101": RuleDefinition{"Look for hardcoded credentials", NewHardcodedCredentials}, - "G102": RuleDefinition{"Bind to all interfaces", NewBindsToAllNetworkInterfaces}, - "G103": RuleDefinition{"Audit the use of unsafe block", NewUsingUnsafe}, - "G104": RuleDefinition{"Audit errors not checked", NewNoErrorCheck}, - "G105": RuleDefinition{"Audit the use of big.Exp function", NewUsingBigExp}, - "G106": RuleDefinition{"Audit the use of ssh.InsecureIgnoreHostKey function", NewSSHHostKey}, + "G101": {"Look for hardcoded credentials", NewHardcodedCredentials}, + "G102": {"Bind to all interfaces", NewBindsToAllNetworkInterfaces}, + "G103": {"Audit the use of unsafe block", NewUsingUnsafe}, + "G104": {"Audit errors not checked", NewNoErrorCheck}, + "G105": {"Audit the use of big.Exp function", NewUsingBigExp}, + "G106": {"Audit the use of ssh.InsecureIgnoreHostKey function", NewSSHHostKey}, // injection - "G201": RuleDefinition{"SQL query construction using format string", NewSQLStrFormat}, - "G202": RuleDefinition{"SQL query construction using string concatenation", NewSQLStrConcat}, - "G203": RuleDefinition{"Use of unescaped data in HTML templates", NewTemplateCheck}, - "G204": RuleDefinition{"Audit use of command execution", NewSubproc}, + "G201": {"SQL query construction using format string", NewSQLStrFormat}, + "G202": {"SQL query construction using string concatenation", NewSQLStrConcat}, + "G203": {"Use of unescaped data in HTML templates", NewTemplateCheck}, + "G204": {"Audit use of command execution", NewSubproc}, // filesystem - "G301": RuleDefinition{"Poor file permissions used when creating a directory", NewMkdirPerms}, - "G302": RuleDefinition{"Poor file permisions used when creation file or using chmod", NewFilePerms}, - "G303": RuleDefinition{"Creating tempfile using a predictable path", NewBadTempFile}, + "G301": {"Poor file permissions used when creating a directory", NewMkdirPerms}, + "G302": {"Poor file permisions used when creation file or using chmod", NewFilePerms}, + "G303": {"Creating tempfile using a predictable path", NewBadTempFile}, // crypto - "G401": RuleDefinition{"Detect the usage of DES, RC4, or MD5", NewUsesWeakCryptography}, - "G402": RuleDefinition{"Look for bad TLS connection settings", NewIntermediateTLSCheck}, - "G403": RuleDefinition{"Ensure minimum RSA key length of 2048 bits", NewWeakKeyStrength}, - "G404": RuleDefinition{"Insecure random number source (rand)", NewWeakRandCheck}, + "G401": {"Detect the usage of DES, RC4, or MD5", NewUsesWeakCryptography}, + "G402": {"Look for bad TLS connection settings", NewIntermediateTLSCheck}, + "G403": {"Ensure minimum RSA key length of 2048 bits", NewWeakKeyStrength}, + "G404": {"Insecure random number source (rand)", NewWeakRandCheck}, // blacklist - "G501": RuleDefinition{"Import blacklist: crypto/md5", NewBlacklistedImportMD5}, - "G502": RuleDefinition{"Import blacklist: crypto/des", NewBlacklistedImportDES}, - "G503": RuleDefinition{"Import blacklist: crypto/rc4", NewBlacklistedImportRC4}, - "G504": RuleDefinition{"Import blacklist: net/http/cgi", NewBlacklistedImportCGI}, + "G501": {"Import blacklist: crypto/md5", NewBlacklistedImportMD5}, + "G502": {"Import blacklist: crypto/des", NewBlacklistedImportDES}, + "G503": {"Import blacklist: crypto/rc4", NewBlacklistedImportRC4}, + "G504": {"Import blacklist: net/http/cgi", NewBlacklistedImportCGI}, } for rule := range rules { From 7355f0a119e7fc2a0adc83c19d443cfeaca6c736 Mon Sep 17 00:00:00 2001 From: Cosmin Cojocar Date: Sat, 10 Feb 2018 20:10:56 +0100 Subject: [PATCH 26/29] Fix some gas warnings --- analyzer.go | 5 ++++- issue.go | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/analyzer.go b/analyzer.go index e336869..b7a5e8f 100644 --- a/analyzer.go +++ b/analyzer.go @@ -102,7 +102,10 @@ func (gas *Analyzer) Process(packagePaths ...string) error { AllowErrors: true, } for _, packagePath := range packagePaths { - abspath, _ := filepath.Abs(packagePath) + abspath, err := filepath.Abs(packagePath) + if err != nil { + return err + } gas.logger.Println("Searching directory:", abspath) basePackage, err := build.Default.ImportDir(packagePath, build.ImportComment) diff --git a/issue.go b/issue.go index 1060f43..2113529 100644 --- a/issue.go +++ b/issue.go @@ -76,7 +76,7 @@ func codeSnippet(file *os.File, start int64, end int64, n ast.Node) (string, err } size := (int)(end - start) // Go bug, os.File.Read should return int64 ... - file.Seek(start, 0) + file.Seek(start, 0) // #nosec buf := make([]byte, size) if nread, err := file.Read(buf); err != nil || nread != size { From 1c58cbd378dcd475004adff338b48c0a417bb8f6 Mon Sep 17 00:00:00 2001 From: cosmincojocar Date: Thu, 15 Feb 2018 10:53:01 +0100 Subject: [PATCH 27/29] Make the folder permissions more permissive to avoid false positives (#175) --- rules/fileperms.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rules/fileperms.go b/rules/fileperms.go index d69e46f..b48720f 100644 --- a/rules/fileperms.go +++ b/rules/fileperms.go @@ -75,7 +75,7 @@ func NewFilePerms(conf gas.Config) (gas.Rule, []ast.Node) { // NewMkdirPerms creates a rule to detect directory creation with more permissive than // configured permission mask. func NewMkdirPerms(conf gas.Config) (gas.Rule, []ast.Node) { - mode := getConfiguredMode(conf, "G301", 0700) + mode := getConfiguredMode(conf, "G301", 0750) return &filePermissions{ mode: mode, pkg: "os", From edb362fc9d80dd292e27f32a8070494d52d66ed9 Mon Sep 17 00:00:00 2001 From: cosmincojocar Date: Wed, 21 Feb 2018 06:59:18 +0100 Subject: [PATCH 28/29] Add a tool to generate the TLS configuration form Mozilla's ciphers recommendation (#178) * Add a tool which generates the TLS rule configuration from Mozilla server side TLS configuration * Update README * Remove trailing space in README * Update dependencies * Fix the commends of the generated functions --- Godeps/Godeps.json | 19 ++- README.md | 18 +++ cmd/tlsconfig/header_template.go | 13 ++ cmd/tlsconfig/rule_template.go | 19 +++ cmd/tlsconfig/tlsconfig.go | 204 +++++++++++++++++++++++++++++++ rules/tls.go | 73 +---------- rules/tls_config.go | 132 ++++++++++++++++++++ 7 files changed, 406 insertions(+), 72 deletions(-) create mode 100644 cmd/tlsconfig/header_template.go create mode 100644 cmd/tlsconfig/rule_template.go create mode 100644 cmd/tlsconfig/tlsconfig.go create mode 100644 rules/tls_config.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index a7508b4..7763c31 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -1,8 +1,20 @@ { "ImportPath": "github.com/GoASTScanner/gas", "GoVersion": "go1.9", - "GodepVersion": "v79", + "GodepVersion": "v80", + "Packages": [ + "./..." + ], "Deps": [ + { + "ImportPath": "github.com/kisielk/gotool", + "Rev": "0de1eaf82fa3f583ce21fde859f1e7e0c5e9b220" + }, + { + "ImportPath": "github.com/mozilla/tls-observatory/constants", + "Comment": "1.2.32-17-g17e0ce4b", + "Rev": "17e0ce4bfc46eae3d57acf13a2d7c7517655d493" + }, { "ImportPath": "github.com/nbutton23/zxcvbn-go", "Rev": "a22cb81b2ecdde8b68e9ffb8824731cbf88e1de4" @@ -189,6 +201,11 @@ "Comment": "v1.2.0-2-gdcabb60", "Rev": "dcabb60a477c2b6f456df65037cb6708210fbb02" }, + { + "ImportPath": "github.com/ryanuber/go-glob", + "Comment": "v0.1-4-g256dc44", + "Rev": "256dc444b735e061061cf46c809487313d5b0065" + }, { "ImportPath": "golang.org/x/net/html", "Rev": "8351a756f30f1297fe94bbf4b767ec589c6ea6d0" diff --git a/README.md b/README.md index 1c9a1e4..7dace9e 100644 --- a/README.md +++ b/README.md @@ -114,3 +114,21 @@ file. The output format is controlled by the '-fmt' flag, and the output file is $ gas -fmt=json -out=results.json *.go ``` +### Generate TLS rule + +The configuration of TLS rule can be generated from [Mozilla's TLS ciphers recommendation](https://statics.tls.security.mozilla.org/server-side-tls-conf.json). + + +First you need to install the generator tool: + +``` +go get github.com/GoASTScanner/gas/cmd/tlsconfig/... +``` + +You can invoke now the `go generate` in the root of the project: + +``` +go generate ./... +``` + +This will generate the `rules/tls_config.go` file with will contain the current ciphers recommendation from Mozilla. diff --git a/cmd/tlsconfig/header_template.go b/cmd/tlsconfig/header_template.go new file mode 100644 index 0000000..618221b --- /dev/null +++ b/cmd/tlsconfig/header_template.go @@ -0,0 +1,13 @@ +package main + +import "text/template" + +var generatedHeaderTmpl = template.Must(template.New("generated").Parse(` +package {{.}} + +import ( + "go/ast" + + "github.com/GoASTScanner/gas" +) +`)) diff --git a/cmd/tlsconfig/rule_template.go b/cmd/tlsconfig/rule_template.go new file mode 100644 index 0000000..05ed831 --- /dev/null +++ b/cmd/tlsconfig/rule_template.go @@ -0,0 +1,19 @@ +package main + +import "text/template" + +var generatedRuleTmpl = template.Must(template.New("generated").Parse(` +// New{{.Name}}TLSCheck creates a check for {{.Name}} TLS ciphers +// DO NOT EDIT - generated by tlsconfig tool +func New{{.Name}}TLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { + return &insecureConfigTLS{ + requiredType: "crypto/tls.Config", + MinVersion: {{ .MinVersion }}, + MaxVersion: {{ .MaxVersion }}, + goodCiphers: []string{ +{{range $cipherName := .Ciphers }} "{{$cipherName}}", +{{end}} + }, + }, []ast.Node{(*ast.CompositeLit)(nil)} +} +`)) diff --git a/cmd/tlsconfig/tlsconfig.go b/cmd/tlsconfig/tlsconfig.go new file mode 100644 index 0000000..90a324a --- /dev/null +++ b/cmd/tlsconfig/tlsconfig.go @@ -0,0 +1,204 @@ +package main + +import ( + "bytes" + "crypto/tls" + "encoding/json" + "errors" + "flag" + "fmt" + "go/format" + "io/ioutil" + "log" + "net/http" + "path/filepath" + "sort" + "strings" + + "github.com/mozilla/tls-observatory/constants" +) + +var ( + pkg = flag.String("pkg", "rules", "package name to be added to the output file") + outputFile = flag.String("outputFile", "tls_config.go", "name of the output file") +) + +// TLSConfURL url where Mozilla publishes the TLS ciphers recommendations +const TLSConfURL = "https://statics.tls.security.mozilla.org/server-side-tls-conf.json" + +// ServerSideTLSJson contains all the available configurations and the version of the current document. +type ServerSideTLSJson struct { + Configurations map[string]Configuration `json:"configurations"` + Version float64 `json:"version"` +} + +// Configuration represents configurations levels declared by the Mozilla server-side-tls +// see https://wiki.mozilla.org/Security/Server_Side_TLS +type Configuration struct { + OpenSSLCiphersuites string `json:"openssl_ciphersuites"` + Ciphersuites []string `json:"ciphersuites"` + TLSVersions []string `json:"tls_versions"` + TLSCurves []string `json:"tls_curves"` + CertificateTypes []string `json:"certificate_types"` + CertificateCurves []string `json:"certificate_curves"` + CertificateSignatures []string `json:"certificate_signatures"` + RsaKeySize float64 `json:"rsa_key_size"` + DHParamSize float64 `json:"dh_param_size"` + ECDHParamSize float64 `json:"ecdh_param_size"` + HstsMinAge float64 `json:"hsts_min_age"` + OldestClients []string `json:"oldest_clients"` +} + +type goCipherConfiguration struct { + Name string + Ciphers []string + MinVersion string + MaxVersion string +} + +type goTLSConfiguration struct { + cipherConfigs []goCipherConfiguration +} + +// getTLSConfFromURL retrieves the json containing the TLS configurations from the specified URL. +func getTLSConfFromURL(url string) (*ServerSideTLSJson, error) { + r, err := http.Get(url) + if err != nil { + return nil, err + } + defer r.Body.Close() + + var sstls ServerSideTLSJson + err = json.NewDecoder(r.Body).Decode(&sstls) + if err != nil { + return nil, err + } + + return &sstls, nil +} + +func getGoCipherConfig(name string, sstls ServerSideTLSJson) (goCipherConfiguration, error) { + cipherConf := goCipherConfiguration{Name: strings.Title(name)} + conf, ok := sstls.Configurations[name] + if !ok { + return cipherConf, fmt.Errorf("TLS configuration '%s' not found", name) + } + + for _, cipherName := range conf.Ciphersuites { + cipherSuite, ok := constants.CipherSuites[cipherName] + if !ok { + log.Printf("Warning: cannot map cipher '%s'\n", cipherName) + } + if len(cipherSuite.IANAName) > 0 { + cipherConf.Ciphers = append(cipherConf.Ciphers, cipherSuite.IANAName) + } + } + + versions := mapTLSVersions(conf.TLSVersions) + if len(versions) > 0 { + cipherConf.MinVersion = fmt.Sprintf("0x%04x", versions[0]) + cipherConf.MaxVersion = fmt.Sprintf("0x%04x", versions[len(versions)-1]) + } else { + return cipherConf, fmt.Errorf("No TLS versions found for configuration '%s'", name) + } + return cipherConf, nil +} + +func mapTLSVersions(tlsVersions []string) []int { + var versions []int + for _, tlsVersion := range tlsVersions { + switch tlsVersion { + case "TLSv1.2": + versions = append(versions, tls.VersionTLS12) + case "TLSv1.1": + versions = append(versions, tls.VersionTLS11) + case "TLSv1": + versions = append(versions, tls.VersionTLS10) + case "SSLv3": + versions = append(versions, tls.VersionSSL30) + default: + continue + } + } + sort.Ints(versions) + return versions +} + +func getGoTLSConf() (goTLSConfiguration, error) { + sstls, err := getTLSConfFromURL(TLSConfURL) + if err != nil || sstls == nil { + msg := fmt.Sprintf("Could not load the Server Side TLS configuration from Mozilla's website. Check the URL: %s. Error: %v\n", + TLSConfURL, err) + panic(msg) + } + + tlsConfg := goTLSConfiguration{} + + modern, err := getGoCipherConfig("modern", *sstls) + if err != nil { + return tlsConfg, err + } + tlsConfg.cipherConfigs = append(tlsConfg.cipherConfigs, modern) + + intermediate, err := getGoCipherConfig("intermediate", *sstls) + if err != nil { + return tlsConfg, err + } + tlsConfg.cipherConfigs = append(tlsConfg.cipherConfigs, intermediate) + + old, err := getGoCipherConfig("old", *sstls) + if err != nil { + return tlsConfg, err + } + tlsConfg.cipherConfigs = append(tlsConfg.cipherConfigs, old) + + return tlsConfg, nil +} + +func getCurrentDir() (string, error) { + dir := "." + if args := flag.Args(); len(args) == 1 { + dir = args[0] + } else if len(args) > 1 { + return "", errors.New("only one directory at a time") + } + dir, err := filepath.Abs(dir) + if err != nil { + return "", err + } + return dir, nil +} + +func main() { + dir, err := getCurrentDir() + if err != nil { + log.Fatalln(err) + } + tlsConfig, err := getGoTLSConf() + if err != nil { + log.Fatalln(err) + } + + var buf bytes.Buffer + err = generatedHeaderTmpl.Execute(&buf, *pkg) + if err != nil { + log.Fatalf("Failed to generate the header: %v", err) + } + for _, cipherConfig := range tlsConfig.cipherConfigs { + err := generatedRuleTmpl.Execute(&buf, cipherConfig) + if err != nil { + log.Fatalf("Failed to generated the cipher config: %v", err) + } + } + + src, err := format.Source(buf.Bytes()) + if err != nil { + log.Printf("warnings: Failed to format the code: %v", err) + src = buf.Bytes() + } + + outputPath := filepath.Join(dir, *outputFile) + if err := ioutil.WriteFile(outputPath, src, 0644); err != nil { + log.Fatalf("Writing output: %s", err) + } +} diff --git a/rules/tls.go b/rules/tls.go index e5a8914..f0eb2d8 100644 --- a/rules/tls.go +++ b/rules/tls.go @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +//go:generate tlsconfig + package rules import ( @@ -118,74 +120,3 @@ func (t *insecureConfigTLS) Match(n ast.Node, c *gas.Context) (*gas.Issue, error } return nil, nil } - -// NewModernTLSCheck see: https://wiki.mozilla.org/Security/Server_Side_TLS#Modern_compatibility -func NewModernTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { - return &insecureConfigTLS{ - requiredType: "crypto/tls.Config", - MinVersion: 0x0303, // TLS 1.2 only - MaxVersion: 0x0303, - goodCiphers: []string{ - "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305", - "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305", - }, - }, []ast.Node{(*ast.CompositeLit)(nil)} -} - -// NewIntermediateTLSCheck see: https://wiki.mozilla.org/Security/Server_Side_TLS#Intermediate_compatibility_.28default.29 -func NewIntermediateTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { - return &insecureConfigTLS{ - requiredType: "crypto/tls.Config", - MinVersion: 0x0301, // TLS 1.2, 1.1, 1.0 - MaxVersion: 0x0303, - goodCiphers: []string{ - "TLS_RSA_WITH_AES_128_CBC_SHA", - "TLS_RSA_WITH_AES_256_CBC_SHA", - "TLS_RSA_WITH_AES_128_GCM_SHA256", - "TLS_RSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_RSA_WITH_RC4_128_SHA", - "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", - }, - }, []ast.Node{(*ast.CompositeLit)(nil)} -} - -// NewCompatTLSCheck see: https://wiki.mozilla.org/Security/Server_Side_TLS#Old_compatibility_.28default.29 -func NewCompatTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { - return &insecureConfigTLS{ - requiredType: "crypto/tls.Config", - MinVersion: 0x0301, // TLS 1.2, 1.1, 1.0 - MaxVersion: 0x0303, - goodCiphers: []string{ - "TLS_RSA_WITH_RC4_128_SHA", - "TLS_RSA_WITH_3DES_EDE_CBC_SHA", - "TLS_RSA_WITH_AES_128_CBC_SHA", - "TLS_RSA_WITH_AES_256_CBC_SHA", - "TLS_RSA_WITH_AES_128_GCM_SHA256", - "TLS_RSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", - "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_RSA_WITH_RC4_128_SHA", - "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", - "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", - }, - }, []ast.Node{(*ast.CompositeLit)(nil)} -} diff --git a/rules/tls_config.go b/rules/tls_config.go new file mode 100644 index 0000000..4f7afd3 --- /dev/null +++ b/rules/tls_config.go @@ -0,0 +1,132 @@ +package rules + +import ( + "go/ast" + + "github.com/GoASTScanner/gas" +) + +// NewModernTLSCheck creates a check for Modern TLS ciphers +// DO NOT EDIT - generated by tlsconfig tool +func NewModernTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { + return &insecureConfigTLS{ + requiredType: "crypto/tls.Config", + MinVersion: 0x0303, + MaxVersion: 0x0303, + goodCiphers: []string{ + "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384", + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", + }, + }, []ast.Node{(*ast.CompositeLit)(nil)} +} + +// NewIntermediateTLSCheck creates a check for Intermediate TLS ciphers +// DO NOT EDIT - generated by tlsconfig tool +func NewIntermediateTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { + return &insecureConfigTLS{ + requiredType: "crypto/tls.Config", + MinVersion: 0x0301, + MaxVersion: 0x0303, + goodCiphers: []string{ + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_DHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_DHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384", + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384", + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", + "TLS_DHE_RSA_WITH_AES_128_CBC_SHA256", + "TLS_DHE_RSA_WITH_AES_128_CBC_SHA", + "TLS_DHE_RSA_WITH_AES_256_CBC_SHA256", + "TLS_DHE_RSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA", + "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_RSA_WITH_AES_128_GCM_SHA256", + "TLS_RSA_WITH_AES_256_GCM_SHA384", + "TLS_RSA_WITH_AES_128_CBC_SHA256", + "TLS_RSA_WITH_AES_256_CBC_SHA256", + "TLS_RSA_WITH_AES_128_CBC_SHA", + "TLS_RSA_WITH_AES_256_CBC_SHA", + "TLS_RSA_WITH_3DES_EDE_CBC_SHA", + }, + }, []ast.Node{(*ast.CompositeLit)(nil)} +} + +// NewOldTLSCheck creates a check for Old TLS ciphers +// DO NOT EDIT - generated by tlsconfig tool +func NewOldTLSCheck(conf gas.Config) (gas.Rule, []ast.Node) { + return &insecureConfigTLS{ + requiredType: "crypto/tls.Config", + MinVersion: 0x0300, + MaxVersion: 0x0303, + goodCiphers: []string{ + "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", + "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", + "TLS_DHE_RSA_WITH_AES_128_GCM_SHA256", + "TLS_DHE_DSS_WITH_AES_128_GCM_SHA256", + "TLS_DHE_DSS_WITH_AES_256_GCM_SHA384", + "TLS_DHE_RSA_WITH_AES_256_GCM_SHA384", + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", + "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384", + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384", + "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", + "TLS_DHE_RSA_WITH_AES_128_CBC_SHA256", + "TLS_DHE_RSA_WITH_AES_128_CBC_SHA", + "TLS_DHE_DSS_WITH_AES_128_CBC_SHA256", + "TLS_DHE_RSA_WITH_AES_256_CBC_SHA256", + "TLS_DHE_DSS_WITH_AES_256_CBC_SHA", + "TLS_DHE_RSA_WITH_AES_256_CBC_SHA", + "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_ECDHE_ECDSA_WITH_3DES_EDE_CBC_SHA", + "TLS_DHE_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_RSA_WITH_AES_128_GCM_SHA256", + "TLS_RSA_WITH_AES_256_GCM_SHA384", + "TLS_RSA_WITH_AES_128_CBC_SHA256", + "TLS_RSA_WITH_AES_256_CBC_SHA256", + "TLS_RSA_WITH_AES_128_CBC_SHA", + "TLS_RSA_WITH_AES_256_CBC_SHA", + "TLS_DHE_DSS_WITH_AES_256_CBC_SHA256", + "TLS_DHE_DSS_WITH_AES_128_CBC_SHA", + "TLS_RSA_WITH_3DES_EDE_CBC_SHA", + "TLS_ECDHE_RSA_WITH_CAMELLIA_256_CBC_SHA384", + "TLS_ECDHE_ECDSA_WITH_CAMELLIA_256_CBC_SHA384", + "TLS_DHE_RSA_WITH_CAMELLIA_256_CBC_SHA256", + "TLS_DHE_DSS_WITH_CAMELLIA_256_CBC_SHA256", + "TLS_DHE_RSA_WITH_CAMELLIA_256_CBC_SHA", + "TLS_DHE_DSS_WITH_CAMELLIA_256_CBC_SHA", + "TLS_RSA_WITH_CAMELLIA_256_CBC_SHA256", + "TLS_RSA_WITH_CAMELLIA_256_CBC_SHA", + "TLS_ECDHE_RSA_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_ECDHE_ECDSA_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_DHE_RSA_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_DHE_DSS_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_DHE_RSA_WITH_CAMELLIA_128_CBC_SHA", + "TLS_DHE_DSS_WITH_CAMELLIA_128_CBC_SHA", + "TLS_RSA_WITH_CAMELLIA_128_CBC_SHA256", + "TLS_RSA_WITH_CAMELLIA_128_CBC_SHA", + "TLS_DHE_RSA_WITH_SEED_CBC_SHA", + "TLS_DHE_DSS_WITH_SEED_CBC_SHA", + "TLS_RSA_WITH_SEED_CBC_SHA", + }, + }, []ast.Node{(*ast.CompositeLit)(nil)} +} From c6183b4d5c4c5ad1621ce4346ddf0c38f6cf9fd4 Mon Sep 17 00:00:00 2001 From: Grant Murphy Date: Wed, 28 Feb 2018 04:29:25 +1000 Subject: [PATCH 29/29] Add nil pointer check to rule. (#181) TypeOf returns the type of expression e, or nil if not found. We are calling .String() on a value that may be nil in this clause. Relates to #174 --- rules/tls.go | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/rules/tls.go b/rules/tls.go index f0eb2d8..2dce11d 100644 --- a/rules/tls.go +++ b/rules/tls.go @@ -108,12 +108,15 @@ func (t *insecureConfigTLS) processTLSConfVal(n *ast.KeyValueExpr, c *gas.Contex } func (t *insecureConfigTLS) Match(n ast.Node, c *gas.Context) (*gas.Issue, error) { - if complit, ok := n.(*ast.CompositeLit); ok && complit.Type != nil && c.Info.TypeOf(complit.Type).String() == t.requiredType { - for _, elt := range complit.Elts { - if kve, ok := elt.(*ast.KeyValueExpr); ok { - issue := t.processTLSConfVal(kve, c) - if issue != nil { - return issue, nil + if complit, ok := n.(*ast.CompositeLit); ok && complit.Type != nil { + actualType := c.Info.TypeOf(complit.Type) + if actualType != nil && actualType.String() == t.requiredType { + for _, elt := range complit.Elts { + if kve, ok := elt.(*ast.KeyValueExpr); ok { + issue := t.processTLSConfVal(kve, c) + if issue != nil { + return issue, nil + } } } }