ad52587 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Document GoSec nosec skip comments

commitad52587ae691a40b4b8ecc528eb84b8fe05c729b authorNick Meves <nick.meves@greenhouse.io> dateMon 20 Jul 2020, 18:49 (6 years ago) parent2bb0160

8 files changed+20−31

compare with master

unifiedsplit

http.go+4−4 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+4−25 validator.go+4−2

http.go+4−4

@@ -119,18 +119,18 @@ type tcpKeepAliveListener struct {

119119 *net.TCPListener

120120}

121121

122−func (ln tcpKeepAliveListener) Accept() (c net.Conn, err error) {

122+func (ln tcpKeepAliveListener) Accept() (net.Conn, error) {

123123 tc, err := ln.AcceptTCP()

124124 if err != nil {

125− return

125+ return nil, err

126126 }

127127 err = tc.SetKeepAlive(true)

128128 if err != nil {

129− return nil, err

129+ logger.Printf("Error setting Keep-Alive: %v", err)

130130 }

131131 err = tc.SetKeepAlivePeriod(3 * time.Minute)

132132 if err != nil {

133− return nil, err

133+ logger.Printf("Error setting Keep-Alive period: %v", err)

134134 }

135135 return tc, nil

136136}

oauthproxy.go+2

@@ -413,6 +413,8 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code

413413 redirectURL = "/"

414414 }

415415

416+ // We allow unescaped template.HTML since it is user configured options

417+ /* #nosec G203 */

416418 t := struct {

417419 ProviderName string

418420 SignInMessage template.HTML

pkg/authentication/basic/htpasswd.go+3

@@ -1,6 +1,7 @@

11package basic

33import (

4+ // We support SHA1 & bcrypt in HTPasswd

45 "crypto/sha1" // #nosec G505

56 "encoding/base64"

67 "encoding/csv"

@@ -29,6 +30,7 @@ type sha1Pass string

2930// NewHTPasswdValidator constructs an httpasswd based validator from the file

3031// at the path given.

3132func NewHTPasswdValidator(path string) (Validator, error) {

33+ // We allow HTPasswd location via config options

3234 r, err := os.Open(path) // #nosec G304

3335 if err != nil {

3436 return nil, fmt.Errorf("could not open htpasswd file: %v", err)

@@ -90,6 +92,7 @@ func (h *htpasswdMap) Validate(user string, password string) bool {

9092

9193 switch rp := realPassword.(type) {

9294 case sha1Pass:

95+ // We support SHA1 HTPasswd entries

9396 d := sha1.New() // #nosec G401

9497 _, err := d.Write([]byte(password))

9598 if err != nil {

pkg/upstream/http.go+1

@@ -103,6 +103,7 @@ func newReverseProxy(target *url.URL, upstream options.Upstream, errorHandler Pr

103103 proxy.FlushInterval = 1 * time.Second

104104 }

105105

106+ // InsecureSkipVerify is a configurable option we allow

106107 /* #nosec G402 */

107108 if upstream.InsecureSkipTLSVerify {

108109 proxy.Transport = &http.Transport{

pkg/util/util.go+1

@@ -12,6 +12,7 @@ func GetCertPool(paths []string) (*x509.CertPool, error) {

1212 }

1313 pool := x509.NewCertPool()

1414 for _, path := range paths {

15+ // Cert paths are a configurable option

1516 data, err := ioutil.ReadFile(path) // #nosec G304

1617 if err != nil {

1718 return nil, fmt.Errorf("certificate authority file (%s) could not be read - %s", path, err)

pkg/validation/options.go+1

@@ -30,6 +30,7 @@ func Validate(o *options.Options) error {

3030 msgs = append(msgs, validateSessionCookieMinimal(o)...)

3131

3232 if o.SSLInsecureSkipVerify {

33+ // InsecureSkipVerify is a configurable option we allow

3334 /* #nosec G402 */

3435 insecureTransport := &http.Transport{

3536 TLSClientConfig: &tls.Config{InsecureSkipVerify: true},

providers/logingov.go+4−25

@@ -4,12 +4,9 @@ import (

44 "bytes"

55 "context"

66 "crypto/rsa"

7− "encoding/json"

87 "errors"

98 "fmt"

10− "io/ioutil"

119 "math/rand"

12− "net/http"

1310 "net/url"

1411 "time"

1512

@@ -106,30 +103,12 @@ type loginGovCustomClaims struct {

106103// checkNonce checks the nonce in the id_token

107104func checkNonce(idToken string, p *LoginGovProvider) (err error) {

108105 token, err := jwt.ParseWithClaims(idToken, &loginGovCustomClaims{}, func(token *jwt.Token) (interface{}, error) {

109− resp, myerr := http.Get(p.PubJWKURL.String())

110− if myerr != nil {

111− return nil, myerr

112− }

113− if resp.StatusCode != 200 {

114− myerr = fmt.Errorf("got %d from %q", resp.StatusCode, p.PubJWKURL.String())

115− return nil, myerr

116− }

117− body, myerr := ioutil.ReadAll(resp.Body)

118− if myerr != nil {

119− return nil, myerr

120− }

121− if myerr = resp.Body.Close(); myerr != nil {

122− return nil, myerr

123− }

124−

125106 var pubkeys jose.JSONWebKeySet

126− myerr = json.Unmarshal(body, &pubkeys)

127− if myerr != nil {

128− return nil, myerr

107+ rerr := requests.New(p.PubJWKURL.String()).Do().UnmarshalInto(&pubkeys)

108+ if rerr != nil {

109+ return nil, rerr

129110 }

130− pubkey := pubkeys.Keys[0]

131−

132− return pubkey.Key, nil

111+ return pubkeys.Keys[0].Key, nil

133112 })

134113 if err != nil {

135114 return

validator.go+4−2

@@ -19,10 +19,12 @@ type UserMap struct {

1919}

2020

2121// NewUserMap parses the authenticated emails file into a new UserMap

22+//

23+// TODO (@NickMeves): Audit usage of `unsafe.Pointer` and potentially refactor

2224func NewUserMap(usersFile string, done <-chan bool, onUpdate func()) *UserMap {

2325 um := &UserMap{usersFile: usersFile}

2426 m := make(map[string]bool)

25− atomic.StorePointer(&um.m, unsafe.Pointer(&m))

27+ atomic.StorePointer(&um.m, unsafe.Pointer(&m)) // #nosec G103

2628 if usersFile != "" {

2729 logger.Printf("using authenticated emails file %s", usersFile)

2830 WatchForUpdates(usersFile, done, func() {

@@ -68,7 +70,7 @@ func (um *UserMap) LoadAuthenticatedEmailsFile() {

6870 address := strings.ToLower(strings.TrimSpace(r[0]))

6971 updated[address] = true

7072 }

71− atomic.StorePointer(&um.m, unsafe.Pointer(&updated))

73+ atomic.StorePointer(&um.m, unsafe.Pointer(&updated)) // #nosec G103

7274}

7375

7476func newValidatorImpl(domains []string, usersFile string,