From 7d8ee6125421e2cf033f6d6c89ea59e6fd061ccf Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sat, 18 Jul 2020 00:41:42 +0100 Subject: [PATCH 1/3] Add HTPasswdValidator to basic authentication package --- pkg/authentication/basic/basic_suite_test.go | 16 ++++ pkg/authentication/basic/htpasswd.go | 96 +++++++++++++++++++ pkg/authentication/basic/htpasswd_test.go | 96 +++++++++++++++++++ .../basic/test/htpasswd-bcrypt.txt | 8 ++ .../basic/test/htpasswd-mixed.txt | 8 ++ .../basic/test/htpasswd-sha1.txt | 8 ++ pkg/authentication/basic/validator.go | 7 ++ 7 files changed, 239 insertions(+) create mode 100644 pkg/authentication/basic/basic_suite_test.go create mode 100644 pkg/authentication/basic/htpasswd.go create mode 100644 pkg/authentication/basic/htpasswd_test.go create mode 100644 pkg/authentication/basic/test/htpasswd-bcrypt.txt create mode 100644 pkg/authentication/basic/test/htpasswd-mixed.txt create mode 100644 pkg/authentication/basic/test/htpasswd-sha1.txt create mode 100644 pkg/authentication/basic/validator.go diff --git a/pkg/authentication/basic/basic_suite_test.go b/pkg/authentication/basic/basic_suite_test.go new file mode 100644 index 00000000..4d5fa806 --- /dev/null +++ b/pkg/authentication/basic/basic_suite_test.go @@ -0,0 +1,16 @@ +package basic + +import ( + "testing" + + "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +func TestBasicSuite(t *testing.T) { + logger.SetOutput(GinkgoWriter) + + RegisterFailHandler(Fail) + RunSpecs(t, "Basic") +} diff --git a/pkg/authentication/basic/htpasswd.go b/pkg/authentication/basic/htpasswd.go new file mode 100644 index 00000000..ff87894f --- /dev/null +++ b/pkg/authentication/basic/htpasswd.go @@ -0,0 +1,96 @@ +package basic + +import ( + "crypto/sha1" + "encoding/base64" + "encoding/csv" + "fmt" + "io" + "os" + + "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" + "golang.org/x/crypto/bcrypt" +) + +// htpasswdMap represents the structure of an htpasswd file. +// Passwords must be generated with -B for bcrypt or -s for SHA1. +type htpasswdMap struct { + users map[string]interface{} +} + +// bcryptPass is used to identify bcrypt passwords in the +// htpasswdMap users. +type bcryptPass string + +// sha1Pass os used to identify sha1 passwords in the +// htpasswdMap users. +type sha1Pass string + +// NewHTPasswdValidator constructs an httpasswd based validator from the file +// at the path given. +func NewHTPasswdValidator(path string) (Validator, error) { + r, err := os.Open(path) + if err != nil { + return nil, fmt.Errorf("could not open htpasswd file: %v", err) + } + defer r.Close() + return newHtpasswd(r) +} + +// newHtpasswd consctructs an htpasswd from an io.Reader (an opened file). +func newHtpasswd(file io.Reader) (*htpasswdMap, error) { + csvReader := csv.NewReader(file) + csvReader.Comma = ':' + csvReader.Comment = '#' + csvReader.TrimLeadingSpace = true + + records, err := csvReader.ReadAll() + if err != nil { + return nil, fmt.Errorf("could not read htpasswd file: %v", err) + } + + return createHtpasswdMap(records) +} + +// createHtasswdMap constructs an htpasswdMap from the given records +func createHtpasswdMap(records [][]string) (*htpasswdMap, error) { + h := &htpasswdMap{users: make(map[string]interface{})} + for _, record := range records { + user, realPassword := record[0], record[1] + shaPrefix := realPassword[:5] + if shaPrefix == "{SHA}" { + h.users[user] = sha1Pass(realPassword[5:]) + continue + } + + bcryptPrefix := realPassword[:4] + if bcryptPrefix == "$2a$" || bcryptPrefix == "$2b$" || bcryptPrefix == "$2x$" || bcryptPrefix == "$2y$" { + h.users[user] = bcryptPass(realPassword) + continue + } + + // Password is neither sha1 or bcrypt + // TODO(JoelSpeed): In the next breaking release, make this return an error. + logger.Printf("Invalid htpasswd entry for %s. Must be a SHA or bcrypt entry.", user) + } + return h, nil +} + +// Validate checks a users password against the htpasswd entries +func (h *htpasswdMap) Validate(user string, password string) bool { + realPassword, exists := h.users[user] + if !exists { + return false + } + + switch real := realPassword.(type) { + case sha1Pass: + d := sha1.New() + d.Write([]byte(password)) + return string(real) == base64.StdEncoding.EncodeToString(d.Sum(nil)) + case bcryptPass: + return bcrypt.CompareHashAndPassword([]byte(real), []byte(password)) == nil + default: + return false + } +} diff --git a/pkg/authentication/basic/htpasswd_test.go b/pkg/authentication/basic/htpasswd_test.go new file mode 100644 index 00000000..0c2a197e --- /dev/null +++ b/pkg/authentication/basic/htpasswd_test.go @@ -0,0 +1,96 @@ +package basic + +import ( + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" +) + +const ( + adminUser = "admin" + adminPassword = "Adm1n1str$t0r" + user1 = "user1" + user1Password = "UsErOn3P455" + user2 = "user2" + user2Password = "us3r2P455W0Rd!" +) + +var _ = Describe("HTPasswd Suite", func() { + Context("with an HTPassword Validator", func() { + assertHtpasswdMapFromFile := func(filePath string) { + var htpasswd *htpasswdMap + var err error + + BeforeEach(func() { + var validator Validator + validator, err = NewHTPasswdValidator(filePath) + + var ok bool + htpasswd, ok = validator.(*htpasswdMap) + Expect(ok).To(BeTrue()) + }) + + It("does not return an error", func() { + Expect(err).ToNot(HaveOccurred()) + }) + + It("has the correct number of users", func() { + Expect(htpasswd.users).To(HaveLen(3)) + }) + + It("accepts the correct passwords", func() { + Expect(htpasswd.Validate(adminUser, adminPassword)).To(BeTrue()) + Expect(htpasswd.Validate(user1, user1Password)).To(BeTrue()) + Expect(htpasswd.Validate(user2, user2Password)).To(BeTrue()) + }) + + It("rejects incorrect passwords", func() { + Expect(htpasswd.Validate(adminUser, "asvdfda")).To(BeFalse()) + Expect(htpasswd.Validate(user1, "BHEdgbtr")).To(BeFalse()) + Expect(htpasswd.Validate(user2, "12345")).To(BeFalse()) + }) + + It("rejects a non existent user", func() { + // Users are case sensitive + Expect(htpasswd.Validate("ADMIN", adminPassword)).To(BeFalse()) + }) + } + + Context("load from file", func() { + Context("with sha1 entries", func() { + const filePath = "./test/htpasswd-sha1.txt" + + assertHtpasswdMapFromFile(filePath) + }) + + Context("with bcrypt entries", func() { + const filePath = "./test/htpasswd-bcrypt.txt" + + assertHtpasswdMapFromFile(filePath) + }) + + Context("with mixed entries", func() { + const filePath = "./test/htpasswd-mixed.txt" + + assertHtpasswdMapFromFile(filePath) + }) + + Context("with a non existent file", func() { + const filePath = "./test/htpasswd-doesnt-exist.txt" + var validator Validator + var err error + + BeforeEach(func() { + validator, err = NewHTPasswdValidator(filePath) + }) + + It("returns an error", func() { + Expect(err).To(MatchError("could not open htpasswd file: open ./test/htpasswd-doesnt-exist.txt: no such file or directory")) + }) + + It("returns a nil validator", func() { + Expect(validator).To(BeNil()) + }) + }) + }) + }) +}) diff --git a/pkg/authentication/basic/test/htpasswd-bcrypt.txt b/pkg/authentication/basic/test/htpasswd-bcrypt.txt new file mode 100644 index 00000000..5fcaa381 --- /dev/null +++ b/pkg/authentication/basic/test/htpasswd-bcrypt.txt @@ -0,0 +1,8 @@ +# admin:Adm1n1str$t0r +admin:$2y$05$SXWrNM7ldtbRzBvUC3VXyOvUeiUcP45XPwM93P5eeGOEPIiAZmJjC + +# user1:UsErOn3P455 +user1:$2y$05$/sZYJOk8.3Etg4V6fV7puuXfCJLmV5Q7u3xvKpjBSJUka.t2YtmmG + +# user2: us3r2P455W0Rd! +user2:$2y$05$l22MubgKTZFTjTs8TNg5k.YKvcnM2.bA/.iwl0idef5CbekdvBxva diff --git a/pkg/authentication/basic/test/htpasswd-mixed.txt b/pkg/authentication/basic/test/htpasswd-mixed.txt new file mode 100644 index 00000000..fd66c575 --- /dev/null +++ b/pkg/authentication/basic/test/htpasswd-mixed.txt @@ -0,0 +1,8 @@ +# admin:Adm1n1str$t0r +admin:$2y$05$SXWrNM7ldtbRzBvUC3VXyOvUeiUcP45XPwM93P5eeGOEPIiAZmJjC + +# user1:UsErOn3P455 +user1:{SHA}Dvs/L78raajL4jEAHPkwflQXJzI= + +# user2: us3r2P455W0Rd! +user2:{SHA}MoN9/JCJEcYUb6GCQ+2buDvn9pI= diff --git a/pkg/authentication/basic/test/htpasswd-sha1.txt b/pkg/authentication/basic/test/htpasswd-sha1.txt new file mode 100644 index 00000000..beb1f0d2 --- /dev/null +++ b/pkg/authentication/basic/test/htpasswd-sha1.txt @@ -0,0 +1,8 @@ +# admin:Adm1n1str$t0r +admin:{SHA}gXQeRH0bcaCfhAk2gOLm1uaePMA= + +# user1:UsErOn3P455 +user1:{SHA}Dvs/L78raajL4jEAHPkwflQXJzI= + +# user2: us3r2P455W0Rd! +user2:{SHA}MoN9/JCJEcYUb6GCQ+2buDvn9pI= diff --git a/pkg/authentication/basic/validator.go b/pkg/authentication/basic/validator.go new file mode 100644 index 00000000..7a1ceda9 --- /dev/null +++ b/pkg/authentication/basic/validator.go @@ -0,0 +1,7 @@ +package basic + +// Validator is a minimal interface for something that can validate a +// username and password combination. +type Validator interface { + Validate(user, password string) bool +} From 2981a5ed1a3884ad7e3167ff8548441010c0681c Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sat, 18 Jul 2020 10:18:47 +0100 Subject: [PATCH 2/3] Integrate HTPasswdValidator into OAuth2 Proxy --- htpasswd.go | 72 ------------------------------------------------ htpasswd_test.go | 38 ------------------------- main.go | 9 ------ oauthproxy.go | 32 +++++++++++++-------- 4 files changed, 21 insertions(+), 130 deletions(-) delete mode 100644 htpasswd.go delete mode 100644 htpasswd_test.go diff --git a/htpasswd.go b/htpasswd.go deleted file mode 100644 index 670aa729..00000000 --- a/htpasswd.go +++ /dev/null @@ -1,72 +0,0 @@ -package main - -import ( - "crypto/sha1" - "encoding/base64" - "encoding/csv" - "io" - "os" - - "github.com/oauth2-proxy/oauth2-proxy/pkg/logger" - "golang.org/x/crypto/bcrypt" -) - -// Lookup passwords in a htpasswd file -// Passwords must be generated with -B for bcrypt or -s for SHA1. - -// HtpasswdFile represents the structure of an htpasswd file -type HtpasswdFile struct { - Users map[string]string -} - -// NewHtpasswdFromFile constructs an HtpasswdFile from the file at the path given -func NewHtpasswdFromFile(path string) (*HtpasswdFile, error) { - r, err := os.Open(path) - if err != nil { - return nil, err - } - defer r.Close() - return NewHtpasswd(r) -} - -// NewHtpasswd consctructs an HtpasswdFile from an io.Reader (opened file) -func NewHtpasswd(file io.Reader) (*HtpasswdFile, error) { - csvReader := csv.NewReader(file) - csvReader.Comma = ':' - csvReader.Comment = '#' - csvReader.TrimLeadingSpace = true - - records, err := csvReader.ReadAll() - if err != nil { - return nil, err - } - h := &HtpasswdFile{Users: make(map[string]string)} - for _, record := range records { - h.Users[record[0]] = record[1] - } - return h, nil -} - -// Validate checks a users password against the HtpasswdFile entries -func (h *HtpasswdFile) Validate(user string, password string) bool { - realPassword, exists := h.Users[user] - if !exists { - return false - } - - shaPrefix := realPassword[:5] - if shaPrefix == "{SHA}" { - shaValue := realPassword[5:] - d := sha1.New() - d.Write([]byte(password)) - return shaValue == base64.StdEncoding.EncodeToString(d.Sum(nil)) - } - - bcryptPrefix := realPassword[:4] - if bcryptPrefix == "$2a$" || bcryptPrefix == "$2b$" || bcryptPrefix == "$2x$" || bcryptPrefix == "$2y$" { - return bcrypt.CompareHashAndPassword([]byte(realPassword), []byte(password)) == nil - } - - logger.Printf("Invalid htpasswd entry for %s. Must be a SHA or bcrypt entry.", user) - return false -} diff --git a/htpasswd_test.go b/htpasswd_test.go deleted file mode 100644 index 7a043e46..00000000 --- a/htpasswd_test.go +++ /dev/null @@ -1,38 +0,0 @@ -package main - -import ( - "bytes" - "fmt" - "testing" - - "github.com/stretchr/testify/assert" - "golang.org/x/crypto/bcrypt" -) - -func TestSHA(t *testing.T) { - file := bytes.NewBuffer([]byte("testuser:{SHA}PaVBVZkYqAjCQCu6UBL2xgsnZhw=\n")) - h, err := NewHtpasswd(file) - assert.Equal(t, err, nil) - - valid := h.Validate("testuser", "asdf") - assert.Equal(t, valid, true) -} - -func TestBcrypt(t *testing.T) { - hash1, err := bcrypt.GenerateFromPassword([]byte("password"), 1) - assert.Equal(t, err, nil) - hash2, err := bcrypt.GenerateFromPassword([]byte("top-secret"), 2) - assert.Equal(t, err, nil) - - contents := fmt.Sprintf("testuser1:%s\ntestuser2:%s\n", hash1, hash2) - file := bytes.NewBuffer([]byte(contents)) - - h, err := NewHtpasswd(file) - assert.Equal(t, err, nil) - - valid := h.Validate("testuser1", "password") - assert.Equal(t, valid, true) - - valid = h.Validate("testuser2", "top-secret") - assert.Equal(t, valid, true) -} diff --git a/main.go b/main.go index 4ee6af4b..b42c2c05 100644 --- a/main.go +++ b/main.go @@ -66,15 +66,6 @@ func main() { } } - if opts.HtpasswdFile != "" { - logger.Printf("using htpasswd file %s", opts.HtpasswdFile) - oauthproxy.HtpasswdFile, err = NewHtpasswdFromFile(opts.HtpasswdFile) - oauthproxy.DisplayHtpasswdForm = opts.DisplayHtpasswdForm - if err != nil { - logger.Fatalf("FATAL: unable to open %s %s", opts.HtpasswdFile, err) - } - } - rand.Seed(time.Now().UnixNano()) chain := alice.New() diff --git a/oauthproxy.go b/oauthproxy.go index c05c793d..4b310b9b 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -22,6 +22,7 @@ import ( ipapi "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/ip" "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options" sessionsapi "github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions" + "github.com/oauth2-proxy/oauth2-proxy/pkg/authentication/basic" "github.com/oauth2-proxy/oauth2-proxy/pkg/cookies" "github.com/oauth2-proxy/oauth2-proxy/pkg/encryption" "github.com/oauth2-proxy/oauth2-proxy/pkg/ip" @@ -96,8 +97,8 @@ type OAuthProxy struct { sessionStore sessionsapi.SessionStore ProxyPrefix string SignInMessage string - HtpasswdFile *HtpasswdFile - DisplayHtpasswdForm bool + basicAuthValidator basic.Validator + displayHtpasswdForm bool serveMux http.Handler SetXAuthRequest bool PassBasicAuth bool @@ -314,6 +315,16 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr } } + var basicAuthValidator basic.Validator + if opts.HtpasswdFile != "" { + logger.Printf("using htpasswd file: %s", opts.HtpasswdFile) + var err error + basicAuthValidator, err = basic.NewHTPasswdValidator(opts.HtpasswdFile) + if err != nil { + return nil, fmt.Errorf("could not load htpasswdfile: %v", err) + } + } + return &OAuthProxy{ CookieName: opts.Cookie.Name, CSRFCookieName: fmt.Sprintf("%v_%v", opts.Cookie.Name, "csrf"), @@ -364,6 +375,9 @@ func NewOAuthProxy(opts *options.Options, validator func(string) bool) (*OAuthPr trustedIPs: trustedIPs, Banner: opts.Banner, Footer: opts.Footer, + + basicAuthValidator: basicAuthValidator, + displayHtpasswdForm: basicAuthValidator != nil, }, nil } @@ -386,10 +400,6 @@ func (p *OAuthProxy) GetRedirectURI(host string) string { return u.String() } -func (p *OAuthProxy) displayCustomLoginForm() bool { - return p.HtpasswdFile != nil && p.DisplayHtpasswdForm -} - func (p *OAuthProxy) redeemCode(ctx context.Context, host, code string) (s *sessionsapi.SessionState, err error) { if code == "" { return nil, errors.New("missing code") @@ -526,7 +536,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code }{ ProviderName: p.provider.Data().ProviderName, SignInMessage: template.HTML(p.SignInMessage), - CustomLogin: p.displayCustomLoginForm(), + CustomLogin: p.displayHtpasswdForm, Redirect: redirectURL, Version: VERSION, ProxyPrefix: p.ProxyPrefix, @@ -540,7 +550,7 @@ func (p *OAuthProxy) SignInPage(rw http.ResponseWriter, req *http.Request, code // ManualSignIn handles basic auth logins to the proxy func (p *OAuthProxy) ManualSignIn(rw http.ResponseWriter, req *http.Request) (string, bool) { - if req.Method != "POST" || p.HtpasswdFile == nil { + if req.Method != "POST" || p.basicAuthValidator == nil { return "", false } user := req.FormValue("username") @@ -549,7 +559,7 @@ func (p *OAuthProxy) ManualSignIn(rw http.ResponseWriter, req *http.Request) (st return "", false } // check auth - if p.HtpasswdFile.Validate(user, passwd) { + if p.basicAuthValidator.Validate(user, passwd) { logger.PrintAuthf(user, req, logger.AuthSuccess, "Authenticated via HtpasswdFile") return user, true } @@ -1159,7 +1169,7 @@ func (p *OAuthProxy) stripAuthHeaders(req *http.Request) { // CheckBasicAuth checks the requests Authorization header for basic auth // credentials and authenticates these against the proxies HtpasswdFile func (p *OAuthProxy) CheckBasicAuth(req *http.Request) (*sessionsapi.SessionState, error) { - if p.HtpasswdFile == nil { + if p.basicAuthValidator == nil { return nil, nil } auth := req.Header.Get("Authorization") @@ -1178,7 +1188,7 @@ func (p *OAuthProxy) CheckBasicAuth(req *http.Request) (*sessionsapi.SessionStat if len(pair) != 2 { return nil, fmt.Errorf("invalid format %s", b) } - if p.HtpasswdFile.Validate(pair[0], pair[1]) { + if p.basicAuthValidator.Validate(pair[0], pair[1]) { logger.PrintAuthf(pair[0], req, logger.AuthSuccess, "Authenticated via basic auth and HTpasswd File") return &sessionsapi.SessionState{User: pair[0]}, nil } From e73db7df7bf3ec99ac2c88bcd00b864fb69eba4a Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sat, 18 Jul 2020 10:23:50 +0100 Subject: [PATCH 3/3] Add HTPasswd validator refactor to changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62344892..ad30c6ae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ ## Changes since v6.0.0 +- [#687](https://github.com/oauth2-proxy/oauth2-proxy/pull/687) Refactor HTPasswd Validator (@JoelSpeed) - [#624](https://github.com/oauth2-proxy/oauth2-proxy/pull/624) Allow stripping authentication headers from whitelisted requests with `--skip-auth-strip-headers` (@NickMeves) - [#673](https://github.com/oauth2-proxy/oauth2-proxy/pull/673) Add --session-cookie-minimal option to create session cookies with no tokens (@NickMeves) - [#632](https://github.com/oauth2-proxy/oauth2-proxy/pull/632) Reduce session size by encoding with MessagePack and using LZ4 compression (@NickMeves)