From 60262521bd2122e1b98554587891d88baa557d29 Mon Sep 17 00:00:00 2001 From: BadZen Date: Tue, 21 Apr 2015 19:57:12 +0000 Subject: [PATCH 01/11] Implementation of a basic authentication scheme using standard .htpasswd files Signed-off-by: BadZen Signed-off-by: Dave Trombley --- docs/auth/basic/access.go | 112 +++++++++++++++++++++++++++++++++ docs/auth/basic/access_test.go | 100 +++++++++++++++++++++++++++++ docs/auth/basic/htpasswd.go | 49 +++++++++++++++ 3 files changed, 261 insertions(+) create mode 100644 docs/auth/basic/access.go create mode 100644 docs/auth/basic/access_test.go create mode 100644 docs/auth/basic/htpasswd.go diff --git a/docs/auth/basic/access.go b/docs/auth/basic/access.go new file mode 100644 index 00000000..1833296a --- /dev/null +++ b/docs/auth/basic/access.go @@ -0,0 +1,112 @@ +// Package basic provides a simple authentication scheme that checks for the +// user credential hash in an htpasswd formatted file in a configuration-determined +// location. +// +// The use of SHA hashes (htpasswd -s) is enforced since MD5 is insecure and simple +// system crypt() may be as well. +// +// This authentication method MUST be used under TLS, as simple token-replay attack is possible. + +package basic + +import ( + "encoding/base64" + "errors" + "fmt" + "net/http" + "strings" + + ctxu "github.com/docker/distribution/context" + "github.com/docker/distribution/registry/auth" + "golang.org/x/net/context" +) + +type accessController struct { + realm string + htpasswd *HTPasswd +} + +type challenge struct { + realm string + err error +} + +var _ auth.AccessController = &accessController{} +var ( + ErrPasswordRequired = errors.New("authorization credential required") + ErrInvalidCredential = errors.New("invalid authorization credential") +) + +func newAccessController(options map[string]interface{}) (auth.AccessController, error) { + realm, present := options["realm"] + if _, ok := realm.(string); !present || !ok { + return nil, fmt.Errorf(`"realm" must be set for basic access controller`) + } + + path, present := options["path"] + if _, ok := path.(string); !present || !ok { + return nil, fmt.Errorf(`"path" must be set for basic access controller`) + } + + return &accessController{realm: realm.(string), htpasswd: NewHTPasswd(path.(string))}, nil +} + +func (ac *accessController) Authorized(ctx context.Context, accessRecords ...auth.Access) (context.Context, error) { + req, err := ctxu.GetRequest(ctx) + if err != nil { + return nil, err + } + + authHeader := req.Header.Get("Authorization") + + if authHeader == "" { + challenge := challenge{ + realm: ac.realm, + } + return nil, &challenge + } + + parts := strings.Split(req.Header.Get("Authorization"), " ") + + challenge := challenge{ + realm: ac.realm, + } + + if len(parts) != 2 || strings.ToLower(parts[0]) != "basic" { + challenge.err = ErrPasswordRequired + return nil, &challenge + } + + text, err := base64.StdEncoding.DecodeString(parts[1]) + if err != nil { + challenge.err = ErrInvalidCredential + return nil, &challenge + } + + credential := strings.Split(string(text), ":") + if len(credential) != 2 { + challenge.err = ErrInvalidCredential + return nil, &challenge + } + + if res, _ := ac.htpasswd.AuthenticateUser(credential[0], credential[1]); !res { + challenge.err = ErrInvalidCredential + return nil, &challenge + } + + return auth.WithUser(ctx, auth.UserInfo{Name: credential[0]}), nil +} + +func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) { + header := fmt.Sprintf("Realm realm=%q", ch.realm) + w.Header().Set("WWW-Authenticate", header) + w.WriteHeader(http.StatusUnauthorized) +} + +func (ch *challenge) Error() string { + return fmt.Sprintf("basic authentication challenge: %#v", ch) +} + +func init() { + auth.Register("basic", auth.InitFunc(newAccessController)) +} diff --git a/docs/auth/basic/access_test.go b/docs/auth/basic/access_test.go new file mode 100644 index 00000000..d82573b9 --- /dev/null +++ b/docs/auth/basic/access_test.go @@ -0,0 +1,100 @@ +package basic + +import ( + "encoding/base64" + "io/ioutil" + "net/http" + "net/http/httptest" + "testing" + + "github.com/docker/distribution/registry/auth" + "golang.org/x/net/context" +) + +func TestBasicAccessController(t *testing.T) { + + testRealm := "The-Shire" + testUser := "bilbo" + testHtpasswdContent := "bilbo:{SHA}5siv5c0SHx681xU6GiSx9ZQryqs=" + + tempFile, err := ioutil.TempFile("", "htpasswd-test") + if err != nil { + t.Fatal("could not create temporary htpasswd file") + } + if _, err = tempFile.WriteString(testHtpasswdContent); err != nil { + t.Fatal("could not write temporary htpasswd file") + } + + options := map[string]interface{}{ + "realm": testRealm, + "path": tempFile.Name(), + } + + accessController, err := newAccessController(options) + if err != nil { + t.Fatal("error creating access controller") + } + + tempFile.Close() + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + ctx := context.WithValue(nil, "http.request", r) + authCtx, err := accessController.Authorized(ctx) + if err != nil { + switch err := err.(type) { + case auth.Challenge: + err.ServeHTTP(w, r) + return + default: + t.Fatalf("unexpected error authorizing request: %v", err) + } + } + + userInfo, ok := authCtx.Value("auth.user").(auth.UserInfo) + if !ok { + t.Fatal("basic accessController did not set auth.user context") + } + + if userInfo.Name != testUser { + t.Fatalf("expected user name %q, got %q", testUser, userInfo.Name) + } + + w.WriteHeader(http.StatusNoContent) + })) + + client := &http.Client{ + CheckRedirect: nil, + } + + req, _ := http.NewRequest("GET", server.URL, nil) + resp, err := client.Do(req) + + if err != nil { + t.Fatalf("unexpected error during GET: %v", err) + } + defer resp.Body.Close() + + // Request should not be authorized + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("unexpected non-fail response status: %v != %v", resp.StatusCode, http.StatusUnauthorized) + } + + req, _ = http.NewRequest("GET", server.URL, nil) + + sekrit := "bilbo:baggins" + credential := "Basic " + base64.StdEncoding.EncodeToString([]byte(sekrit)) + + req.Header.Set("Authorization", credential) + resp, err = client.Do(req) + + if err != nil { + t.Fatalf("unexpected error during GET: %v", err) + } + defer resp.Body.Close() + + // Request should be authorized + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("unexpected non-success response status: %v != %v", resp.StatusCode, http.StatusNoContent) + } + +} diff --git a/docs/auth/basic/htpasswd.go b/docs/auth/basic/htpasswd.go new file mode 100644 index 00000000..6833bc5c --- /dev/null +++ b/docs/auth/basic/htpasswd.go @@ -0,0 +1,49 @@ +package basic + +import ( + "crypto/sha1" + "encoding/base64" + "encoding/csv" + "errors" + "os" +) + +var ErrSHARequired = errors.New("htpasswd file must use SHA (htpasswd -s)") + +type HTPasswd struct { + path string + reader *csv.Reader +} + +func NewHTPasswd(htpath string) *HTPasswd { + return &HTPasswd{path: htpath} +} + +func (htpasswd *HTPasswd) AuthenticateUser(user string, pwd string) (bool, error) { + + // Hash the credential. + sha := sha1.New() + sha.Write([]byte(pwd)) + hash := base64.StdEncoding.EncodeToString(sha.Sum(nil)) + + // Open the file. + in, err := os.Open(htpasswd.path) + if err != nil { + return false, err + } + + // Parse the contents of the standard .htpasswd until we hit the end or find a match. + reader := csv.NewReader(in) + reader.Comma = ':' + reader.Comment = '#' + reader.TrimLeadingSpace = true + for entry, readerr := reader.Read(); entry != nil || readerr != nil; entry, readerr = reader.Read() { + if entry[0] == user { + if len(entry[1]) < 6 || entry[1][0:5] != "{SHA}" { + return false, ErrSHARequired + } + return entry[1][5:] == hash, nil + } + } + return false, nil +} From 7733b6c892ebf15b603385152ba0a526c5c2af94 Mon Sep 17 00:00:00 2001 From: Dave Trombley Date: Wed, 22 Apr 2015 14:35:59 +0000 Subject: [PATCH 02/11] Fixed WWW-Authenticate: header, added example config and import into main, fixed golint warnings Signed-off-by: Dave Trombley --- docs/auth/basic/access.go | 5 +++-- docs/auth/basic/htpasswd.go | 5 +++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/auth/basic/access.go b/docs/auth/basic/access.go index 1833296a..76f036c0 100644 --- a/docs/auth/basic/access.go +++ b/docs/auth/basic/access.go @@ -6,7 +6,6 @@ // system crypt() may be as well. // // This authentication method MUST be used under TLS, as simple token-replay attack is possible. - package basic import ( @@ -33,7 +32,9 @@ type challenge struct { var _ auth.AccessController = &accessController{} var ( + // ErrPasswordRequired - returned when no auth token is given. ErrPasswordRequired = errors.New("authorization credential required") + // ErrInvalidCredential - returned when the auth token does not authenticate correctly. ErrInvalidCredential = errors.New("invalid authorization credential") ) @@ -98,7 +99,7 @@ func (ac *accessController) Authorized(ctx context.Context, accessRecords ...aut } func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) { - header := fmt.Sprintf("Realm realm=%q", ch.realm) + header := fmt.Sprintf("Basic realm=%q", ch.realm) w.Header().Set("WWW-Authenticate", header) w.WriteHeader(http.StatusUnauthorized) } diff --git a/docs/auth/basic/htpasswd.go b/docs/auth/basic/htpasswd.go index 6833bc5c..36eca347 100644 --- a/docs/auth/basic/htpasswd.go +++ b/docs/auth/basic/htpasswd.go @@ -8,17 +8,22 @@ import ( "os" ) +// ErrSHARequired - returned in error field of challenge when the htpasswd was not made using SHA1 algorithm. +// (SHA1 is considered obsolete but the alternative for htpasswd is MD5, or system crypt...) var ErrSHARequired = errors.New("htpasswd file must use SHA (htpasswd -s)") +// HTPasswd - holds a path to a system .htpasswd file and the machinery to parse it. type HTPasswd struct { path string reader *csv.Reader } +// NewHTPasswd - Create a new HTPasswd with the given path to .htpasswd file. func NewHTPasswd(htpath string) *HTPasswd { return &HTPasswd{path: htpath} } +// AuthenticateUser - Check a given user:password credential against the receiving HTPasswd's file. func (htpasswd *HTPasswd) AuthenticateUser(user string, pwd string) (bool, error) { // Hash the credential. From d2b7988b7f18568112ae65f4382d40e0f7388114 Mon Sep 17 00:00:00 2001 From: Dave Trombley Date: Wed, 22 Apr 2015 15:13:48 +0000 Subject: [PATCH 03/11] Aligned formatting with gofmt Signed-off-by: Dave Trombley --- docs/auth/basic/access.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/auth/basic/access.go b/docs/auth/basic/access.go index 76f036c0..dd792374 100644 --- a/docs/auth/basic/access.go +++ b/docs/auth/basic/access.go @@ -33,8 +33,8 @@ type challenge struct { var _ auth.AccessController = &accessController{} var ( // ErrPasswordRequired - returned when no auth token is given. - ErrPasswordRequired = errors.New("authorization credential required") - // ErrInvalidCredential - returned when the auth token does not authenticate correctly. + ErrPasswordRequired = errors.New("authorization credential required") + // ErrInvalidCredential - returned when the auth token does not authenticate correctly. ErrInvalidCredential = errors.New("invalid authorization credential") ) From ff67393b2b47608c654559fdd51d3c3fe9ee2b5c Mon Sep 17 00:00:00 2001 From: Dave Trombley Date: Thu, 4 Jun 2015 11:46:34 -0400 Subject: [PATCH 04/11] Added support for bcrypt, plaintext; extension points for other htpasswd hash methods. Signed-off-by: Dave Trombley --- docs/auth/basic/access.go | 38 ++++---------- docs/auth/basic/access_test.go | 48 ++++++++++------- docs/auth/basic/htpasswd.go | 95 ++++++++++++++++++++++++++++++---- 3 files changed, 123 insertions(+), 58 deletions(-) diff --git a/docs/auth/basic/access.go b/docs/auth/basic/access.go index dd792374..81a22b40 100644 --- a/docs/auth/basic/access.go +++ b/docs/auth/basic/access.go @@ -9,11 +9,9 @@ package basic import ( - "encoding/base64" "errors" "fmt" "net/http" - "strings" ctxu "github.com/docker/distribution/context" "github.com/docker/distribution/registry/auth" @@ -58,8 +56,7 @@ func (ac *accessController) Authorized(ctx context.Context, accessRecords ...aut return nil, err } - authHeader := req.Header.Get("Authorization") - + authHeader := req.Header.Get("Authorization") if authHeader == "" { challenge := challenge{ realm: ac.realm, @@ -67,35 +64,20 @@ func (ac *accessController) Authorized(ctx context.Context, accessRecords ...aut return nil, &challenge } - parts := strings.Split(req.Header.Get("Authorization"), " ") - - challenge := challenge{ - realm: ac.realm, + user, pass, ok := req.BasicAuth() + if !ok { + return nil, errors.New("Invalid Authorization header") } - - if len(parts) != 2 || strings.ToLower(parts[0]) != "basic" { - challenge.err = ErrPasswordRequired - return nil, &challenge - } - - text, err := base64.StdEncoding.DecodeString(parts[1]) - if err != nil { + + if res, _ := ac.htpasswd.AuthenticateUser(user, pass); !res { + challenge := challenge{ + realm: ac.realm, + } challenge.err = ErrInvalidCredential return nil, &challenge } - credential := strings.Split(string(text), ":") - if len(credential) != 2 { - challenge.err = ErrInvalidCredential - return nil, &challenge - } - - if res, _ := ac.htpasswd.AuthenticateUser(credential[0], credential[1]); !res { - challenge.err = ErrInvalidCredential - return nil, &challenge - } - - return auth.WithUser(ctx, auth.UserInfo{Name: credential[0]}), nil + return auth.WithUser(ctx, auth.UserInfo{Name: user}), nil } func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) { diff --git a/docs/auth/basic/access_test.go b/docs/auth/basic/access_test.go index d82573b9..b731675e 100644 --- a/docs/auth/basic/access_test.go +++ b/docs/auth/basic/access_test.go @@ -14,8 +14,13 @@ import ( func TestBasicAccessController(t *testing.T) { testRealm := "The-Shire" - testUser := "bilbo" - testHtpasswdContent := "bilbo:{SHA}5siv5c0SHx681xU6GiSx9ZQryqs=" + testUsers := []string{"bilbo","frodo","MiShil","DeokMan"} + testPasswords := []string{"baggins","baggins","새주","공주님"} + testHtpasswdContent := `bilbo:{SHA}5siv5c0SHx681xU6GiSx9ZQryqs= + frodo:$2y$05$926C3y10Quzn/LnqQH86VOEVh/18T6RnLaS.khre96jLNL/7e.K5W + MiShil:$2y$05$0oHgwMehvoe8iAWS8I.7l.KoECXrwVaC16RPfaSCU5eVTFrATuMI2 + DeokMan:공주님` + tempFile, err := ioutil.TempFile("", "htpasswd-test") if err != nil { @@ -36,7 +41,9 @@ func TestBasicAccessController(t *testing.T) { } tempFile.Close() - + + var userNumber = 0 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := context.WithValue(nil, "http.request", r) authCtx, err := accessController.Authorized(ctx) @@ -55,8 +62,8 @@ func TestBasicAccessController(t *testing.T) { t.Fatal("basic accessController did not set auth.user context") } - if userInfo.Name != testUser { - t.Fatalf("expected user name %q, got %q", testUser, userInfo.Name) + if userInfo.Name != testUsers[userNumber] { + t.Fatalf("expected user name %q, got %q", testUsers[userNumber], userInfo.Name) } w.WriteHeader(http.StatusNoContent) @@ -79,22 +86,25 @@ func TestBasicAccessController(t *testing.T) { t.Fatalf("unexpected non-fail response status: %v != %v", resp.StatusCode, http.StatusUnauthorized) } - req, _ = http.NewRequest("GET", server.URL, nil) + for i := 0; i < len(testUsers); i++ { + userNumber = i + req, _ = http.NewRequest("GET", server.URL, nil) + sekrit := testUsers[i]+":"+testPasswords[i] + credential := "Basic " + base64.StdEncoding.EncodeToString([]byte(sekrit)) - sekrit := "bilbo:baggins" - credential := "Basic " + base64.StdEncoding.EncodeToString([]byte(sekrit)) + req.Header.Set("Authorization", credential) + resp, err = client.Do(req) + + if err != nil { + t.Fatalf("unexpected error during GET: %v", err) + } + defer resp.Body.Close() - req.Header.Set("Authorization", credential) - resp, err = client.Do(req) - - if err != nil { - t.Fatalf("unexpected error during GET: %v", err) - } - defer resp.Body.Close() - - // Request should be authorized - if resp.StatusCode != http.StatusNoContent { - t.Fatalf("unexpected non-success response status: %v != %v", resp.StatusCode, http.StatusNoContent) + // Request should be authorized + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("unexpected non-success response status: %v != %v for %s %s %s", resp.StatusCode, http.StatusNoContent, testUsers[i], testPasswords[i], credential) + } } + } diff --git a/docs/auth/basic/htpasswd.go b/docs/auth/basic/htpasswd.go index 36eca347..69dae9d8 100644 --- a/docs/auth/basic/htpasswd.go +++ b/docs/auth/basic/htpasswd.go @@ -6,11 +6,14 @@ import ( "encoding/csv" "errors" "os" + "regexp" + "strings" + + "golang.org/x/crypto/bcrypt" ) -// ErrSHARequired - returned in error field of challenge when the htpasswd was not made using SHA1 algorithm. -// (SHA1 is considered obsolete but the alternative for htpasswd is MD5, or system crypt...) -var ErrSHARequired = errors.New("htpasswd file must use SHA (htpasswd -s)") +// AuthenticationFailureErr - a generic error message for authentication failure to be presented to agent. +var AuthenticationFailureErr = errors.New("Bad username or password") // HTPasswd - holds a path to a system .htpasswd file and the machinery to parse it. type HTPasswd struct { @@ -18,18 +21,57 @@ type HTPasswd struct { reader *csv.Reader } +// AuthType represents a particular hash function used in the htpasswd file. +type AuthType int +const ( + PlainText AuthType = iota + SHA1 + ApacheMD5 + BCrypt + Crypt +) + +// String returns a text representation of the AuthType +func (at AuthType) String() string { + switch(at) { + case PlainText: return "plaintext" + case SHA1: return "sha1" + case ApacheMD5: return "md5" + case BCrypt: return "bcrypt" + case Crypt: return "system crypt" + } + return "unknown" +} + + // NewHTPasswd - Create a new HTPasswd with the given path to .htpasswd file. func NewHTPasswd(htpath string) *HTPasswd { return &HTPasswd{path: htpath} } +var bcryptPrefixRegexp *regexp.Regexp = regexp.MustCompile(`^\$2[ab]?y\$`) + +// GetAuthCredentialType - Inspect an htpasswd file credential and guess the encryption algorithm used. +func GetAuthCredentialType(cred string) AuthType { + if strings.HasPrefix(cred, "{SHA}") { + return SHA1 + } + if strings.HasPrefix(cred, "$apr1$") { + return ApacheMD5 + } + if bcryptPrefixRegexp.MatchString(cred) { + return BCrypt + } + // There's just not a great way to distinguish between these next two... + if len(cred) == 13 { + return Crypt + } + return PlainText +} + // AuthenticateUser - Check a given user:password credential against the receiving HTPasswd's file. func (htpasswd *HTPasswd) AuthenticateUser(user string, pwd string) (bool, error) { - // Hash the credential. - sha := sha1.New() - sha.Write([]byte(pwd)) - hash := base64.StdEncoding.EncodeToString(sha.Sum(nil)) // Open the file. in, err := os.Open(htpasswd.path) @@ -43,12 +85,43 @@ func (htpasswd *HTPasswd) AuthenticateUser(user string, pwd string) (bool, error reader.Comment = '#' reader.TrimLeadingSpace = true for entry, readerr := reader.Read(); entry != nil || readerr != nil; entry, readerr = reader.Read() { + if readerr != nil { + return false, readerr + } + if len(entry) == 0 { + continue + } if entry[0] == user { - if len(entry[1]) < 6 || entry[1][0:5] != "{SHA}" { - return false, ErrSHARequired + credential := entry[1] + credType := GetAuthCredentialType(credential) + switch(credType) { + case SHA1: { + sha := sha1.New() + sha.Write([]byte(pwd)) + hash := base64.StdEncoding.EncodeToString(sha.Sum(nil)) + return entry[1][5:] == hash, nil + } + case ApacheMD5: { + return false, errors.New(ApacheMD5.String()+" htpasswd hash function not yet supported") + } + case BCrypt: { + err := bcrypt.CompareHashAndPassword([]byte(credential),[]byte(pwd)) + if err != nil { + return false, err + } + return true, nil + } + case Crypt: { + return false, errors.New(Crypt.String()+" htpasswd hash function not yet supported") + } + case PlainText: { + if pwd == credential { + return true, nil + } + return false, AuthenticationFailureErr + } } - return entry[1][5:] == hash, nil } } - return false, nil + return false, AuthenticationFailureErr } From 15bbde99c1cbb7ecd21e6ea310bd592a45fb2125 Mon Sep 17 00:00:00 2001 From: Dave Trombley Date: Thu, 4 Jun 2015 12:02:13 -0400 Subject: [PATCH 05/11] Fixed golint, gofmt warning advice. Signed-off-by: Dave Trombley --- docs/auth/basic/access.go | 4 +-- docs/auth/basic/access_test.go | 14 ++++---- docs/auth/basic/htpasswd.go | 64 +++++++++++++++++++++------------- 3 files changed, 47 insertions(+), 35 deletions(-) diff --git a/docs/auth/basic/access.go b/docs/auth/basic/access.go index 81a22b40..0b3e2788 100644 --- a/docs/auth/basic/access.go +++ b/docs/auth/basic/access.go @@ -56,7 +56,7 @@ func (ac *accessController) Authorized(ctx context.Context, accessRecords ...aut return nil, err } - authHeader := req.Header.Get("Authorization") + authHeader := req.Header.Get("Authorization") if authHeader == "" { challenge := challenge{ realm: ac.realm, @@ -68,7 +68,7 @@ func (ac *accessController) Authorized(ctx context.Context, accessRecords ...aut if !ok { return nil, errors.New("Invalid Authorization header") } - + if res, _ := ac.htpasswd.AuthenticateUser(user, pass); !res { challenge := challenge{ realm: ac.realm, diff --git a/docs/auth/basic/access_test.go b/docs/auth/basic/access_test.go index b731675e..62699a63 100644 --- a/docs/auth/basic/access_test.go +++ b/docs/auth/basic/access_test.go @@ -14,13 +14,12 @@ import ( func TestBasicAccessController(t *testing.T) { testRealm := "The-Shire" - testUsers := []string{"bilbo","frodo","MiShil","DeokMan"} - testPasswords := []string{"baggins","baggins","새주","공주님"} + testUsers := []string{"bilbo", "frodo", "MiShil", "DeokMan"} + testPasswords := []string{"baggins", "baggins", "새주", "공주님"} testHtpasswdContent := `bilbo:{SHA}5siv5c0SHx681xU6GiSx9ZQryqs= frodo:$2y$05$926C3y10Quzn/LnqQH86VOEVh/18T6RnLaS.khre96jLNL/7e.K5W MiShil:$2y$05$0oHgwMehvoe8iAWS8I.7l.KoECXrwVaC16RPfaSCU5eVTFrATuMI2 DeokMan:공주님` - tempFile, err := ioutil.TempFile("", "htpasswd-test") if err != nil { @@ -41,9 +40,9 @@ func TestBasicAccessController(t *testing.T) { } tempFile.Close() - + var userNumber = 0 - + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { ctx := context.WithValue(nil, "http.request", r) authCtx, err := accessController.Authorized(ctx) @@ -89,12 +88,12 @@ func TestBasicAccessController(t *testing.T) { for i := 0; i < len(testUsers); i++ { userNumber = i req, _ = http.NewRequest("GET", server.URL, nil) - sekrit := testUsers[i]+":"+testPasswords[i] + sekrit := testUsers[i] + ":" + testPasswords[i] credential := "Basic " + base64.StdEncoding.EncodeToString([]byte(sekrit)) req.Header.Set("Authorization", credential) resp, err = client.Do(req) - + if err != nil { t.Fatalf("unexpected error during GET: %v", err) } @@ -105,6 +104,5 @@ func TestBasicAccessController(t *testing.T) { t.Fatalf("unexpected non-success response status: %v != %v for %s %s %s", resp.StatusCode, http.StatusNoContent, testUsers[i], testPasswords[i], credential) } } - } diff --git a/docs/auth/basic/htpasswd.go b/docs/auth/basic/htpasswd.go index 69dae9d8..89e4b749 100644 --- a/docs/auth/basic/htpasswd.go +++ b/docs/auth/basic/htpasswd.go @@ -8,12 +8,12 @@ import ( "os" "regexp" "strings" - + "golang.org/x/crypto/bcrypt" ) // AuthenticationFailureErr - a generic error message for authentication failure to be presented to agent. -var AuthenticationFailureErr = errors.New("Bad username or password") +var ErrAuthenticationFailure = errors.New("Bad username or password") // HTPasswd - holds a path to a system .htpasswd file and the machinery to parse it. type HTPasswd struct { @@ -22,34 +22,44 @@ type HTPasswd struct { } // AuthType represents a particular hash function used in the htpasswd file. -type AuthType int +type AuthType int + const ( - PlainText AuthType = iota + // PlainText - Plain-text password storage (htpasswd -p) + PlainText AuthType = iota + // SHA1 - sha hashed password storage (htpasswd -s) SHA1 + // ApacheMD5 - apr iterated md5 hashing (htpasswd -m) ApacheMD5 + // BCrypt - BCrypt adapative password hashing (htpasswd -B) BCrypt + // Crypt - System crypt() hashes. (htpasswd -d) Crypt ) // String returns a text representation of the AuthType func (at AuthType) String() string { - switch(at) { - case PlainText: return "plaintext" - case SHA1: return "sha1" - case ApacheMD5: return "md5" - case BCrypt: return "bcrypt" - case Crypt: return "system crypt" + switch at { + case PlainText: + return "plaintext" + case SHA1: + return "sha1" + case ApacheMD5: + return "md5" + case BCrypt: + return "bcrypt" + case Crypt: + return "system crypt" } return "unknown" } - // NewHTPasswd - Create a new HTPasswd with the given path to .htpasswd file. func NewHTPasswd(htpath string) *HTPasswd { return &HTPasswd{path: htpath} } -var bcryptPrefixRegexp *regexp.Regexp = regexp.MustCompile(`^\$2[ab]?y\$`) +var bcryptPrefixRegexp = regexp.MustCompile(`^\$2[ab]?y\$`) // GetAuthCredentialType - Inspect an htpasswd file credential and guess the encryption algorithm used. func GetAuthCredentialType(cred string) AuthType { @@ -72,7 +82,6 @@ func GetAuthCredentialType(cred string) AuthType { // AuthenticateUser - Check a given user:password credential against the receiving HTPasswd's file. func (htpasswd *HTPasswd) AuthenticateUser(user string, pwd string) (bool, error) { - // Open the file. in, err := os.Open(htpasswd.path) if err != nil { @@ -94,34 +103,39 @@ func (htpasswd *HTPasswd) AuthenticateUser(user string, pwd string) (bool, error if entry[0] == user { credential := entry[1] credType := GetAuthCredentialType(credential) - switch(credType) { - case SHA1: { + switch credType { + case SHA1: + { sha := sha1.New() sha.Write([]byte(pwd)) hash := base64.StdEncoding.EncodeToString(sha.Sum(nil)) return entry[1][5:] == hash, nil } - case ApacheMD5: { - return false, errors.New(ApacheMD5.String()+" htpasswd hash function not yet supported") + case ApacheMD5: + { + return false, errors.New(ApacheMD5.String() + " htpasswd hash function not yet supported") } - case BCrypt: { - err := bcrypt.CompareHashAndPassword([]byte(credential),[]byte(pwd)) + case BCrypt: + { + err := bcrypt.CompareHashAndPassword([]byte(credential), []byte(pwd)) if err != nil { return false, err } return true, nil } - case Crypt: { - return false, errors.New(Crypt.String()+" htpasswd hash function not yet supported") + case Crypt: + { + return false, errors.New(Crypt.String() + " htpasswd hash function not yet supported") } - case PlainText: { + case PlainText: + { if pwd == credential { return true, nil - } - return false, AuthenticationFailureErr + } + return false, ErrAuthenticationFailure } } } } - return false, AuthenticationFailureErr + return false, ErrAuthenticationFailure } From fe9ca88946c54cd14ea3481caaad1541dd2461cf Mon Sep 17 00:00:00 2001 From: Dave Trombley Date: Sat, 6 Jun 2015 01:37:32 -0400 Subject: [PATCH 06/11] Removed dashes from comments, unexported htpasswd struct Signed-off-by: Dave Trombley --- docs/auth/basic/access.go | 9 +++------ docs/auth/basic/htpasswd.go | 32 ++++++++++++++++---------------- 2 files changed, 19 insertions(+), 22 deletions(-) diff --git a/docs/auth/basic/access.go b/docs/auth/basic/access.go index 0b3e2788..52b790d2 100644 --- a/docs/auth/basic/access.go +++ b/docs/auth/basic/access.go @@ -2,9 +2,6 @@ // user credential hash in an htpasswd formatted file in a configuration-determined // location. // -// The use of SHA hashes (htpasswd -s) is enforced since MD5 is insecure and simple -// system crypt() may be as well. -// // This authentication method MUST be used under TLS, as simple token-replay attack is possible. package basic @@ -20,7 +17,7 @@ import ( type accessController struct { realm string - htpasswd *HTPasswd + htpasswd *htpasswd } type challenge struct { @@ -30,9 +27,9 @@ type challenge struct { var _ auth.AccessController = &accessController{} var ( - // ErrPasswordRequired - returned when no auth token is given. + // ErrPasswordRequired Returned when no auth token is given. ErrPasswordRequired = errors.New("authorization credential required") - // ErrInvalidCredential - returned when the auth token does not authenticate correctly. + // ErrInvalidCredential is returned when the auth token does not authenticate correctly. ErrInvalidCredential = errors.New("invalid authorization credential") ) diff --git a/docs/auth/basic/htpasswd.go b/docs/auth/basic/htpasswd.go index 89e4b749..91d45e77 100644 --- a/docs/auth/basic/htpasswd.go +++ b/docs/auth/basic/htpasswd.go @@ -12,32 +12,32 @@ import ( "golang.org/x/crypto/bcrypt" ) -// AuthenticationFailureErr - a generic error message for authentication failure to be presented to agent. +// ErrAuthenticationFailure A generic error message for authentication failure to be presented to agent. var ErrAuthenticationFailure = errors.New("Bad username or password") -// HTPasswd - holds a path to a system .htpasswd file and the machinery to parse it. -type HTPasswd struct { +// htpasswd Holds a path to a system .htpasswd file and the machinery to parse it. +type htpasswd struct { path string reader *csv.Reader } -// AuthType represents a particular hash function used in the htpasswd file. +// AuthType Represents a particular hash function used in the htpasswd file. type AuthType int const ( - // PlainText - Plain-text password storage (htpasswd -p) + // PlainText Plain-text password storage (htpasswd -p) PlainText AuthType = iota - // SHA1 - sha hashed password storage (htpasswd -s) + // SHA1 sha hashed password storage (htpasswd -s) SHA1 - // ApacheMD5 - apr iterated md5 hashing (htpasswd -m) + // ApacheMD5 apr iterated md5 hashing (htpasswd -m) ApacheMD5 - // BCrypt - BCrypt adapative password hashing (htpasswd -B) + // BCrypt BCrypt adapative password hashing (htpasswd -B) BCrypt - // Crypt - System crypt() hashes. (htpasswd -d) + // Crypt System crypt() hashes. (htpasswd -d) Crypt ) -// String returns a text representation of the AuthType +// String Returns a text representation of the AuthType func (at AuthType) String() string { switch at { case PlainText: @@ -54,14 +54,14 @@ func (at AuthType) String() string { return "unknown" } -// NewHTPasswd - Create a new HTPasswd with the given path to .htpasswd file. -func NewHTPasswd(htpath string) *HTPasswd { - return &HTPasswd{path: htpath} +// NewHTPasswd Create a new HTPasswd with the given path to .htpasswd file. +func NewHTPasswd(htpath string) *htpasswd { + return &htpasswd{path: htpath} } var bcryptPrefixRegexp = regexp.MustCompile(`^\$2[ab]?y\$`) -// GetAuthCredentialType - Inspect an htpasswd file credential and guess the encryption algorithm used. +// GetAuthCredentialType Inspect an htpasswd file credential and guess the encryption algorithm used. func GetAuthCredentialType(cred string) AuthType { if strings.HasPrefix(cred, "{SHA}") { return SHA1 @@ -79,8 +79,8 @@ func GetAuthCredentialType(cred string) AuthType { return PlainText } -// AuthenticateUser - Check a given user:password credential against the receiving HTPasswd's file. -func (htpasswd *HTPasswd) AuthenticateUser(user string, pwd string) (bool, error) { +// AuthenticateUser Check a given user:password credential against the receiving HTPasswd's file. +func (htpasswd *htpasswd) AuthenticateUser(user string, pwd string) (bool, error) { // Open the file. in, err := os.Open(htpasswd.path) From 350444568082cfc54a10c30e229b00aeeb0e8e1a Mon Sep 17 00:00:00 2001 From: Dave Trombley Date: Sat, 6 Jun 2015 01:58:45 -0400 Subject: [PATCH 07/11] Unexported function to comply with golint Signed-off-by: Dave Trombley --- docs/auth/basic/access.go | 2 +- docs/auth/basic/htpasswd.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/auth/basic/access.go b/docs/auth/basic/access.go index 52b790d2..24f4009f 100644 --- a/docs/auth/basic/access.go +++ b/docs/auth/basic/access.go @@ -44,7 +44,7 @@ func newAccessController(options map[string]interface{}) (auth.AccessController, return nil, fmt.Errorf(`"path" must be set for basic access controller`) } - return &accessController{realm: realm.(string), htpasswd: NewHTPasswd(path.(string))}, nil + return &accessController{realm: realm.(string), htpasswd: newHTPasswd(path.(string))}, nil } func (ac *accessController) Authorized(ctx context.Context, accessRecords ...auth.Access) (context.Context, error) { diff --git a/docs/auth/basic/htpasswd.go b/docs/auth/basic/htpasswd.go index 91d45e77..cc305ff1 100644 --- a/docs/auth/basic/htpasswd.go +++ b/docs/auth/basic/htpasswd.go @@ -55,7 +55,7 @@ func (at AuthType) String() string { } // NewHTPasswd Create a new HTPasswd with the given path to .htpasswd file. -func NewHTPasswd(htpath string) *htpasswd { +func newHTPasswd(htpath string) *htpasswd { return &htpasswd{path: htpath} } From 427c457801637e3d5b8a0e1f88cc1095a3f193c9 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Mon, 8 Jun 2015 18:56:48 -0700 Subject: [PATCH 08/11] Refactor Basic Authentication package This change refactors the basic authentication implementation to better follow Go coding standards. Many types are no longer exported. The parser is now a separate function from the authentication code. The standard functions (*http.Request).BasicAuth/SetBasicAuth are now used where appropriate. Signed-off-by: Stephen J Day --- docs/auth/basic/access.go | 54 +++++---- docs/auth/basic/access_test.go | 19 +-- docs/auth/basic/htpasswd.go | 203 +++++++++++++++++---------------- 3 files changed, 142 insertions(+), 134 deletions(-) diff --git a/docs/auth/basic/access.go b/docs/auth/basic/access.go index 24f4009f..11e4ae5a 100644 --- a/docs/auth/basic/access.go +++ b/docs/auth/basic/access.go @@ -15,23 +15,20 @@ import ( "golang.org/x/net/context" ) +var ( + // ErrInvalidCredential is returned when the auth token does not authenticate correctly. + ErrInvalidCredential = errors.New("invalid authorization credential") + + // ErrAuthenticationFailure returned when authentication failure to be presented to agent. + ErrAuthenticationFailure = errors.New("authentication failured") +) + type accessController struct { realm string htpasswd *htpasswd } -type challenge struct { - realm string - err error -} - var _ auth.AccessController = &accessController{} -var ( - // ErrPasswordRequired Returned when no auth token is given. - ErrPasswordRequired = errors.New("authorization credential required") - // ErrInvalidCredential is returned when the auth token does not authenticate correctly. - ErrInvalidCredential = errors.New("invalid authorization credential") -) func newAccessController(options map[string]interface{}) (auth.AccessController, error) { realm, present := options["realm"] @@ -53,28 +50,29 @@ func (ac *accessController) Authorized(ctx context.Context, accessRecords ...aut return nil, err } - authHeader := req.Header.Get("Authorization") - if authHeader == "" { - challenge := challenge{ - realm: ac.realm, - } - return nil, &challenge - } - - user, pass, ok := req.BasicAuth() + username, password, ok := req.BasicAuth() if !ok { - return nil, errors.New("Invalid Authorization header") - } - - if res, _ := ac.htpasswd.AuthenticateUser(user, pass); !res { - challenge := challenge{ + return nil, &challenge{ realm: ac.realm, + err: ErrInvalidCredential, } - challenge.err = ErrInvalidCredential - return nil, &challenge } - return auth.WithUser(ctx, auth.UserInfo{Name: user}), nil + if err := ac.htpasswd.authenticateUser(ctx, username, password); err != nil { + ctxu.GetLogger(ctx).Errorf("error authenticating user %q: %v", username, err) + return nil, &challenge{ + realm: ac.realm, + err: ErrAuthenticationFailure, + } + } + + return auth.WithUser(ctx, auth.UserInfo{Name: username}), nil +} + +// challenge implements the auth.Challenge interface. +type challenge struct { + realm string + err error } func (ch *challenge) ServeHTTP(w http.ResponseWriter, r *http.Request) { diff --git a/docs/auth/basic/access_test.go b/docs/auth/basic/access_test.go index 62699a63..3bc99437 100644 --- a/docs/auth/basic/access_test.go +++ b/docs/auth/basic/access_test.go @@ -1,14 +1,13 @@ package basic import ( - "encoding/base64" "io/ioutil" "net/http" "net/http/httptest" "testing" + "github.com/docker/distribution/context" "github.com/docker/distribution/registry/auth" - "golang.org/x/net/context" ) func TestBasicAccessController(t *testing.T) { @@ -33,6 +32,7 @@ func TestBasicAccessController(t *testing.T) { "realm": testRealm, "path": tempFile.Name(), } + ctx := context.Background() accessController, err := newAccessController(options) if err != nil { @@ -44,7 +44,7 @@ func TestBasicAccessController(t *testing.T) { var userNumber = 0 server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - ctx := context.WithValue(nil, "http.request", r) + ctx := context.WithRequest(ctx, r) authCtx, err := accessController.Authorized(ctx) if err != nil { switch err := err.(type) { @@ -87,13 +87,14 @@ func TestBasicAccessController(t *testing.T) { for i := 0; i < len(testUsers); i++ { userNumber = i - req, _ = http.NewRequest("GET", server.URL, nil) - sekrit := testUsers[i] + ":" + testPasswords[i] - credential := "Basic " + base64.StdEncoding.EncodeToString([]byte(sekrit)) + req, err := http.NewRequest("GET", server.URL, nil) + if err != nil { + t.Fatalf("error allocating new request: %v", err) + } + + req.SetBasicAuth(testUsers[i], testPasswords[i]) - req.Header.Set("Authorization", credential) resp, err = client.Do(req) - if err != nil { t.Fatalf("unexpected error during GET: %v", err) } @@ -101,7 +102,7 @@ func TestBasicAccessController(t *testing.T) { // Request should be authorized if resp.StatusCode != http.StatusNoContent { - t.Fatalf("unexpected non-success response status: %v != %v for %s %s %s", resp.StatusCode, http.StatusNoContent, testUsers[i], testPasswords[i], credential) + t.Fatalf("unexpected non-success response status: %v != %v for %s %s", resp.StatusCode, http.StatusNoContent, testUsers[i], testPasswords[i]) } } diff --git a/docs/auth/basic/htpasswd.go b/docs/auth/basic/htpasswd.go index cc305ff1..f50805e7 100644 --- a/docs/auth/basic/htpasswd.go +++ b/docs/auth/basic/htpasswd.go @@ -1,54 +1,66 @@ package basic import ( + "bufio" "crypto/sha1" "encoding/base64" - "encoding/csv" - "errors" + "io" "os" "regexp" "strings" + "github.com/docker/distribution/context" "golang.org/x/crypto/bcrypt" ) -// ErrAuthenticationFailure A generic error message for authentication failure to be presented to agent. -var ErrAuthenticationFailure = errors.New("Bad username or password") - -// htpasswd Holds a path to a system .htpasswd file and the machinery to parse it. +// htpasswd holds a path to a system .htpasswd file and the machinery to parse it. type htpasswd struct { - path string - reader *csv.Reader + path string } -// AuthType Represents a particular hash function used in the htpasswd file. -type AuthType int +// authType represents a particular hash function used in the htpasswd file. +type authType int const ( - // PlainText Plain-text password storage (htpasswd -p) - PlainText AuthType = iota - // SHA1 sha hashed password storage (htpasswd -s) - SHA1 - // ApacheMD5 apr iterated md5 hashing (htpasswd -m) - ApacheMD5 - // BCrypt BCrypt adapative password hashing (htpasswd -B) - BCrypt - // Crypt System crypt() hashes. (htpasswd -d) - Crypt + authTypePlainText authType = iota // Plain-text password storage (htpasswd -p) + authTypeSHA1 // sha hashed password storage (htpasswd -s) + authTypeApacheMD5 // apr iterated md5 hashing (htpasswd -m) + authTypeBCrypt // BCrypt adapative password hashing (htpasswd -B) + authTypeCrypt // System crypt() hashes. (htpasswd -d) ) +var bcryptPrefixRegexp = regexp.MustCompile(`^\$2[ab]?y\$`) + +// detectAuthCredentialType inspects the credential and resolves the encryption scheme. +func detectAuthCredentialType(cred string) authType { + if strings.HasPrefix(cred, "{SHA}") { + return authTypeSHA1 + } + if strings.HasPrefix(cred, "$apr1$") { + return authTypeApacheMD5 + } + if bcryptPrefixRegexp.MatchString(cred) { + return authTypeBCrypt + } + // There's just not a great way to distinguish between these next two... + if len(cred) == 13 { + return authTypeCrypt + } + return authTypePlainText +} + // String Returns a text representation of the AuthType -func (at AuthType) String() string { +func (at authType) String() string { switch at { - case PlainText: + case authTypePlainText: return "plaintext" - case SHA1: + case authTypeSHA1: return "sha1" - case ApacheMD5: + case authTypeApacheMD5: return "md5" - case BCrypt: + case authTypeBCrypt: return "bcrypt" - case Crypt: + case authTypeCrypt: return "system crypt" } return "unknown" @@ -59,83 +71,80 @@ func newHTPasswd(htpath string) *htpasswd { return &htpasswd{path: htpath} } -var bcryptPrefixRegexp = regexp.MustCompile(`^\$2[ab]?y\$`) - -// GetAuthCredentialType Inspect an htpasswd file credential and guess the encryption algorithm used. -func GetAuthCredentialType(cred string) AuthType { - if strings.HasPrefix(cred, "{SHA}") { - return SHA1 - } - if strings.HasPrefix(cred, "$apr1$") { - return ApacheMD5 - } - if bcryptPrefixRegexp.MatchString(cred) { - return BCrypt - } - // There's just not a great way to distinguish between these next two... - if len(cred) == 13 { - return Crypt - } - return PlainText -} - -// AuthenticateUser Check a given user:password credential against the receiving HTPasswd's file. -func (htpasswd *htpasswd) AuthenticateUser(user string, pwd string) (bool, error) { - +// AuthenticateUser checks a given user:password credential against the +// receiving HTPasswd's file. If the check passes, nil is returned. Note that +// this parses the htpasswd file on each request so ensure that updates are +// available. +func (htpasswd *htpasswd) authenticateUser(ctx context.Context, username string, password string) error { // Open the file. in, err := os.Open(htpasswd.path) if err != nil { - return false, err + return err + } + defer in.Close() + + for _, entry := range parseHTPasswd(ctx, in) { + if entry.username != username { + continue // wrong entry + } + + switch t := detectAuthCredentialType(entry.password); t { + case authTypeSHA1: + sha := sha1.New() + sha.Write([]byte(password)) + hash := base64.StdEncoding.EncodeToString(sha.Sum(nil)) + + if entry.password[5:] != hash { + return ErrAuthenticationFailure + } + + return nil + case authTypeBCrypt: + err := bcrypt.CompareHashAndPassword([]byte(entry.password), []byte(password)) + if err != nil { + return ErrAuthenticationFailure + } + + return nil + case authTypePlainText: + if password != entry.password { + return ErrAuthenticationFailure + } + + return nil + default: + context.GetLogger(ctx).Errorf("unsupported basic authentication type: %v", t) + } } - // Parse the contents of the standard .htpasswd until we hit the end or find a match. - reader := csv.NewReader(in) - reader.Comma = ':' - reader.Comment = '#' - reader.TrimLeadingSpace = true - for entry, readerr := reader.Read(); entry != nil || readerr != nil; entry, readerr = reader.Read() { - if readerr != nil { - return false, readerr - } - if len(entry) == 0 { + return ErrAuthenticationFailure +} + +// htpasswdEntry represents a line in an htpasswd file. +type htpasswdEntry struct { + username string // username, plain text + password string // stores hashed passwd +} + +// parseHTPasswd parses the contents of htpasswd. Bad entries are skipped and +// logged, so this may return empty. This will read all the entries in the +// file, whether or not they are needed. +func parseHTPasswd(ctx context.Context, rd io.Reader) []htpasswdEntry { + entries := []htpasswdEntry{} + scanner := bufio.NewScanner(rd) + for scanner.Scan() { + t := strings.TrimSpace(scanner.Text()) + i := strings.Index(t, ":") + if i < 0 || i >= len(t) { + context.GetLogger(ctx).Errorf("bad entry in htpasswd: %q", t) continue } - if entry[0] == user { - credential := entry[1] - credType := GetAuthCredentialType(credential) - switch credType { - case SHA1: - { - sha := sha1.New() - sha.Write([]byte(pwd)) - hash := base64.StdEncoding.EncodeToString(sha.Sum(nil)) - return entry[1][5:] == hash, nil - } - case ApacheMD5: - { - return false, errors.New(ApacheMD5.String() + " htpasswd hash function not yet supported") - } - case BCrypt: - { - err := bcrypt.CompareHashAndPassword([]byte(credential), []byte(pwd)) - if err != nil { - return false, err - } - return true, nil - } - case Crypt: - { - return false, errors.New(Crypt.String() + " htpasswd hash function not yet supported") - } - case PlainText: - { - if pwd == credential { - return true, nil - } - return false, ErrAuthenticationFailure - } - } - } + + entries = append(entries, htpasswdEntry{ + username: t[:i], + password: t[i+1:], + }) } - return false, ErrAuthenticationFailure + + return entries } From 14f3b07db099d41b47e956ba8509a71f2f022012 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Wed, 10 Jun 2015 19:29:27 -0700 Subject: [PATCH 09/11] Harden basic auth implementation After consideration, the basic authentication implementation has been simplified to only support bcrypt entries in an htpasswd file. This greatly increases the security of the implementation by reducing the possibility of timing attacks and other problems trying to detect the password hash type. Also, the htpasswd file is only parsed at startup, ensuring that the file can be edited and not effect ongoing requests. Newly added passwords take effect on restart. Subsequently, password hash entries are now stored in a map. Test cases have been modified accordingly. Signed-off-by: Stephen J Day --- docs/auth/basic/access.go | 16 ++- docs/auth/basic/access_test.go | 20 +++- docs/auth/basic/htpasswd.go | 166 +++++++++---------------------- docs/auth/basic/htpasswd_test.go | 85 ++++++++++++++++ docs/handlers/app.go | 1 + 5 files changed, 164 insertions(+), 124 deletions(-) create mode 100644 docs/auth/basic/htpasswd_test.go diff --git a/docs/auth/basic/access.go b/docs/auth/basic/access.go index 11e4ae5a..f7d5e79b 100644 --- a/docs/auth/basic/access.go +++ b/docs/auth/basic/access.go @@ -9,6 +9,7 @@ import ( "errors" "fmt" "net/http" + "os" ctxu "github.com/docker/distribution/context" "github.com/docker/distribution/registry/auth" @@ -41,7 +42,18 @@ func newAccessController(options map[string]interface{}) (auth.AccessController, return nil, fmt.Errorf(`"path" must be set for basic access controller`) } - return &accessController{realm: realm.(string), htpasswd: newHTPasswd(path.(string))}, nil + f, err := os.Open(path.(string)) + if err != nil { + return nil, err + } + defer f.Close() + + h, err := newHTPasswd(f) + if err != nil { + return nil, err + } + + return &accessController{realm: realm.(string), htpasswd: h}, nil } func (ac *accessController) Authorized(ctx context.Context, accessRecords ...auth.Access) (context.Context, error) { @@ -58,7 +70,7 @@ func (ac *accessController) Authorized(ctx context.Context, accessRecords ...aut } } - if err := ac.htpasswd.authenticateUser(ctx, username, password); err != nil { + if err := ac.htpasswd.authenticateUser(username, password); err != nil { ctxu.GetLogger(ctx).Errorf("error authenticating user %q: %v", username, err) return nil, &challenge{ realm: ac.realm, diff --git a/docs/auth/basic/access_test.go b/docs/auth/basic/access_test.go index 3bc99437..1976b32e 100644 --- a/docs/auth/basic/access_test.go +++ b/docs/auth/basic/access_test.go @@ -11,7 +11,6 @@ import ( ) func TestBasicAccessController(t *testing.T) { - testRealm := "The-Shire" testUsers := []string{"bilbo", "frodo", "MiShil", "DeokMan"} testPasswords := []string{"baggins", "baggins", "새주", "공주님"} @@ -85,6 +84,11 @@ func TestBasicAccessController(t *testing.T) { t.Fatalf("unexpected non-fail response status: %v != %v", resp.StatusCode, http.StatusUnauthorized) } + nonbcrypt := map[string]struct{}{ + "bilbo": struct{}{}, + "DeokMan": struct{}{}, + } + for i := 0; i < len(testUsers); i++ { userNumber = i req, err := http.NewRequest("GET", server.URL, nil) @@ -100,9 +104,17 @@ func TestBasicAccessController(t *testing.T) { } defer resp.Body.Close() - // Request should be authorized - if resp.StatusCode != http.StatusNoContent { - t.Fatalf("unexpected non-success response status: %v != %v for %s %s", resp.StatusCode, http.StatusNoContent, testUsers[i], testPasswords[i]) + if _, ok := nonbcrypt[testUsers[i]]; ok { + // these are not allowed. + // Request should be authorized + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("unexpected non-success response status: %v != %v for %s %s", resp.StatusCode, http.StatusUnauthorized, testUsers[i], testPasswords[i]) + } + } else { + // Request should be authorized + if resp.StatusCode != http.StatusNoContent { + t.Fatalf("unexpected non-success response status: %v != %v for %s %s", resp.StatusCode, http.StatusNoContent, testUsers[i], testPasswords[i]) + } } } diff --git a/docs/auth/basic/htpasswd.go b/docs/auth/basic/htpasswd.go index f50805e7..dd9bb1ac 100644 --- a/docs/auth/basic/htpasswd.go +++ b/docs/auth/basic/htpasswd.go @@ -2,149 +2,79 @@ package basic import ( "bufio" - "crypto/sha1" - "encoding/base64" + "fmt" "io" - "os" - "regexp" "strings" - "github.com/docker/distribution/context" "golang.org/x/crypto/bcrypt" ) -// htpasswd holds a path to a system .htpasswd file and the machinery to parse it. +// htpasswd holds a path to a system .htpasswd file and the machinery to parse +// it. Only bcrypt hash entries are supported. type htpasswd struct { - path string + entries map[string][]byte // maps username to password byte slice. } -// authType represents a particular hash function used in the htpasswd file. -type authType int - -const ( - authTypePlainText authType = iota // Plain-text password storage (htpasswd -p) - authTypeSHA1 // sha hashed password storage (htpasswd -s) - authTypeApacheMD5 // apr iterated md5 hashing (htpasswd -m) - authTypeBCrypt // BCrypt adapative password hashing (htpasswd -B) - authTypeCrypt // System crypt() hashes. (htpasswd -d) -) - -var bcryptPrefixRegexp = regexp.MustCompile(`^\$2[ab]?y\$`) - -// detectAuthCredentialType inspects the credential and resolves the encryption scheme. -func detectAuthCredentialType(cred string) authType { - if strings.HasPrefix(cred, "{SHA}") { - return authTypeSHA1 +// newHTPasswd parses the reader and returns an htpasswd or an error. +func newHTPasswd(rd io.Reader) (*htpasswd, error) { + entries, err := parseHTPasswd(rd) + if err != nil { + return nil, err } - if strings.HasPrefix(cred, "$apr1$") { - return authTypeApacheMD5 - } - if bcryptPrefixRegexp.MatchString(cred) { - return authTypeBCrypt - } - // There's just not a great way to distinguish between these next two... - if len(cred) == 13 { - return authTypeCrypt - } - return authTypePlainText -} -// String Returns a text representation of the AuthType -func (at authType) String() string { - switch at { - case authTypePlainText: - return "plaintext" - case authTypeSHA1: - return "sha1" - case authTypeApacheMD5: - return "md5" - case authTypeBCrypt: - return "bcrypt" - case authTypeCrypt: - return "system crypt" - } - return "unknown" -} - -// NewHTPasswd Create a new HTPasswd with the given path to .htpasswd file. -func newHTPasswd(htpath string) *htpasswd { - return &htpasswd{path: htpath} + return &htpasswd{entries: entries}, nil } // AuthenticateUser checks a given user:password credential against the -// receiving HTPasswd's file. If the check passes, nil is returned. Note that -// this parses the htpasswd file on each request so ensure that updates are -// available. -func (htpasswd *htpasswd) authenticateUser(ctx context.Context, username string, password string) error { - // Open the file. - in, err := os.Open(htpasswd.path) +// receiving HTPasswd's file. If the check passes, nil is returned. +func (htpasswd *htpasswd) authenticateUser(username string, password string) error { + credentials, ok := htpasswd.entries[username] + if !ok { + // timing attack paranoia + bcrypt.CompareHashAndPassword([]byte{}, []byte(password)) + + return ErrAuthenticationFailure + } + + err := bcrypt.CompareHashAndPassword([]byte(credentials), []byte(password)) if err != nil { - return err - } - defer in.Close() - - for _, entry := range parseHTPasswd(ctx, in) { - if entry.username != username { - continue // wrong entry - } - - switch t := detectAuthCredentialType(entry.password); t { - case authTypeSHA1: - sha := sha1.New() - sha.Write([]byte(password)) - hash := base64.StdEncoding.EncodeToString(sha.Sum(nil)) - - if entry.password[5:] != hash { - return ErrAuthenticationFailure - } - - return nil - case authTypeBCrypt: - err := bcrypt.CompareHashAndPassword([]byte(entry.password), []byte(password)) - if err != nil { - return ErrAuthenticationFailure - } - - return nil - case authTypePlainText: - if password != entry.password { - return ErrAuthenticationFailure - } - - return nil - default: - context.GetLogger(ctx).Errorf("unsupported basic authentication type: %v", t) - } + return ErrAuthenticationFailure } - return ErrAuthenticationFailure + return nil } -// htpasswdEntry represents a line in an htpasswd file. -type htpasswdEntry struct { - username string // username, plain text - password string // stores hashed passwd -} - -// parseHTPasswd parses the contents of htpasswd. Bad entries are skipped and -// logged, so this may return empty. This will read all the entries in the -// file, whether or not they are needed. -func parseHTPasswd(ctx context.Context, rd io.Reader) []htpasswdEntry { - entries := []htpasswdEntry{} +// parseHTPasswd parses the contents of htpasswd. This will read all the +// entries in the file, whether or not they are needed. An error is returned +// if an syntax errors are encountered or if the reader fails. +func parseHTPasswd(rd io.Reader) (map[string][]byte, error) { + entries := map[string][]byte{} scanner := bufio.NewScanner(rd) + var line int for scanner.Scan() { + line++ // 1-based line numbering t := strings.TrimSpace(scanner.Text()) - i := strings.Index(t, ":") - if i < 0 || i >= len(t) { - context.GetLogger(ctx).Errorf("bad entry in htpasswd: %q", t) + + if len(t) < 1 { continue } - entries = append(entries, htpasswdEntry{ - username: t[:i], - password: t[i+1:], - }) + // lines that *begin* with a '#' are considered comments + if t[0] == '#' { + continue + } + + i := strings.Index(t, ":") + if i < 0 || i >= len(t) { + return nil, fmt.Errorf("htpasswd: invalid entry at line %d: %q", line, scanner.Text()) + } + + entries[t[:i]] = []byte(t[i+1:]) } - return entries + if err := scanner.Err(); err != nil { + return nil, err + } + + return entries, nil } diff --git a/docs/auth/basic/htpasswd_test.go b/docs/auth/basic/htpasswd_test.go new file mode 100644 index 00000000..5cc86126 --- /dev/null +++ b/docs/auth/basic/htpasswd_test.go @@ -0,0 +1,85 @@ +package basic + +import ( + "fmt" + "reflect" + "strings" + "testing" +) + +func TestParseHTPasswd(t *testing.T) { + + for _, tc := range []struct { + desc string + input string + err error + entries map[string][]byte + }{ + { + desc: "basic example", + input: ` +# This is a comment in a basic example. +bilbo:{SHA}5siv5c0SHx681xU6GiSx9ZQryqs= +frodo:$2y$05$926C3y10Quzn/LnqQH86VOEVh/18T6RnLaS.khre96jLNL/7e.K5W +MiShil:$2y$05$0oHgwMehvoe8iAWS8I.7l.KoECXrwVaC16RPfaSCU5eVTFrATuMI2 +DeokMan:공주님 +`, + entries: map[string][]byte{ + "bilbo": []byte("{SHA}5siv5c0SHx681xU6GiSx9ZQryqs="), + "frodo": []byte("$2y$05$926C3y10Quzn/LnqQH86VOEVh/18T6RnLaS.khre96jLNL/7e.K5W"), + "MiShil": []byte("$2y$05$0oHgwMehvoe8iAWS8I.7l.KoECXrwVaC16RPfaSCU5eVTFrATuMI2"), + "DeokMan": []byte("공주님"), + }, + }, + { + desc: "ensures comments are filtered", + input: ` +# asdf:asdf +`, + }, + { + desc: "ensure midline hash is not comment", + input: ` +asdf:as#df +`, + entries: map[string][]byte{ + "asdf": []byte("as#df"), + }, + }, + { + desc: "ensure midline hash is not comment", + input: ` +# A valid comment +valid:entry +asdf +`, + err: fmt.Errorf(`htpasswd: invalid entry at line 4: "asdf"`), + }, + } { + + entries, err := parseHTPasswd(strings.NewReader(tc.input)) + if err != tc.err { + if tc.err == nil { + t.Fatalf("%s: unexpected error: %v", tc.desc, err) + } else { + if err.Error() != tc.err.Error() { // use string equality here. + t.Fatalf("%s: expected error not returned: %v != %v", tc.desc, err, tc.err) + } + } + } + + if tc.err != nil { + continue // don't test output + } + + // allow empty and nil to be equal + if tc.entries == nil { + tc.entries = map[string][]byte{} + } + + if !reflect.DeepEqual(entries, tc.entries) { + t.Fatalf("%s: entries not parsed correctly: %v != %v", tc.desc, entries, tc.entries) + } + } + +} diff --git a/docs/handlers/app.go b/docs/handlers/app.go index 2f37aa53..08c1c004 100644 --- a/docs/handlers/app.go +++ b/docs/handlers/app.go @@ -147,6 +147,7 @@ func NewApp(ctx context.Context, configuration configuration.Configuration) *App panic(fmt.Sprintf("unable to configure authorization (%s): %v", authType, err)) } app.accessController = accessController + ctxu.GetLogger(app).Debugf("configured %q access controller", authType) } return app From e667be389a1700c6c98be72405879d755a7003f4 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Wed, 10 Jun 2015 19:40:05 -0700 Subject: [PATCH 10/11] Rename the basic access controller to htpasswd Signed-off-by: Stephen J Day --- docs/auth/{basic => htpasswd}/access.go | 10 +++++----- docs/auth/{basic => htpasswd}/access_test.go | 2 +- docs/auth/{basic => htpasswd}/htpasswd.go | 2 +- docs/auth/{basic => htpasswd}/htpasswd_test.go | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) rename docs/auth/{basic => htpasswd}/access.go (88%) rename docs/auth/{basic => htpasswd}/access_test.go (99%) rename docs/auth/{basic => htpasswd}/htpasswd.go (99%) rename docs/auth/{basic => htpasswd}/htpasswd_test.go (99%) diff --git a/docs/auth/basic/access.go b/docs/auth/htpasswd/access.go similarity index 88% rename from docs/auth/basic/access.go rename to docs/auth/htpasswd/access.go index f7d5e79b..5425b1da 100644 --- a/docs/auth/basic/access.go +++ b/docs/auth/htpasswd/access.go @@ -1,9 +1,9 @@ -// Package basic provides a simple authentication scheme that checks for the +// Package htpasswd provides a simple authentication scheme that checks for the // user credential hash in an htpasswd formatted file in a configuration-determined // location. // // This authentication method MUST be used under TLS, as simple token-replay attack is possible. -package basic +package htpasswd import ( "errors" @@ -34,12 +34,12 @@ var _ auth.AccessController = &accessController{} func newAccessController(options map[string]interface{}) (auth.AccessController, error) { realm, present := options["realm"] if _, ok := realm.(string); !present || !ok { - return nil, fmt.Errorf(`"realm" must be set for basic access controller`) + return nil, fmt.Errorf(`"realm" must be set for htpasswd access controller`) } path, present := options["path"] if _, ok := path.(string); !present || !ok { - return nil, fmt.Errorf(`"path" must be set for basic access controller`) + return nil, fmt.Errorf(`"path" must be set for htpasswd access controller`) } f, err := os.Open(path.(string)) @@ -98,5 +98,5 @@ func (ch *challenge) Error() string { } func init() { - auth.Register("basic", auth.InitFunc(newAccessController)) + auth.Register("htpasswd", auth.InitFunc(newAccessController)) } diff --git a/docs/auth/basic/access_test.go b/docs/auth/htpasswd/access_test.go similarity index 99% rename from docs/auth/basic/access_test.go rename to docs/auth/htpasswd/access_test.go index 1976b32e..5cb2d7c9 100644 --- a/docs/auth/basic/access_test.go +++ b/docs/auth/htpasswd/access_test.go @@ -1,4 +1,4 @@ -package basic +package htpasswd import ( "io/ioutil" diff --git a/docs/auth/basic/htpasswd.go b/docs/auth/htpasswd/htpasswd.go similarity index 99% rename from docs/auth/basic/htpasswd.go rename to docs/auth/htpasswd/htpasswd.go index dd9bb1ac..494ad0a7 100644 --- a/docs/auth/basic/htpasswd.go +++ b/docs/auth/htpasswd/htpasswd.go @@ -1,4 +1,4 @@ -package basic +package htpasswd import ( "bufio" diff --git a/docs/auth/basic/htpasswd_test.go b/docs/auth/htpasswd/htpasswd_test.go similarity index 99% rename from docs/auth/basic/htpasswd_test.go rename to docs/auth/htpasswd/htpasswd_test.go index 5cc86126..309c359a 100644 --- a/docs/auth/basic/htpasswd_test.go +++ b/docs/auth/htpasswd/htpasswd_test.go @@ -1,4 +1,4 @@ -package basic +package htpasswd import ( "fmt" From f6ee0f46af41827082ee63ab261b6ddcbe4aa807 Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Thu, 11 Jun 2015 17:06:35 -0700 Subject: [PATCH 11/11] Minor formatting fixes related to htpasswd auth Signed-off-by: Stephen J Day --- docs/auth/htpasswd/access_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/auth/htpasswd/access_test.go b/docs/auth/htpasswd/access_test.go index 5cb2d7c9..ea0de425 100644 --- a/docs/auth/htpasswd/access_test.go +++ b/docs/auth/htpasswd/access_test.go @@ -85,8 +85,8 @@ func TestBasicAccessController(t *testing.T) { } nonbcrypt := map[string]struct{}{ - "bilbo": struct{}{}, - "DeokMan": struct{}{}, + "bilbo": {}, + "DeokMan": {}, } for i := 0; i < len(testUsers); i++ {