From ad52587ae691a40b4b8ecc528eb84b8fe05c729b Mon Sep 17 00:00:00 2001 From: Nick Meves Date: Mon, 20 Jul 2020 18:49:45 -0700 Subject: [PATCH] Document GoSec nosec skip comments --- http.go | 8 ++++---- oauthproxy.go | 2 ++ pkg/authentication/basic/htpasswd.go | 3 +++ pkg/upstream/http.go | 1 + pkg/util/util.go | 1 + pkg/validation/options.go | 1 + providers/logingov.go | 29 ++++------------------------ validator.go | 6 ++++-- 8 files changed, 20 insertions(+), 31 deletions(-) diff --git a/http.go b/http.go index 024c617b..41198de4 100644 --- a/http.go +++ b/http.go @@ -119,18 +119,18 @@ type tcpKeepAliveListener struct { *net.TCPListener } -func (ln tcpKeepAliveListener) Accept() (c net.Conn, err error) { +func (ln tcpKeepAliveListener) Accept() (net.Conn, error) { tc, err := ln.AcceptTCP() if err != nil { - return + return nil, err } err = tc.SetKeepAlive(true) if err != nil { - return nil, err + logger.Printf("Error setting Keep-Alive: %v", err) } err = tc.SetKeepAlivePeriod(3 * time.Minute) if err != nil { - return nil, err + logger.Printf("Error setting Keep-Alive period: %v", err) } return tc, nil } diff --git a/oauthproxy.go b/oauthproxy.go index 58c8f0be..4e733a8c 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -413,6 +413,8 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code redirectURL = "/" } + // We allow unescaped template.HTML since it is user configured options + /* #nosec G203 */ t := struct { ProviderName string SignInMessage template.HTML diff --git a/pkg/authentication/basic/htpasswd.go b/pkg/authentication/basic/htpasswd.go index f4f46813..321b7bae 100644 --- a/pkg/authentication/basic/htpasswd.go +++ b/pkg/authentication/basic/htpasswd.go @@ -1,6 +1,7 @@ package basic import ( + // We support SHA1 & bcrypt in HTPasswd "crypto/sha1" // #nosec G505 "encoding/base64" "encoding/csv" @@ -29,6 +30,7 @@ type sha1Pass string // NewHTPasswdValidator constructs an httpasswd based validator from the file // at the path given. func NewHTPasswdValidator(path string) (Validator, error) { + // We allow HTPasswd location via config options r, err := os.Open(path) // #nosec G304 if err != nil { return nil, fmt.Errorf("could not open htpasswd file: %v", err) @@ -90,6 +92,7 @@ func (h *htpasswdMap) Validate(user string, password string) bool { switch rp := realPassword.(type) { case sha1Pass: + // We support SHA1 HTPasswd entries d := sha1.New() // #nosec G401 _, err := d.Write([]byte(password)) if err != nil { diff --git a/pkg/upstream/http.go b/pkg/upstream/http.go index e18d0c77..833b1399 100644 --- a/pkg/upstream/http.go +++ b/pkg/upstream/http.go @@ -103,6 +103,7 @@ func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler Pr proxy.FlushInterval = 1 * time.Second } + // InsecureSkipVerify is a configurable option we allow /* #nosec G402 */ if upstream.InsecureSkipTLSVerify { proxy.Transport = &http.Transport{ diff --git a/pkg/util/util.go b/pkg/util/util.go index 0f68ca70..4519fdb8 100644 --- a/pkg/util/util.go +++ b/pkg/util/util.go @@ -12,6 +12,7 @@ func GetCertPool(paths []string) (*x509.CertPool, error) { } pool := x509.NewCertPool() for _, path := range paths { + // Cert paths are a configurable option data, err := ioutil.ReadFile(path) // #nosec G304 if err != nil { return nil, fmt.Errorf("certificate authority file (%s) could not be read - %s", path, err) diff --git a/pkg/validation/options.go b/pkg/validation/options.go index 741c81df..86197c06 100644 --- a/pkg/validation/options.go +++ b/pkg/validation/options.go @@ -30,6 +30,7 @@ func Validate(o *options.Options) error { msgs = append(msgs, validateSessionCookieMinimal(o)...) if o.SSLInsecureSkipVerify { + // InsecureSkipVerify is a configurable option we allow /* #nosec G402 */ insecureTransport := &http.Transport{ TLSClientConfig: &tls.Config{InsecureSkipVerify: true}, diff --git a/providers/logingov.go b/providers/logingov.go index b1a62c58..c524741f 100644 --- a/providers/logingov.go +++ b/providers/logingov.go @@ -4,12 +4,9 @@ import ( "bytes" "context" "crypto/rsa" - "encoding/json" "errors" "fmt" - "io/ioutil" "math/rand" - "net/http" "net/url" "time" @@ -106,30 +103,12 @@ type loginGovCustomClaims struct { // checkNonce checks the nonce in the id_token func checkNonce(idToken string, p *LoginGovProvider) (err error) { token, err := jwt.ParseWithClaims(idToken, &loginGovCustomClaims{}, func(token *jwt.Token) (interface{}, error) { - resp, myerr := http.Get(p.PubJWKURL.String()) - if myerr != nil { - return nil, myerr - } - if resp.StatusCode != 200 { - myerr = fmt.Errorf("got %d from %q", resp.StatusCode, p.PubJWKURL.String()) - return nil, myerr - } - body, myerr := ioutil.ReadAll(resp.Body) - if myerr != nil { - return nil, myerr - } - if myerr = resp.Body.Close(); myerr != nil { - return nil, myerr - } - var pubkeys jose.JSONWebKeySet - myerr = json.Unmarshal(body, &pubkeys) - if myerr != nil { - return nil, myerr + rerr := requests.New(p.PubJWKURL.String()).Do().UnmarshalInto(&pubkeys) + if rerr != nil { + return nil, rerr } - pubkey := pubkeys.Keys[0] - - return pubkey.Key, nil + return pubkeys.Keys[0].Key, nil }) if err != nil { return diff --git a/validator.go b/validator.go index e45543af..9db34a27 100644 --- a/validator.go +++ b/validator.go @@ -19,10 +19,12 @@ type UserMap struct { } // NewUserMap parses the authenticated emails file into a new UserMap +// +// TODO (@NickMeves): Audit usage of `unsafe.Pointer` and potentially refactor func NewUserMap(usersFile string, done <-chan bool, onUpdate func()) *UserMap { um := &UserMap{usersFile: usersFile} m := make(map[string]bool) - atomic.StorePointer(&um.m, unsafe.Pointer(&m)) + atomic.StorePointer(&um.m, unsafe.Pointer(&m)) // #nosec G103 if usersFile != "" { logger.Printf("using authenticated emails file %s", usersFile) WatchForUpdates(usersFile, done, func() { @@ -68,7 +70,7 @@ func (um *UserMap) LoadAuthenticatedEmailsFile() { address := strings.ToLower(strings.TrimSpace(r[0])) updated[address] = true } - atomic.StorePointer(&um.m, unsafe.Pointer(&updated)) + atomic.StorePointer(&um.m, unsafe.Pointer(&updated)) // #nosec G103 } func newValidatorImpl(domains []string, usersFile string,