Formatting and linting #38
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
```
$ staticcheck
apihandler.go:434:4: error strings should not be capitalized (ST1005)
dnshandler.go:49:6: func xxxGetKeepFunc is unused (U1000)
dnshandler.go:101:12: unnecessary assignment to the blank identifier (S1005)
dnshandler.go:267:3: redundant return statement (S1023)
dnshandler.go:378:3: redundant return statement (S1023)
main.go:26:2: var soreuseport is unused (U1000)
main.go:76:3: printf-style function with dynamic format string and no further arguments should use print-style function instead (SA1006)
output.go:115:11: unnecessary assignment to the blank identifier (S1005)
output.go:116:5: empty branch (SA9003)
output.go:172:12: unnecessary assignment to the blank identifier (S1005)
output.go:215:18: this result of append is never used, except maybe in other appends (SA4010)
output.go:409:12: empty branch (SA9003)
refreshengine.go:58:14: should use time.Until instead of t.Sub(time.Now()) (S1024)
refreshengine.go:64:3: should use for range instead of for { select {} } (S1000)
sources.go:284:8: should omit comparison to bool constant, can be simplified to !s["active"].(bool) (S1002)
sources.go:466:10: error strings should not be capitalized (ST1005)
sources.go:466:10: error strings should not end with punctuation or newlines (ST1005)
version.go:4:7: const appDate is unused (U1000)
version.go:5:7: const appName is unused (U1000)
xfr.go:153:11: assigning the result of this type assertion to a variable (switch rr := rr.(type)) could eliminate type assertions in switch cases (S1034)
xfr.go:155:17: could eliminate this type assertion
```
```
$ gosec ./...
[...]
Results:
[/Users/patlu/git/sunet/dnstapir/tem/apihandler.go:402] - G114 (CWE-676): Use of net/http serve function that has no support for setting timeouts (Confidence: HIGH, Severity: MEDIUM)
401: log.Println("Starting API dispatcher #1. Listening on", address)
> 402: TEMExiter(http.ListenAndServe(address, router))
403: }()
[/Users/patlu/git/sunet/dnstapir/tem/output.go:65] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
64: if serialFile != "" {
> 65: serialData, err := os.ReadFile(serialFile)
66: if err != nil {
[/Users/patlu/git/sunet/dnstapir/tem/logging.go:73] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
72: if logfile != "" {
> 73: f, err := os.OpenFile(logfile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
74: if err != nil {
[/Users/patlu/git/sunet/dnstapir/tem/logging.go:53] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
52: if logfile != "" {
> 53: f, err := os.OpenFile(logfile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
54: if err != nil {
[/Users/patlu/git/sunet/dnstapir/tem/logging.go:33] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
32: if logfile != "" {
> 33: f, err := os.OpenFile(logfile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
34: if err != nil {
[/Users/patlu/git/sunet/dnstapir/tem/apihandler.go:392-396] - G112 (CWE-400): Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (Confidence: LOW, Severity: MEDIUM)
391:
> 392: tlsServer := &http.Server{
> 393: Addr: tlsaddress,
> 394: Handler: router,
> 395: TLSConfig: tlsConfig,
> 396: }
397:
[/Users/patlu/git/sunet/dnstapir/tem/logging.go:73] - G302 (CWE-276): Expect file permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
72: if logfile != "" {
> 73: f, err := os.OpenFile(logfile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
74: if err != nil {
[/Users/patlu/git/sunet/dnstapir/tem/logging.go:53] - G302 (CWE-276): Expect file permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
52: if logfile != "" {
> 53: f, err := os.OpenFile(logfile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
54: if err != nil {
[/Users/patlu/git/sunet/dnstapir/tem/logging.go:33] - G302 (CWE-276): Expect file permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
32: if logfile != "" {
> 33: f, err := os.OpenFile(logfile, os.O_RDWR|os.O_CREATE|os.O_APPEND, 0666)
34: if err != nil {
[/Users/patlu/git/sunet/dnstapir/tem/main.go:37] - G306 (CWE-276): Expect WriteFile permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)
36: serialData := []byte(fmt.Sprintf("%d", td.Rpz.CurrentSerial))
> 37: err := os.WriteFile(serialFile, serialData, 0644)
38: if err != nil {
[/Users/patlu/git/sunet/dnstapir/tem/xfr.go:267] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
266: wg.Wait() // wait until everything is written out
> 267: w.Close() // close connection
268:
[/Users/patlu/git/sunet/dnstapir/tem/xfr.go:177] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
176: go func() {
> 177: tr.Out(w, r, outbound_xfr)
178: wg.Done()
[/Users/patlu/git/sunet/dnstapir/tem/xfr.go:114] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
113: wg.Wait() // wait until everything is written out
> 114: w.Close() // close connection
115:
[/Users/patlu/git/sunet/dnstapir/tem/refreshengine.go:304] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
303: if cmd.ListType == "greylist" {
> 304: td.GreylistAdd(cmd.Domain, cmd.Policy, cmd.RpzSource)
305: resp.Msg = fmt.Sprintf("Domain name \"%s\" (policy %s) added to greylisting DB.",
[/Users/patlu/git/sunet/dnstapir/tem/refreshengine.go:270] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
269: rc = refreshCounters[zone]
> 270: td.NotifyDownstreams()
271: resp.Msg = fmt.Sprintf("Zone %s: bumped serial from %d to %d. Notified downstreams: %v",
[/Users/patlu/git/sunet/dnstapir/tem/refreshengine.go:242] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
241: if updated {
> 242: td.NotifyDownstreams()
243: }
[/Users/patlu/git/sunet/dnstapir/tem/refreshengine.go:138] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
137: }
> 138: td.NotifyDownstreams()
139: }
[/Users/patlu/git/sunet/dnstapir/tem/refreshengine.go:90] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
89: tpkg.Data.SrcName, len(tpkg.Data.Added), len(tpkg.Data.Removed))
> 90: td.ProcessTapirUpdate(tpkg)
91: log.Printf("RefreshEngine: Tapir IntelUpdate evaluated.")
[/Users/patlu/git/sunet/dnstapir/tem/main.go:159] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
158:
> 159: ValidateConfig(nil, cfgFileUsed) // will terminate on error
160:
[/Users/patlu/git/sunet/dnstapir/tem/main.go:103] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
102: log.Printf("mainloop: API instruction to stop\n")
> 103: td.SaveRpzSerial()
104: wg.Done()
[/Users/patlu/git/sunet/dnstapir/tem/main.go:87] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
86: log.Println("mainloop: Exit signal received. Cleaning up.")
> 87: td.SaveRpzSerial()
88: // do whatever we need to do to wrap up nicely
[/Users/patlu/git/sunet/dnstapir/tem/main.go:57] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
56:
> 57: td.SaveRpzSerial()
58:
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:384] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
383: }
> 384: w.WriteMsg(m)
385: return nil
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:366] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
365: m.Ns = append(m.Ns, dns.RR(&td.Rpz.Axfr.SOA))
> 366: w.WriteMsg(m)
367: }
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:350] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
349: m.Extra = append(m.Extra, glue.RRs...)
> 350: w.WriteMsg(m)
351: }
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:341] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
340: }
> 341: w.WriteMsg(m)
342: return nil
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:313] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
312: }
> 313: w.WriteMsg(m)
314: return nil
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:291] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
290: m.Ns = append(m.Ns, apex.RRtypes[dns.TypeSOA].RRs...)
> 291: w.WriteMsg(m)
292: return nil
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:276] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
275: m.Ns = append(m.Ns, apex.RRtypes[dns.TypeSOA].RRs...)
> 276: w.WriteMsg(m)
277: return nil
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:256] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
255: m.Ns = append(m.Ns, apex.RRtypes[dns.TypeSOA].RRs...)
> 256: w.WriteMsg(m)
257: }
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:226] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
225: }
> 226: w.WriteMsg(m)
227: return nil
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:185] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
184: }
> 185: w.WriteMsg(m)
186: return nil
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:119] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
118: lg.Printf("Found matching full zone for qname %s: %s", qname, zd.ZoneName)
> 119: QueryResponder(w, r, zd, qname, qtype, lg)
120: return
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:115] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
114: m.SetRcode(r, dns.RcodeRefused)
> 115: w.WriteMsg(m)
116: return // didn't find any zone for that qname or found zone, but it is an XFR zone only
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:108] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
107: m.SetRcode(r, dns.RcodeRefused)
> 108: w.WriteMsg(m)
109: return // didn't find any zone for that qname or found zone, but it is an XFR zone only
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:100] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
99: qname, td.Rpz.ZoneName)
> 100: td.QueryResponder(w, r, qname, qtype, lg)
101: return
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:87] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
86: // The qname is equal to the name of a zone we have
> 87: ApexResponder(w, r, zd, qname, qtype, lg)
88: } else {
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:84] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
83: if qname == td.Rpz.ZoneName {
> 84: td.RpzResponder(w, r, qtype, lg)
85: } else if zd, ok := td.RpzSources[qname]; ok {
[/Users/patlu/git/sunet/dnstapir/tem/dnshandler.go:67] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
66: m.SetReply(r)
> 67: w.WriteMsg(m)
68:
[/Users/patlu/git/sunet/dnstapir/tem/apihandler.go:96] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
95: case "mqtt-restart":
> 96: conf.TemData.MqttEngine.RestartEngine()
97: resp.Msg = "MQTT engine restarted"
[/Users/patlu/git/sunet/dnstapir/tem/apihandler.go:92] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
91: case "mqtt-stop":
> 92: conf.TemData.MqttEngine.StopEngine()
93: resp.Msg = "MQTT engine stopped"
[/Users/patlu/git/sunet/dnstapir/tem/apihandler.go:88] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
87: case "mqtt-start":
> 88: conf.TemData.MqttEngine.StartEngine()
89: resp.Msg = "MQTT engine started"
[/Users/patlu/git/sunet/dnstapir/tem/apihandler.go:64] - G104 (CWE-703): Errors unhandled. (Confidence: HIGH, Severity: LOW)
63: w.Header().Set("Content-Type", "application/json")
> 64: json.NewEncoder(w).Encode(resp)
65: }()
Summary:
Gosec : dev
Files : 12
Lines : 3281
Nosec : 0
Issues : 43
```
``` $ golangci-lint run main.go:202:14: Error return value is not checked (errcheck) go DnsEngine(&conf) ^ ```
oej
approved these changes
May 27, 2024
|
All of the added error handling was the result of dealing with |
johanix
reviewed
May 27, 2024
johanix
reviewed
May 27, 2024
johanix
reviewed
May 27, 2024
| @@ -185,7 +186,6 @@ func (td *TemData) GenerateRpzAxfr() error { | |||
| RR: &rr, | |||
| Action: td.Policy.BlacklistAction, | |||
| } | |||
| newaxfrdata = append(newaxfrdata, &rpzn) | |||
There was a problem hiding this comment.
Hmm. Detta måste jag titta på. Jag undrar vad som hänt med newaxfrdata.
johanix
reviewed
May 27, 2024
sources.go
Outdated
There was a problem hiding this comment.
Låt oss inte göra några ändringar i sources.go. Den är rejält omskriven i min gobify-branch, så det skulle bara göra merge mycket jobbigare.
|
This PR is no longer relevant as @johanix have dealt with the linting issues separately. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Comb through code with my standard linting tools
Some error handling can probably be improved but at least we make noise if errors occur now.