From 72f21046a21c2fe404634e7a93f128105bcd75d2 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Tue, 4 Jan 2022 16:14:32 +0800 Subject: [PATCH 01/34] refactor repeated code --- pkg/filter/validator/validator.go | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/pkg/filter/validator/validator.go b/pkg/filter/validator/validator.go index c59636cf27..a1e8a7cd83 100644 --- a/pkg/filter/validator/validator.go +++ b/pkg/filter/validator/validator.go @@ -120,38 +120,35 @@ func (v *Validator) Handle(ctx context.HTTPContext) string { func (v *Validator) handle(ctx context.HTTPContext) string { req := ctx.Request() + prepareErrorResponse := func(status int, tagPrefix string, err error) { + ctx.Response().SetStatusCode(status) + ctx.AddTag(stringtool.Cat(tagPrefix, err.Error())) + } + if v.headers != nil { - err := v.headers.Validate(req.Header()) - if err != nil { - ctx.Response().SetStatusCode(http.StatusBadRequest) - ctx.AddTag(stringtool.Cat("header validator: ", err.Error())) + if err := v.headers.Validate(req.Header()); err != nil { + prepareErrorResponse(http.StatusBadRequest, "header validator: ", err) return resultInvalid } } if v.jwt != nil { - err := v.jwt.Validate(req) - if err != nil { - ctx.Response().SetStatusCode(http.StatusUnauthorized) - ctx.AddTag(stringtool.Cat("JWT validator: ", err.Error())) + if err := v.jwt.Validate(req); err != nil { + prepareErrorResponse(http.StatusUnauthorized, "JWT validator: ", err) return resultInvalid } } if v.signer != nil { - err := v.signer.Verify(req.Std()) - if err != nil { - ctx.Response().SetStatusCode(http.StatusUnauthorized) - ctx.AddTag(stringtool.Cat("signature validator: ", err.Error())) + if err := v.signer.Verify(req.Std()); err != nil { + prepareErrorResponse(http.StatusUnauthorized, "signature validator: ", err) return resultInvalid } } if v.oauth2 != nil { - err := v.oauth2.Validate(req) - if err != nil { - ctx.Response().SetStatusCode(http.StatusUnauthorized) - ctx.AddTag(stringtool.Cat("oauth2 validator: ", err.Error())) + if err := v.oauth2.Validate(req); err != nil { + prepareErrorResponse(http.StatusUnauthorized, "oauth2 validator: ", err) return resultInvalid } } From 791272e97b88c6644198b0e8f0a6845a2e51b4c0 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Wed, 5 Jan 2022 11:40:49 +0800 Subject: [PATCH 02/34] basicauth validator using userFile --- pkg/filter/validator/basicauth.go | 120 +++++++++++++++++++++++++ pkg/filter/validator/oauth2.go | 18 ++-- pkg/filter/validator/validator.go | 27 ++++-- pkg/filter/validator/validator_test.go | 67 +++++++++++++- 4 files changed, 220 insertions(+), 12 deletions(-) create mode 100644 pkg/filter/validator/basicauth.go diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go new file mode 100644 index 0000000000..86989e84b8 --- /dev/null +++ b/pkg/filter/validator/basicauth.go @@ -0,0 +1,120 @@ +/* + * Copyright (c) 2017, MegaEase + * All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package validator + +import ( + "os" + "bufio" + "fmt" + "encoding/base64" + "strings" + + "github.com/megaease/easegress/pkg/context" +) + +// BasicAuthValidatorSpec defines the configuration of Basic Auth validator. +// Only one of UserFile or UseEtcd should be defined. +type BasicAuthValidatorSpec struct { + // UserFile is path to file containing encrypted user credentials in apache2-utils/htpasswd format. + // To add user `userY`, use `sudo htpasswd /etc/apache2/.htpasswd userY` + // Reference: https://manpages.debian.org/testing/apache2-utils/htpasswd.1.en.html#EXAMPLES + UserFile string `yaml:"userFile" jsonschema:"omitempty"` + // When UseEtcd is true, verify user credentials from etcd. + // TODO add some details about this. + UseEtcd bool `yaml:"useEtcd" jsonschema:"omitempty"` +} + +type AuthorizedUsers struct { + nameToToken map[string]string +} + +func NewAuthorizedUsers() *AuthorizedUsers { + return &AuthorizedUsers{ + nameToToken: make(map[string]string), + } +} + +func (au *AuthorizedUsers) Validate(user string, token string) error { + if expectedToken, ok := au.nameToToken[user]; ok && expectedToken == token { + return nil + } + return fmt.Errorf("unauthorized") +} + +func parseCredentials(creds string) (string, string, error) { + parts := strings.Split(creds, ":") + if len(parts) < 2 { + return "", "", fmt.Errorf("bad format") + } + return parts[0], parts[1], nil +} + +func (au *AuthorizedUsers) UpdateFromUserFile(fileName string) error { + file, err := os.OpenFile(fileName, os.O_RDONLY, os.ModePerm) + if err != nil { + return fmt.Errorf("open file error: %v", err) + } + defer file.Close() + sc := bufio.NewScanner(file) + for sc.Scan() { + line := sc.Text() + name, token, err := parseCredentials(line) + if err != nil { + return err + } + au.nameToToken[name] = token + } + return nil +} + +// NewBasicAuthValidator creates a new Basic Auth validator +func NewBasicAuthValidator(spec *BasicAuthValidatorSpec) *BasicAuthValidator { + au := NewAuthorizedUsers() + if spec.UserFile != "" { + au.UpdateFromUserFile(spec.UserFile) + } + return &BasicAuthValidator{ + spec: spec, + authorizedUsers: au, + } +} + +// BasicAuthValidator defines the Basic Auth validator +type BasicAuthValidator struct { + spec *BasicAuthValidatorSpec + authorizedUsers *AuthorizedUsers +} + +// Validate validates the Authorization header of a http request +func (bav *BasicAuthValidator) Validate(req context.HTTPRequest) error { + hdr := req.Header() + base64credentials, err := parseAuthorizationHeader(hdr) + if err != nil { + return err + } + credentialBytes, err := base64.StdEncoding.DecodeString(base64credentials) + if err != nil { + return fmt.Errorf("error occured during base64 decode: %s", err.Error()) + } + credentials := string(credentialBytes) + name, token, err := parseCredentials(credentials) + if err != nil { + return fmt.Errorf("unauthorized") + } + return bav.authorizedUsers.Validate(name, token) +} diff --git a/pkg/filter/validator/oauth2.go b/pkg/filter/validator/oauth2.go index 6274426532..cf28659c1f 100644 --- a/pkg/filter/validator/oauth2.go +++ b/pkg/filter/validator/oauth2.go @@ -29,6 +29,7 @@ import ( "github.com/golang-jwt/jwt" "github.com/megaease/easegress/pkg/context" + "github.com/megaease/easegress/pkg/util/httpheader" ) type ( @@ -137,16 +138,23 @@ func (v *OAuth2Validator) introspectToken(tokenStr string) (*tokenInfo, error) { return &ti.tokenInfo, nil } -// Validate validates the access token of a http request -func (v *OAuth2Validator) Validate(req context.HTTPRequest) error { +func parseAuthorizationHeader(hdr *httpheader.HTTPHeader) (string, error) { const prefix = "Bearer " - hdr := req.Header() tokenStr := hdr.Get("Authorization") if !strings.HasPrefix(tokenStr, prefix) { - return fmt.Errorf("unexpected authorization header: %s", tokenStr) + return "", fmt.Errorf("unexpected authorization header: %s", tokenStr) + } + return tokenStr[len(prefix):], nil +} + +// Validate validates the access token of a http request +func (v *OAuth2Validator) Validate(req context.HTTPRequest) error { + hdr := req.Header() + tokenStr, err := parseAuthorizationHeader(hdr) + if err != nil { + return err } - tokenStr = tokenStr[len(prefix):] var subject, scope string if v.spec.TokenIntrospect != nil { diff --git a/pkg/filter/validator/validator.go b/pkg/filter/validator/validator.go index a1e8a7cd83..8868addc61 100644 --- a/pkg/filter/validator/validator.go +++ b/pkg/filter/validator/validator.go @@ -20,6 +20,8 @@ package validator import ( "net/http" + "fmt" + "github.com/megaease/easegress/pkg/context" "github.com/megaease/easegress/pkg/object/httppipeline" "github.com/megaease/easegress/pkg/util/httpheader" @@ -50,6 +52,7 @@ type ( jwt *JWTValidator signer *signer.Signer oauth2 *OAuth2Validator + basicAuth *BasicAuthValidator } // Spec describes the Validator. @@ -58,9 +61,18 @@ type ( JWT *JWTValidatorSpec `yaml:"jwt,omitempty" jsonschema:"omitempty"` Signature *signer.Spec `yaml:"signature,omitempty" jsonschema:"omitempty"` OAuth2 *OAuth2ValidatorSpec `yaml:"oauth2,omitempty" jsonschema:"omitempty"` + BasicAuth *BasicAuthValidatorSpec `yaml:"basicAuth,omitempty" jsonschema:"omitempty"` } ) +// Validate verifies that at least one of the validations is defined. +func (spec Spec) Validate() error { + if spec == (Spec{}) { + return fmt.Errorf("none of the validations are defined") + } + return nil +} + // Kind returns the kind of Validator. func (v *Validator) Kind() string { return Kind @@ -97,18 +109,18 @@ func (v *Validator) reload() { if v.spec.Headers != nil { v.headers = httpheader.NewValidator(v.spec.Headers) } - if v.spec.JWT != nil { v.jwt = NewJWTValidator(v.spec.JWT) } - if v.spec.Signature != nil { v.signer = signer.CreateFromSpec(v.spec.Signature) } - if v.spec.OAuth2 != nil { v.oauth2 = NewOAuth2Validator(v.spec.OAuth2) } + if v.spec.BasicAuth != nil { + v.basicAuth = NewBasicAuthValidator(v.spec.BasicAuth) + } } // Handle validates HTTPContext. @@ -131,27 +143,30 @@ func (v *Validator) handle(ctx context.HTTPContext) string { return resultInvalid } } - if v.jwt != nil { if err := v.jwt.Validate(req); err != nil { prepareErrorResponse(http.StatusUnauthorized, "JWT validator: ", err) return resultInvalid } } - if v.signer != nil { if err := v.signer.Verify(req.Std()); err != nil { prepareErrorResponse(http.StatusUnauthorized, "signature validator: ", err) return resultInvalid } } - if v.oauth2 != nil { if err := v.oauth2.Validate(req); err != nil { prepareErrorResponse(http.StatusUnauthorized, "oauth2 validator: ", err) return resultInvalid } } + if v.basicAuth != nil { + if err := v.basicAuth.Validate(req); err != nil { + prepareErrorResponse(http.StatusUnauthorized, "http basic validator: ", err) + return resultInvalid + } + } return "" } diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 054abb9811..8affebbcaa 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -19,6 +19,7 @@ package validator import ( "fmt" + "encoding/base64" "io" "net/http" "os" @@ -41,7 +42,10 @@ func TestMain(m *testing.M) { func createValidator(yamlSpec string, prev *Validator) *Validator { rawSpec := make(map[string]interface{}) yamltool.Unmarshal([]byte(yamlSpec), &rawSpec) - spec, _ := httppipeline.NewFilterSpec(rawSpec, nil) + spec, err := httppipeline.NewFilterSpec(rawSpec, nil) + if err != nil { + panic(err.Error()) + } v := &Validator{} if prev == nil { v.Init(spec) @@ -289,3 +293,64 @@ signature: t.Errorf("OAuth/2 Authorization should fail") } } + +func check(e error) { + if e != nil { + panic(e) + } +} + +func TestHTTPBasicAuth(t *testing.T) { + userFile, err := os.CreateTemp("/tmp/", "apache2-htpasswd") + check(err) + + defer os.Remove(userFile.Name()) + + yamlSpec := ` +kind: Validator +name: validator +basicAuth: + userFile: ` + userFile.Name() + + credentials1 := "userY:md5-encrypted-pw-1" + credentials2 := "userZ:md5-encrypted-pw-2" + userFile.Write([]byte(credentials1 + "\n" + credentials2)) + + v := createValidator(yamlSpec, nil) + // userY + ctx := &contexttest.MockedHTTPContext{} + header := http.Header{} + ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { + return httpheader.New(header) + } + b64creds := base64.StdEncoding.EncodeToString([]byte(credentials1)) + header.Set("Authorization", "Bearer "+b64creds) + result := v.Handle(ctx) + if result == resultInvalid { + t.Errorf("the basic auth head should be valid") + } + // userZ + ctx = &contexttest.MockedHTTPContext{} + header = http.Header{} + ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { + return httpheader.New(header) + } + b64creds = base64.StdEncoding.EncodeToString([]byte(credentials2)) + header.Set("Authorization", "Bearer "+b64creds) + result = v.Handle(ctx) + if result == resultInvalid { + t.Errorf("the basic auth head should be valid") + } + // nonExistingUser + ctx = &contexttest.MockedHTTPContext{} + header = http.Header{} + ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { + return httpheader.New(header) + } + b64creds = base64.StdEncoding.EncodeToString([]byte("nonExistingUser:md5-encrypted-pw3")) + header.Set("Authorization", "Bearer "+b64creds) + result = v.Handle(ctx) + if result != resultInvalid { + t.Errorf("the basic auth head should be invalid") + } +} From 484789926edb0dd11dac1783019acabdd3c8da36 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Wed, 5 Jan 2022 11:43:17 +0800 Subject: [PATCH 03/34] format code --- pkg/filter/validator/basicauth.go | 26 +++++++++++++------------- pkg/filter/validator/validator.go | 10 +++++----- pkg/filter/validator/validator_test.go | 10 +++++----- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index 86989e84b8..c216e8957e 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -18,10 +18,10 @@ package validator import ( - "os" "bufio" - "fmt" "encoding/base64" + "fmt" + "os" "strings" "github.com/megaease/easegress/pkg/context" @@ -40,7 +40,7 @@ type BasicAuthValidatorSpec struct { } type AuthorizedUsers struct { - nameToToken map[string]string + nameToToken map[string]string } func NewAuthorizedUsers() *AuthorizedUsers { @@ -66,19 +66,19 @@ func parseCredentials(creds string) (string, string, error) { func (au *AuthorizedUsers) UpdateFromUserFile(fileName string) error { file, err := os.OpenFile(fileName, os.O_RDONLY, os.ModePerm) - if err != nil { - return fmt.Errorf("open file error: %v", err) - } - defer file.Close() + if err != nil { + return fmt.Errorf("open file error: %v", err) + } + defer file.Close() sc := bufio.NewScanner(file) - for sc.Scan() { - line := sc.Text() + for sc.Scan() { + line := sc.Text() name, token, err := parseCredentials(line) if err != nil { return err } au.nameToToken[name] = token - } + } return nil } @@ -89,15 +89,15 @@ func NewBasicAuthValidator(spec *BasicAuthValidatorSpec) *BasicAuthValidator { au.UpdateFromUserFile(spec.UserFile) } return &BasicAuthValidator{ - spec: spec, + spec: spec, authorizedUsers: au, } } // BasicAuthValidator defines the Basic Auth validator type BasicAuthValidator struct { - spec *BasicAuthValidatorSpec - authorizedUsers *AuthorizedUsers + spec *BasicAuthValidatorSpec + authorizedUsers *AuthorizedUsers } // Validate validates the Authorization header of a http request diff --git a/pkg/filter/validator/validator.go b/pkg/filter/validator/validator.go index 8868addc61..876100b76b 100644 --- a/pkg/filter/validator/validator.go +++ b/pkg/filter/validator/validator.go @@ -48,10 +48,10 @@ type ( filterSpec *httppipeline.FilterSpec spec *Spec - headers *httpheader.Validator - jwt *JWTValidator - signer *signer.Signer - oauth2 *OAuth2Validator + headers *httpheader.Validator + jwt *JWTValidator + signer *signer.Signer + oauth2 *OAuth2Validator basicAuth *BasicAuthValidator } @@ -61,7 +61,7 @@ type ( JWT *JWTValidatorSpec `yaml:"jwt,omitempty" jsonschema:"omitempty"` Signature *signer.Spec `yaml:"signature,omitempty" jsonschema:"omitempty"` OAuth2 *OAuth2ValidatorSpec `yaml:"oauth2,omitempty" jsonschema:"omitempty"` - BasicAuth *BasicAuthValidatorSpec `yaml:"basicAuth,omitempty" jsonschema:"omitempty"` + BasicAuth *BasicAuthValidatorSpec `yaml:"basicAuth,omitempty" jsonschema:"omitempty"` } ) diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 8affebbcaa..4ed494eece 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -18,8 +18,8 @@ package validator import ( - "fmt" "encoding/base64" + "fmt" "io" "net/http" "os" @@ -295,14 +295,14 @@ signature: } func check(e error) { - if e != nil { - panic(e) - } + if e != nil { + panic(e) + } } func TestHTTPBasicAuth(t *testing.T) { userFile, err := os.CreateTemp("/tmp/", "apache2-htpasswd") - check(err) + check(err) defer os.Remove(userFile.Name()) From 0d4644f9fad896f50a20ec22705bc19e33e552b5 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Wed, 5 Jan 2022 17:09:29 +0800 Subject: [PATCH 04/34] basic auth validator using etcd; cache results to LRU cache --- pkg/filter/validator/basicauth.go | 111 ++++++++++----- pkg/filter/validator/validator.go | 2 +- pkg/filter/validator/validator_test.go | 183 +++++++++++++++++-------- 3 files changed, 204 insertions(+), 92 deletions(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index c216e8957e..1e5d0b4d03 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -24,7 +24,12 @@ import ( "os" "strings" + lru "github.com/hashicorp/golang-lru" + + "github.com/megaease/easegress/pkg/cluster" "github.com/megaease/easegress/pkg/context" + "github.com/megaease/easegress/pkg/logger" + "github.com/megaease/easegress/pkg/supervisor" ) // BasicAuthValidatorSpec defines the configuration of Basic Auth validator. @@ -34,26 +39,17 @@ type BasicAuthValidatorSpec struct { // To add user `userY`, use `sudo htpasswd /etc/apache2/.htpasswd userY` // Reference: https://manpages.debian.org/testing/apache2-utils/htpasswd.1.en.html#EXAMPLES UserFile string `yaml:"userFile" jsonschema:"omitempty"` - // When UseEtcd is true, verify user credentials from etcd. - // TODO add some details about this. + // When UseEtcd is true, verify user credentials from etcd. Etcd stores them: + // key: /credentials/{user id} + // value: {encrypted password} UseEtcd bool `yaml:"useEtcd" jsonschema:"omitempty"` } -type AuthorizedUsers struct { - nameToToken map[string]string -} - -func NewAuthorizedUsers() *AuthorizedUsers { - return &AuthorizedUsers{ - nameToToken: make(map[string]string), - } -} - -func (au *AuthorizedUsers) Validate(user string, token string) error { - if expectedToken, ok := au.nameToToken[user]; ok && expectedToken == token { - return nil - } - return fmt.Errorf("unauthorized") +// BasicAuthValidator defines the Basic Auth validator +type BasicAuthValidator struct { + spec *BasicAuthValidatorSpec + authorizedUsersCache *lru.Cache + cluster cluster.Cluster } func parseCredentials(creds string) (string, string, error) { @@ -64,40 +60,77 @@ func parseCredentials(creds string) (string, string, error) { return parts[0], parts[1], nil } -func (au *AuthorizedUsers) UpdateFromUserFile(fileName string) error { - file, err := os.OpenFile(fileName, os.O_RDONLY, os.ModePerm) +func (bav *BasicAuthValidator) getFromFile(targetUserID string) (string, error) { + file, err := os.OpenFile(bav.spec.UserFile, os.O_RDONLY, os.ModePerm) if err != nil { - return fmt.Errorf("open file error: %v", err) + return "", fmt.Errorf("open file error: %v", err) } defer file.Close() sc := bufio.NewScanner(file) for sc.Scan() { line := sc.Text() - name, token, err := parseCredentials(line) + userID, password, err := parseCredentials(line) if err != nil { - return err + return "", err + } + if userID == targetUserID { + return password, nil } - au.nameToToken[name] = token } - return nil + return "", fmt.Errorf("unauthorized") +} + +func (bav *BasicAuthValidator) getFromEtcd(targetUserID string) (string, error) { + const prefix = "/credentials/" + password, err := bav.cluster.Get(prefix + targetUserID) + if err != nil { + return "", err + } + if password == nil { + return "", fmt.Errorf("unauthorized") + } + return *password, nil } // NewBasicAuthValidator creates a new Basic Auth validator -func NewBasicAuthValidator(spec *BasicAuthValidatorSpec) *BasicAuthValidator { - au := NewAuthorizedUsers() - if spec.UserFile != "" { - au.UpdateFromUserFile(spec.UserFile) +func NewBasicAuthValidator(spec *BasicAuthValidatorSpec, supervisor *supervisor.Supervisor) *BasicAuthValidator { + var cluster cluster.Cluster + if spec.UseEtcd { + if supervisor == nil || supervisor.Cluster() == nil { + logger.Errorf("BasicAuth validator : failed to read data from etcd") + } else { + cluster = supervisor.Cluster() + } + } + cache, err := lru.New(256) + if err != nil { + panic(err) } - return &BasicAuthValidator{ - spec: spec, - authorizedUsers: au, + bav := &BasicAuthValidator{ + spec: spec, + authorizedUsersCache: cache, + cluster: cluster, } + return bav } -// BasicAuthValidator defines the Basic Auth validator -type BasicAuthValidator struct { - spec *BasicAuthValidatorSpec - authorizedUsers *AuthorizedUsers +func (bav *BasicAuthValidator) getUserFromCache(userID string) (string, bool) { + if val, ok := bav.authorizedUsersCache.Get(userID); ok { + return val.(string), true + } + var password string + var err error + if bav.spec.UserFile != "" { + password, err = bav.getFromFile(userID) + } + if bav.spec.UseEtcd == true { + password, err = bav.getFromEtcd(userID) + } + if err != nil { + return "", false + } + bav.authorizedUsersCache.Add(userID, password) + return password, true } // Validate validates the Authorization header of a http request @@ -112,9 +145,13 @@ func (bav *BasicAuthValidator) Validate(req context.HTTPRequest) error { return fmt.Errorf("error occured during base64 decode: %s", err.Error()) } credentials := string(credentialBytes) - name, token, err := parseCredentials(credentials) + userID, token, err := parseCredentials(credentials) if err != nil { return fmt.Errorf("unauthorized") } - return bav.authorizedUsers.Validate(name, token) + + if expectedToken, ok := bav.getUserFromCache(userID); ok && expectedToken == token { + return nil + } + return fmt.Errorf("unauthorized") } diff --git a/pkg/filter/validator/validator.go b/pkg/filter/validator/validator.go index 876100b76b..30e6a946b1 100644 --- a/pkg/filter/validator/validator.go +++ b/pkg/filter/validator/validator.go @@ -119,7 +119,7 @@ func (v *Validator) reload() { v.oauth2 = NewOAuth2Validator(v.spec.OAuth2) } if v.spec.BasicAuth != nil { - v.basicAuth = NewBasicAuthValidator(v.spec.BasicAuth) + v.basicAuth = NewBasicAuthValidator(v.spec.BasicAuth, v.filterSpec.Super()) } } diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 4ed494eece..2bb9523845 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -21,14 +21,22 @@ import ( "encoding/base64" "fmt" "io" + "io/ioutil" "net/http" "os" "strings" + "sync" "testing" + "github.com/phayes/freeport" + + cluster "github.com/megaease/easegress/pkg/cluster" "github.com/megaease/easegress/pkg/context/contexttest" + "github.com/megaease/easegress/pkg/env" "github.com/megaease/easegress/pkg/logger" "github.com/megaease/easegress/pkg/object/httppipeline" + "github.com/megaease/easegress/pkg/option" + "github.com/megaease/easegress/pkg/supervisor" "github.com/megaease/easegress/pkg/util/httpheader" "github.com/megaease/easegress/pkg/util/yamltool" ) @@ -39,10 +47,10 @@ func TestMain(m *testing.M) { os.Exit(code) } -func createValidator(yamlSpec string, prev *Validator) *Validator { +func createValidator(yamlSpec string, prev *Validator, supervisor *supervisor.Supervisor) *Validator { rawSpec := make(map[string]interface{}) yamltool.Unmarshal([]byte(yamlSpec), &rawSpec) - spec, err := httppipeline.NewFilterSpec(rawSpec, nil) + spec, err := httppipeline.NewFilterSpec(rawSpec, supervisor) if err != nil { panic(err.Error()) } @@ -65,7 +73,7 @@ headers: regexp: "^ok-.+$" ` - v := createValidator(yamlSpec, nil) + v := createValidator(yamlSpec, nil, nil) header := http.Header{} ctx := &contexttest.MockedHTTPContext{} @@ -106,7 +114,7 @@ jwt: algorithm: HS256 secret: 313233343536 ` - v := createValidator(yamlSpec, nil) + v := createValidator(yamlSpec, nil, nil) ctx := &contexttest.MockedHTTPContext{} ctx.MockedRequest.MockedCookie = func(name string) (*http.Cookie, error) { @@ -152,7 +160,7 @@ jwt: t.Errorf("the jwt token in cookie should be valid") } - v = createValidator(yamlSpec, v) + v = createValidator(yamlSpec, v, nil) result = v.Handle(ctx) if result == resultInvalid { t.Errorf("the jwt token in cookie should be valid") @@ -173,7 +181,7 @@ oauth2: algorithm: HS256 secret: 313233343536 ` - v := createValidator(yamlSpec, nil) + v := createValidator(yamlSpec, nil, nil) ctx := &contexttest.MockedHTTPContext{} @@ -220,7 +228,7 @@ oauth2: clientId: megaease clientSecret: secret ` - v := createValidator(yamlSpec, nil) + v := createValidator(yamlSpec, nil, nil) ctx := &contexttest.MockedHTTPContext{} header := http.Header{} @@ -257,7 +265,7 @@ oauth2: clientSecret: secret basicAuth: megaease@megaease ` - v = createValidator(yamlSpec, nil) + v = createValidator(yamlSpec, nil, nil) body = `{ "subject":"megaease.com", @@ -280,7 +288,7 @@ signature: accessKeys: AKID: SECRET ` - v := createValidator(yamlSpec, nil) + v := createValidator(yamlSpec, nil, nil) ctx := &contexttest.MockedHTTPContext{} ctx.MockedRequest.MockedStd = func() *http.Request { @@ -300,57 +308,124 @@ func check(e error) { } } -func TestHTTPBasicAuth(t *testing.T) { - userFile, err := os.CreateTemp("/tmp/", "apache2-htpasswd") +func createCluster(tempDir string) cluster.Cluster { + ports, err := freeport.GetFreePorts(3) + check(err) + name := fmt.Sprintf("test-member-x") + opt := option.New() + opt.Name = name + opt.ClusterName = "test-cluster" + opt.ClusterRole = "primary" + opt.ClusterRequestTimeout = "10s" + opt.Cluster.ListenClientURLs = []string{fmt.Sprintf("http://localhost:%d", ports[0])} + opt.Cluster.AdvertiseClientURLs = opt.Cluster.ListenClientURLs + opt.Cluster.ListenPeerURLs = []string{fmt.Sprintf("http://localhost:%d", ports[1])} + opt.Cluster.InitialAdvertisePeerURLs = opt.Cluster.ListenPeerURLs + opt.Cluster.InitialCluster = make(map[string]string) + opt.Cluster.InitialCluster[name] = opt.Cluster.InitialAdvertisePeerURLs[0] + opt.APIAddr = fmt.Sprintf("localhost:%d", ports[2]) + opt.DataDir = fmt.Sprintf("%s/data", tempDir) + opt.LogDir = fmt.Sprintf("%s/log", tempDir) + opt.MemberDir = fmt.Sprintf("%s/member", tempDir) + + _, err = opt.Parse() check(err) - defer os.Remove(userFile.Name()) + env.InitServerDir(opt) - yamlSpec := ` + clusterInstance, err := cluster.New(opt) + check(err) + return clusterInstance +} + +func TestBasicAuthUser(t *testing.T) { + userIds := []string{ + "userY", "userZ", "nonExistingUser", + } + passwords := []string{ + "md5-encrypted-pw-1", "md5-encrypted-pw-2", "md5-encrypted-pw3", + } + t.Run("credentials from userFile", func(t *testing.T) { + userFile, err := os.CreateTemp("/tmp/", "apache2-htpasswd") + check(err) + + defer os.Remove(userFile.Name()) + + yamlSpec := ` kind: Validator name: validator basicAuth: userFile: ` + userFile.Name() - credentials1 := "userY:md5-encrypted-pw-1" - credentials2 := "userZ:md5-encrypted-pw-2" - userFile.Write([]byte(credentials1 + "\n" + credentials2)) - - v := createValidator(yamlSpec, nil) - // userY - ctx := &contexttest.MockedHTTPContext{} - header := http.Header{} - ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { - return httpheader.New(header) - } - b64creds := base64.StdEncoding.EncodeToString([]byte(credentials1)) - header.Set("Authorization", "Bearer "+b64creds) - result := v.Handle(ctx) - if result == resultInvalid { - t.Errorf("the basic auth head should be valid") - } - // userZ - ctx = &contexttest.MockedHTTPContext{} - header = http.Header{} - ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { - return httpheader.New(header) - } - b64creds = base64.StdEncoding.EncodeToString([]byte(credentials2)) - header.Set("Authorization", "Bearer "+b64creds) - result = v.Handle(ctx) - if result == resultInvalid { - t.Errorf("the basic auth head should be valid") - } - // nonExistingUser - ctx = &contexttest.MockedHTTPContext{} - header = http.Header{} - ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { - return httpheader.New(header) - } - b64creds = base64.StdEncoding.EncodeToString([]byte("nonExistingUser:md5-encrypted-pw3")) - header.Set("Authorization", "Bearer "+b64creds) - result = v.Handle(ctx) - if result != resultInvalid { - t.Errorf("the basic auth head should be invalid") - } + userFile.Write([]byte(userIds[0] + ":" + passwords[0] + "\n" + userIds[1] + ":" + passwords[1])) + expectedValid := []bool{true, true, false} + + v := createValidator(yamlSpec, nil, nil) + for i := 0; i < 3; i++ { + ctx := &contexttest.MockedHTTPContext{} + header := http.Header{} + ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { + return httpheader.New(header) + } + b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[i] + ":" + passwords[i])) + header.Set("Authorization", "Bearer "+b64creds) + result := v.Handle(ctx) + if expectedValid[i] { + if result == resultInvalid { + t.Errorf("the basic auth head should be valid") + } + } else { + if result != resultInvalid { + t.Errorf("the basic auth head should be invalid") + } + } + } + }) + t.Run("credentials from etcd", func(t *testing.T) { + etcdDirName, err := ioutil.TempDir("", "etcd-validator-test") + check(err) + defer os.RemoveAll(etcdDirName) + clusterInstance := createCluster(etcdDirName) + + clusterInstance.Put("/credentials/"+userIds[0], passwords[0]) + clusterInstance.Put("/credentials/"+userIds[2], passwords[2]) + + var mockMap sync.Map + supervisor := supervisor.NewMock( + nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil) + + yamlSpec := ` +kind: Validator +name: validator +basicAuth: + useEtcd: true` + + expectedValid := []bool{true, false, true} + + v := createValidator(yamlSpec, nil, supervisor) + + for i := 0; i < 3; i++ { + ctx := &contexttest.MockedHTTPContext{} + header := http.Header{} + ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { + return httpheader.New(header) + } + b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[i] + ":" + passwords[i])) + header.Set("Authorization", "Bearer "+b64creds) + result := v.Handle(ctx) + if expectedValid[i] { + if result == resultInvalid { + t.Errorf("the basic auth head should be valid") + } + } else { + if result != resultInvalid { + t.Errorf("the basic auth head should be invalid") + } + } + } + wg := &sync.WaitGroup{} + wg.Add(1) + clusterInstance.CloseServer(wg) + wg.Wait() + }) } From 30944df3c089d8d60c7dc6899ddb0a0dde8c939e Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Wed, 5 Jan 2022 17:39:49 +0800 Subject: [PATCH 05/34] fix header parsing --- pkg/filter/validator/basicauth.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index 1e5d0b4d03..2154d746d8 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -30,6 +30,7 @@ import ( "github.com/megaease/easegress/pkg/context" "github.com/megaease/easegress/pkg/logger" "github.com/megaease/easegress/pkg/supervisor" + "github.com/megaease/easegress/pkg/util/httpheader" ) // BasicAuthValidatorSpec defines the configuration of Basic Auth validator. @@ -133,10 +134,20 @@ func (bav *BasicAuthValidator) getUserFromCache(userID string) (string, bool) { return password, true } +func parseBasicAuthorizationHeader(hdr *httpheader.HTTPHeader) (string, error) { + const prefix = "Basic " + + tokenStr := hdr.Get("Authorization") + if !strings.HasPrefix(tokenStr, prefix) { + return "", fmt.Errorf("unexpected authorization header: %s", tokenStr) + } + return tokenStr[len(prefix):], nil +} + // Validate validates the Authorization header of a http request func (bav *BasicAuthValidator) Validate(req context.HTTPRequest) error { hdr := req.Header() - base64credentials, err := parseAuthorizationHeader(hdr) + base64credentials, err := parseBasicAuthorizationHeader(hdr) if err != nil { return err } From bc8f83a7015b1c40eff78dd8cfb2fd901e29ee79 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Wed, 5 Jan 2022 17:50:25 +0800 Subject: [PATCH 06/34] reset changes in oauth2 --- pkg/filter/validator/oauth2.go | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/pkg/filter/validator/oauth2.go b/pkg/filter/validator/oauth2.go index cf28659c1f..6274426532 100644 --- a/pkg/filter/validator/oauth2.go +++ b/pkg/filter/validator/oauth2.go @@ -29,7 +29,6 @@ import ( "github.com/golang-jwt/jwt" "github.com/megaease/easegress/pkg/context" - "github.com/megaease/easegress/pkg/util/httpheader" ) type ( @@ -138,23 +137,16 @@ func (v *OAuth2Validator) introspectToken(tokenStr string) (*tokenInfo, error) { return &ti.tokenInfo, nil } -func parseAuthorizationHeader(hdr *httpheader.HTTPHeader) (string, error) { +// Validate validates the access token of a http request +func (v *OAuth2Validator) Validate(req context.HTTPRequest) error { const prefix = "Bearer " + hdr := req.Header() tokenStr := hdr.Get("Authorization") if !strings.HasPrefix(tokenStr, prefix) { - return "", fmt.Errorf("unexpected authorization header: %s", tokenStr) - } - return tokenStr[len(prefix):], nil -} - -// Validate validates the access token of a http request -func (v *OAuth2Validator) Validate(req context.HTTPRequest) error { - hdr := req.Header() - tokenStr, err := parseAuthorizationHeader(hdr) - if err != nil { - return err + return fmt.Errorf("unexpected authorization header: %s", tokenStr) } + tokenStr = tokenStr[len(prefix):] var subject, scope string if v.spec.TokenIntrospect != nil { From 9316bfa14b0d9721057238dff2c64e4f6f0c617a Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 6 Jan 2022 08:08:34 +0800 Subject: [PATCH 07/34] fix test --- pkg/filter/validator/validator_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 2bb9523845..ab86328331 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -368,7 +368,7 @@ basicAuth: return httpheader.New(header) } b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[i] + ":" + passwords[i])) - header.Set("Authorization", "Bearer "+b64creds) + header.Set("Authorization", "Basic "+b64creds) result := v.Handle(ctx) if expectedValid[i] { if result == resultInvalid { @@ -411,7 +411,7 @@ basicAuth: return httpheader.New(header) } b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[i] + ":" + passwords[i])) - header.Set("Authorization", "Bearer "+b64creds) + header.Set("Authorization", "Basic "+b64creds) result := v.Handle(ctx) if expectedValid[i] { if result == resultInvalid { From 5c81d4293a64bb33c4f121826fb3055969f24489 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 6 Jan 2022 08:34:14 +0800 Subject: [PATCH 08/34] remove unix specific temp file path --- pkg/filter/validator/validator_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index ab86328331..f75d877e48 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -346,7 +346,7 @@ func TestBasicAuthUser(t *testing.T) { "md5-encrypted-pw-1", "md5-encrypted-pw-2", "md5-encrypted-pw3", } t.Run("credentials from userFile", func(t *testing.T) { - userFile, err := os.CreateTemp("/tmp/", "apache2-htpasswd") + userFile, err := os.CreateTemp("", "apache2-htpasswd") check(err) defer os.Remove(userFile.Name()) From de2250cedd6f8371b830b361c0fa34259951a59f Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 6 Jan 2022 11:12:47 +0800 Subject: [PATCH 09/34] sync authorized users --- pkg/filter/validator/basicauth.go | 245 +++++++++++++++++++------ pkg/filter/validator/validator.go | 8 +- pkg/filter/validator/validator_test.go | 83 +++++++-- 3 files changed, 262 insertions(+), 74 deletions(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index 2154d746d8..536f48c6d0 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -19,39 +19,67 @@ package validator import ( "bufio" + "context" "encoding/base64" "fmt" "os" "strings" + "time" lru "github.com/hashicorp/golang-lru" "github.com/megaease/easegress/pkg/cluster" - "github.com/megaease/easegress/pkg/context" + httpcontext "github.com/megaease/easegress/pkg/context" "github.com/megaease/easegress/pkg/logger" "github.com/megaease/easegress/pkg/supervisor" "github.com/megaease/easegress/pkg/util/httpheader" ) -// BasicAuthValidatorSpec defines the configuration of Basic Auth validator. -// Only one of UserFile or UseEtcd should be defined. -type BasicAuthValidatorSpec struct { - // UserFile is path to file containing encrypted user credentials in apache2-utils/htpasswd format. - // To add user `userY`, use `sudo htpasswd /etc/apache2/.htpasswd userY` - // Reference: https://manpages.debian.org/testing/apache2-utils/htpasswd.1.en.html#EXAMPLES - UserFile string `yaml:"userFile" jsonschema:"omitempty"` - // When UseEtcd is true, verify user credentials from etcd. Etcd stores them: - // key: /credentials/{user id} - // value: {encrypted password} - UseEtcd bool `yaml:"useEtcd" jsonschema:"omitempty"` -} +type ( + // BasicAuthValidatorSpec defines the configuration of Basic Auth validator. + // Only one of UserFile or UseEtcd should be defined. + BasicAuthValidatorSpec struct { + // UserFile is path to file containing encrypted user credentials in apache2-utils/htpasswd format. + // To add user `userY`, use `sudo htpasswd /etc/apache2/.htpasswd userY` + // Reference: https://manpages.debian.org/testing/apache2-utils/htpasswd.1.en.html#EXAMPLES + UserFile string `yaml:"userFile" jsonschema:"omitempty"` + // When UseEtcd is true, verify user credentials from etcd. Etcd stores them: + // key: /credentials/{user id} + // value: {encrypted password} + UseEtcd bool `yaml:"useEtcd" jsonschema:"omitempty"` + } -// BasicAuthValidator defines the Basic Auth validator -type BasicAuthValidator struct { - spec *BasicAuthValidatorSpec - authorizedUsersCache *lru.Cache - cluster cluster.Cluster -} + // AuthorizedUsersCache provides cached lookup for authorized users. + // When user is deleted from authorized users, user can still access for time period defined + // by SetSyncInterval. + AuthorizedUsersCache interface { + GetUser(string) (string, bool) + WatchChanges() error + SetSyncInterval(time.Duration) + Close() + } + + htpasswdUserCache struct { + cache *lru.Cache + userFile string + } + + etcdUserCache struct { + cache *lru.Cache + cluster cluster.Cluster + + syncerInterval time.Duration + cancel context.CancelFunc + } + + // BasicAuthValidator defines the Basic Auth validator + BasicAuthValidator struct { + spec *BasicAuthValidatorSpec + authorizedUsersCache AuthorizedUsersCache + } +) + +const credsPrefix = "/credentials/" func parseCredentials(creds string) (string, string, error) { parts := strings.Split(creds, ":") @@ -61,10 +89,26 @@ func parseCredentials(creds string) (string, string, error) { return parts[0], parts[1], nil } -func (bav *BasicAuthValidator) getFromFile(targetUserID string) (string, error) { - file, err := os.OpenFile(bav.spec.UserFile, os.O_RDONLY, os.ModePerm) +func newHtpasswdUserCache(userFile string) *htpasswdUserCache { + cache, err := lru.New(256) if err != nil { - return "", fmt.Errorf("open file error: %v", err) + panic(err) + } + return &htpasswdUserCache{ + cache: cache, + userFile: userFile, + } +} + +func (huc *htpasswdUserCache) GetUser(targetUserID string) (string, bool) { + if val, ok := huc.cache.Get(targetUserID); ok { + return val.(string), true + } + + file, err := os.OpenFile(huc.userFile, os.O_RDONLY, os.ModePerm) + if err != nil { + logger.Errorf("open file error: %v", err) + return "", false } defer file.Close() sc := bufio.NewScanner(file) @@ -72,68 +116,144 @@ func (bav *BasicAuthValidator) getFromFile(targetUserID string) (string, error) line := sc.Text() userID, password, err := parseCredentials(line) if err != nil { - return "", err + logger.Errorf(err.Error()) + return "", false } if userID == targetUserID { - return password, nil + huc.cache.Add(targetUserID, password) + return password, true } } - return "", fmt.Errorf("unauthorized") + + logger.Errorf("unauthorized") + return "", false } -func (bav *BasicAuthValidator) getFromEtcd(targetUserID string) (string, error) { - const prefix = "/credentials/" - password, err := bav.cluster.Get(prefix + targetUserID) +func (huc *htpasswdUserCache) WatchChanges() error { return nil } +func (huc *htpasswdUserCache) Close() {} +func (huc *htpasswdUserCache) SetSyncInterval(time time.Duration) {} + +func newEtcdUserCache(cluster cluster.Cluster) *etcdUserCache { + cache, err := lru.New(256) if err != nil { - return "", err + panic(err) + } + return &etcdUserCache{ + cache: cache, + syncerInterval: 3 * time.Minute, // it takes 3 minutes to remove user access + cluster: cluster, + } +} + +func (euc *etcdUserCache) GetUser(targetUserID string) (string, bool) { + if val, ok := euc.cache.Get(targetUserID); ok { + return val.(string), true + } + + password, err := euc.cluster.Get(credsPrefix + targetUserID) + if err != nil { + logger.Errorf(err.Error()) + return "", false } if password == nil { - return "", fmt.Errorf("unauthorized") + logger.Errorf("unauthorized") + return "", false } - return *password, nil + + euc.cache.Add(targetUserID, *password) + return *password, true +} + +func (euc *etcdUserCache) SetSyncInterval(interval time.Duration) { + euc.syncerInterval = interval +} + +// updateCache updates the intersection of kvs map and cache and removes other keys from cache. +func updateCache(kvs map[string]string, cache *lru.Cache) { + intersection := make(map[string]string) + for key, password := range kvs { + userID := strings.TrimPrefix(key, credsPrefix) + if oldPassword, ok := cache.Peek(userID); ok { + intersection[userID] = password + if password != oldPassword { + cache.Add(userID, password) // update password + } + } + } + // delete cache items that were not in kvs + for _, cacheKey := range cache.Keys() { + if _, exists := intersection[cacheKey.(string)]; !exists { + cache.Remove(cacheKey) + } + } +} + +func (euc *etcdUserCache) WatchChanges() error { + stopCtx, cancel := context.WithCancel(context.Background()) + euc.cancel = cancel + var ( + syncer *cluster.Syncer + err error + ch <-chan map[string]string + ) + + for { + syncer, err = euc.cluster.Syncer(euc.syncerInterval) + if err != nil { + logger.Errorf("failed to create syncer: %v", err) + } else if ch, err = syncer.SyncPrefix(credsPrefix); err != nil { + logger.Errorf("failed to sync raw prefix: %v", err) + syncer.Close() + } else { + break + } + + select { + case <-time.After(10 * time.Second): + case <-stopCtx.Done(): + return nil + } + } + defer syncer.Close() + + for { + select { + case <-stopCtx.Done(): + return nil + case kvs := <-ch: + updateCache(kvs, euc.cache) + } + } + return nil +} + +func (euc *etcdUserCache) Close() { + euc.cancel() } // NewBasicAuthValidator creates a new Basic Auth validator func NewBasicAuthValidator(spec *BasicAuthValidatorSpec, supervisor *supervisor.Supervisor) *BasicAuthValidator { - var cluster cluster.Cluster + var cache AuthorizedUsersCache if spec.UseEtcd { if supervisor == nil || supervisor.Cluster() == nil { logger.Errorf("BasicAuth validator : failed to read data from etcd") } else { - cluster = supervisor.Cluster() + cache = newEtcdUserCache(supervisor.Cluster()) } + } else if spec.UserFile != "" { + cache = newHtpasswdUserCache(spec.UserFile) + } else { + logger.Errorf("BasicAuth validator spec unvalid.") + return nil } - cache, err := lru.New(256) - if err != nil { - panic(err) - } + go cache.WatchChanges() bav := &BasicAuthValidator{ spec: spec, authorizedUsersCache: cache, - cluster: cluster, } return bav } -func (bav *BasicAuthValidator) getUserFromCache(userID string) (string, bool) { - if val, ok := bav.authorizedUsersCache.Get(userID); ok { - return val.(string), true - } - var password string - var err error - if bav.spec.UserFile != "" { - password, err = bav.getFromFile(userID) - } - if bav.spec.UseEtcd == true { - password, err = bav.getFromEtcd(userID) - } - if err != nil { - return "", false - } - bav.authorizedUsersCache.Add(userID, password) - return password, true -} - func parseBasicAuthorizationHeader(hdr *httpheader.HTTPHeader) (string, error) { const prefix = "Basic " @@ -141,11 +261,11 @@ func parseBasicAuthorizationHeader(hdr *httpheader.HTTPHeader) (string, error) { if !strings.HasPrefix(tokenStr, prefix) { return "", fmt.Errorf("unexpected authorization header: %s", tokenStr) } - return tokenStr[len(prefix):], nil + return strings.TrimPrefix(tokenStr, prefix), nil } // Validate validates the Authorization header of a http request -func (bav *BasicAuthValidator) Validate(req context.HTTPRequest) error { +func (bav *BasicAuthValidator) Validate(req httpcontext.HTTPRequest) error { hdr := req.Header() base64credentials, err := parseBasicAuthorizationHeader(hdr) if err != nil { @@ -161,8 +281,13 @@ func (bav *BasicAuthValidator) Validate(req context.HTTPRequest) error { return fmt.Errorf("unauthorized") } - if expectedToken, ok := bav.getUserFromCache(userID); ok && expectedToken == token { + if expectedToken, ok := bav.authorizedUsersCache.GetUser(userID); ok && expectedToken == token { return nil } return fmt.Errorf("unauthorized") } + +// Close closes authorizedUsersCache. +func (bav *BasicAuthValidator) Close() { + bav.authorizedUsersCache.Close() +} diff --git a/pkg/filter/validator/validator.go b/pkg/filter/validator/validator.go index 30e6a946b1..f32ec0fe56 100644 --- a/pkg/filter/validator/validator.go +++ b/pkg/filter/validator/validator.go @@ -174,5 +174,9 @@ func (v *Validator) handle(ctx context.HTTPContext) string { // Status returns status. func (v *Validator) Status() interface{} { return nil } -// Close closes Validator. -func (v *Validator) Close() {} +// Close closes validations. +func (v *Validator) Close() { + if v.basicAuth != nil { + v.basicAuth.Close() + } +} diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index f75d877e48..ba504f6a3b 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -27,7 +27,9 @@ import ( "strings" "sync" "testing" + "time" + lru "github.com/hashicorp/golang-lru" "github.com/phayes/freeport" cluster "github.com/megaease/easegress/pkg/cluster" @@ -302,6 +304,40 @@ signature: } } +func TestBasicAuthUpdateCache(t *testing.T) { + kvs1 := make(map[string]string) + cache, _ := lru.New(16) + for i := 0; i < 30; i++ { + kvs1[fmt.Sprintf("u%d", i)] = fmt.Sprintf("md5-%d", i) + } + updateCache(kvs1, cache) // does nothing + if cache.Len() != 0 { + t.Errorf("cache should not be modified") + } + + cache.Add("u0", "md5-0") // present in kvs1 + cache.Add("u743", "md5-743") // not present in kvs1 + cache.Add("u744", "md5-744") // not present in kvs1 + + if cache.Len() != 3 { + t.Errorf("cache should have 3 items") + } + updateCache(kvs1, cache) // removes u743 and u744 + if cache.Len() != 1 { + t.Errorf("cache should have 1 item") + } + + cache.Add("u7", "md5-old-password") // present in kvs1 but different value + updateCache(kvs1, cache) // updates u7 + if val, ok := cache.Get("u7"); ok { + if val.(string) != "md5-7" { + t.Errorf("cache value should be updated") + } + } else { + t.Errorf("cache should contain u7") + } +} + func check(e error) { if e != nil { panic(e) @@ -338,7 +374,16 @@ func createCluster(tempDir string) cluster.Cluster { return clusterInstance } -func TestBasicAuthUser(t *testing.T) { +func prepareCtxAndHeader() (*contexttest.MockedHTTPContext, http.Header) { + ctx := &contexttest.MockedHTTPContext{} + header := http.Header{} + ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { + return httpheader.New(header) + } + return ctx, header +} + +func TestBasicAuth(t *testing.T) { userIds := []string{ "userY", "userZ", "nonExistingUser", } @@ -362,11 +407,7 @@ basicAuth: v := createValidator(yamlSpec, nil, nil) for i := 0; i < 3; i++ { - ctx := &contexttest.MockedHTTPContext{} - header := http.Header{} - ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { - return httpheader.New(header) - } + ctx, header := prepareCtxAndHeader() b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[i] + ":" + passwords[i])) header.Set("Authorization", "Basic "+b64creds) result := v.Handle(ctx) @@ -380,6 +421,7 @@ basicAuth: } } } + v.Close() }) t.Run("credentials from etcd", func(t *testing.T) { etcdDirName, err := ioutil.TempDir("", "etcd-validator-test") @@ -401,15 +443,30 @@ basicAuth: useEtcd: true` expectedValid := []bool{true, false, true} - v := createValidator(yamlSpec, nil, supervisor) - + v.basicAuth.authorizedUsersCache.SetSyncInterval(1 * time.Second) for i := 0; i < 3; i++ { - ctx := &contexttest.MockedHTTPContext{} - header := http.Header{} - ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { - return httpheader.New(header) + ctx, header := prepareCtxAndHeader() + b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[i] + ":" + passwords[i])) + header.Set("Authorization", "Basic "+b64creds) + result := v.Handle(ctx) + if expectedValid[i] { + if result == resultInvalid { + t.Errorf("the basic auth head should be valid") + } + } else { + if result != resultInvalid { + t.Errorf("the basic auth head should be invalid") + } } + } + + clusterInstance.Delete("/credentials/" + userIds[0]) // first user is not authorized anymore + time.Sleep(1 * time.Second) // wait that cache item gets deleted + + expectedValid = []bool{false, false, true} + for i := 0; i < 3; i++ { + ctx, header := prepareCtxAndHeader() b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[i] + ":" + passwords[i])) header.Set("Authorization", "Basic "+b64creds) result := v.Handle(ctx) @@ -423,6 +480,8 @@ basicAuth: } } } + + v.Close() wg := &sync.WaitGroup{} wg.Add(1) clusterInstance.CloseServer(wg) From dd32290aaca65ab44ebd4d7e81ad0ae937d44992 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 6 Jan 2022 11:35:38 +0800 Subject: [PATCH 10/34] remove syncInterval --- pkg/filter/validator/basicauth.go | 14 ++---------- pkg/filter/validator/validator_test.go | 31 +++++++++++--------------- 2 files changed, 15 insertions(+), 30 deletions(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index 536f48c6d0..dc6c29752b 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -50,12 +50,9 @@ type ( } // AuthorizedUsersCache provides cached lookup for authorized users. - // When user is deleted from authorized users, user can still access for time period defined - // by SetSyncInterval. AuthorizedUsersCache interface { GetUser(string) (string, bool) WatchChanges() error - SetSyncInterval(time.Duration) Close() } @@ -68,7 +65,6 @@ type ( cache *lru.Cache cluster cluster.Cluster - syncerInterval time.Duration cancel context.CancelFunc } @@ -131,7 +127,6 @@ func (huc *htpasswdUserCache) GetUser(targetUserID string) (string, bool) { func (huc *htpasswdUserCache) WatchChanges() error { return nil } func (huc *htpasswdUserCache) Close() {} -func (huc *htpasswdUserCache) SetSyncInterval(time time.Duration) {} func newEtcdUserCache(cluster cluster.Cluster) *etcdUserCache { cache, err := lru.New(256) @@ -140,7 +135,6 @@ func newEtcdUserCache(cluster cluster.Cluster) *etcdUserCache { } return &etcdUserCache{ cache: cache, - syncerInterval: 3 * time.Minute, // it takes 3 minutes to remove user access cluster: cluster, } } @@ -164,10 +158,6 @@ func (euc *etcdUserCache) GetUser(targetUserID string) (string, bool) { return *password, true } -func (euc *etcdUserCache) SetSyncInterval(interval time.Duration) { - euc.syncerInterval = interval -} - // updateCache updates the intersection of kvs map and cache and removes other keys from cache. func updateCache(kvs map[string]string, cache *lru.Cache) { intersection := make(map[string]string) @@ -198,11 +188,11 @@ func (euc *etcdUserCache) WatchChanges() error { ) for { - syncer, err = euc.cluster.Syncer(euc.syncerInterval) + syncer, err = euc.cluster.Syncer(20 * time.Minute) if err != nil { logger.Errorf("failed to create syncer: %v", err) } else if ch, err = syncer.SyncPrefix(credsPrefix); err != nil { - logger.Errorf("failed to sync raw prefix: %v", err) + logger.Errorf("failed to sync prefix: %v", err) syncer.Close() } else { break diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index ba504f6a3b..7e9f4d9fd5 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -413,11 +413,11 @@ basicAuth: result := v.Handle(ctx) if expectedValid[i] { if result == resultInvalid { - t.Errorf("the basic auth head should be valid") + t.Errorf("should be authorized") } } else { if result != resultInvalid { - t.Errorf("the basic auth head should be invalid") + t.Errorf("should be unauthorized") } } } @@ -444,7 +444,6 @@ basicAuth: expectedValid := []bool{true, false, true} v := createValidator(yamlSpec, nil, supervisor) - v.basicAuth.authorizedUsersCache.SetSyncInterval(1 * time.Second) for i := 0; i < 3; i++ { ctx, header := prepareCtxAndHeader() b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[i] + ":" + passwords[i])) @@ -452,32 +451,28 @@ basicAuth: result := v.Handle(ctx) if expectedValid[i] { if result == resultInvalid { - t.Errorf("the basic auth head should be valid") + t.Errorf("should be authorized") } } else { if result != resultInvalid { - t.Errorf("the basic auth head should be invalid") + t.Errorf("should be unauthorized") } } } clusterInstance.Delete("/credentials/" + userIds[0]) // first user is not authorized anymore - time.Sleep(1 * time.Second) // wait that cache item gets deleted - - expectedValid = []bool{false, false, true} - for i := 0; i < 3; i++ { + tryCount := 5 + for i := 0; i <= tryCount; i++ { + time.Sleep(200 * time.Millisecond) // wait that cache item gets deleted ctx, header := prepareCtxAndHeader() - b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[i] + ":" + passwords[i])) + b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[0] + ":" + passwords[0])) header.Set("Authorization", "Basic "+b64creds) result := v.Handle(ctx) - if expectedValid[i] { - if result == resultInvalid { - t.Errorf("the basic auth head should be valid") - } - } else { - if result != resultInvalid { - t.Errorf("the basic auth head should be invalid") - } + if result == resultInvalid { + break // successfully unauthorized + } + if i == tryCount && result != resultInvalid { + t.Errorf("should be unauthorized") } } From 60acff6c4ce264161bf5914f1024191ac475312a Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 6 Jan 2022 13:56:57 +0800 Subject: [PATCH 11/34] add fileWatcher for unvalidating cache --- pkg/filter/validator/basicauth.go | 112 ++++++++++++++++++++----- pkg/filter/validator/validator_test.go | 26 ++++++ 2 files changed, 116 insertions(+), 22 deletions(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index dc6c29752b..24fbc01d6f 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -54,18 +54,23 @@ type ( GetUser(string) (string, bool) WatchChanges() error Close() + SetSyncInterval(time.Duration) } htpasswdUserCache struct { cache *lru.Cache userFile string + + syncInterval time.Duration + cancel context.CancelFunc } etcdUserCache struct { cache *lru.Cache cluster cluster.Cluster - cancel context.CancelFunc + syncInterval time.Duration + cancel context.CancelFunc } // BasicAuthValidator defines the Basic Auth validator @@ -85,7 +90,27 @@ func parseCredentials(creds string) (string, string, error) { return parts[0], parts[1], nil } -func newHtpasswdUserCache(userFile string) *htpasswdUserCache { +func readPasswordFile(userFile string) (map[string]string, error) { + credentials := make(map[string]string) + file, err := os.OpenFile(userFile, os.O_RDONLY, os.ModePerm) + if err != nil { + return credentials, err + } + defer file.Close() + + sc := bufio.NewScanner(file) + for sc.Scan() { + line := sc.Text() + userID, password, err := parseCredentials(line) + if err != nil { + return credentials, err + } + credentials[userID] = password + } + return credentials, nil +} + +func newHtpasswdUserCache(userFile string, syncInterval time.Duration) *htpasswdUserCache { cache, err := lru.New(256) if err != nil { panic(err) @@ -93,6 +118,8 @@ func newHtpasswdUserCache(userFile string) *htpasswdUserCache { return &htpasswdUserCache{ cache: cache, userFile: userFile, + // Removed authorization or updated passwords are updated according syncInterval. + syncInterval: syncInterval, } } @@ -101,32 +128,64 @@ func (huc *htpasswdUserCache) GetUser(targetUserID string) (string, bool) { return val.(string), true } - file, err := os.OpenFile(huc.userFile, os.O_RDONLY, os.ModePerm) + credentials, err := readPasswordFile(huc.userFile) if err != nil { - logger.Errorf("open file error: %v", err) + logger.Errorf(err.Error()) return "", false } - defer file.Close() - sc := bufio.NewScanner(file) - for sc.Scan() { - line := sc.Text() - userID, password, err := parseCredentials(line) + if password, ok := credentials[targetUserID]; ok { + huc.cache.Add(targetUserID, password) + return password, true + } + + logger.Errorf("unauthorized") + return "", false +} + +func (huc *htpasswdUserCache) WatchChanges() error { + stopCtx, cancel := context.WithCancel(context.Background()) + huc.cancel = cancel + + initialStat, err := os.Stat(huc.userFile) + if err != nil { + return err + } + for { + stat, err := os.Stat(huc.userFile) if err != nil { - logger.Errorf(err.Error()) - return "", false + return err + } + if stat.Size() != initialStat.Size() || stat.ModTime() != initialStat.ModTime() { + credentials, err := readPasswordFile(huc.userFile) + if err != nil { + return err + } + updateCache(credentials, huc.cache) + + initialStat, err = os.Stat(huc.userFile) + if err != nil { + return err + } } - if userID == targetUserID { - huc.cache.Add(targetUserID, password) - return password, true + select { + case <-time.After(huc.syncInterval): + continue + case <-stopCtx.Done(): + return nil } } + return nil +} - logger.Errorf("unauthorized") - return "", false +func (huc *htpasswdUserCache) Close() { + if huc.cancel != nil { + huc.cancel() + } } -func (huc *htpasswdUserCache) WatchChanges() error { return nil } -func (huc *htpasswdUserCache) Close() {} +func (huc *htpasswdUserCache) SetSyncInterval(interval time.Duration) { + huc.syncInterval = interval +} func newEtcdUserCache(cluster cluster.Cluster) *etcdUserCache { cache, err := lru.New(256) @@ -134,8 +193,11 @@ func newEtcdUserCache(cluster cluster.Cluster) *etcdUserCache { panic(err) } return &etcdUserCache{ - cache: cache, - cluster: cluster, + cache: cache, + cluster: cluster, + // cluster.Syncer updates changes (removed access or updated passwords) immediately. + // syncInterval defines data consistency check interval. + syncInterval: 30 * time.Minute, } } @@ -218,7 +280,13 @@ func (euc *etcdUserCache) WatchChanges() error { } func (euc *etcdUserCache) Close() { - euc.cancel() + if euc.cancel != nil { + euc.cancel() + } +} + +func (euc *etcdUserCache) SetSyncInterval(interval time.Duration) { + euc.syncInterval = interval } // NewBasicAuthValidator creates a new Basic Auth validator @@ -231,7 +299,7 @@ func NewBasicAuthValidator(spec *BasicAuthValidatorSpec, supervisor *supervisor. cache = newEtcdUserCache(supervisor.Cluster()) } } else if spec.UserFile != "" { - cache = newHtpasswdUserCache(spec.UserFile) + cache = newHtpasswdUserCache(spec.UserFile, 1*time.Minute) } else { logger.Errorf("BasicAuth validator spec unvalid.") return nil diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 7e9f4d9fd5..392fcc4665 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -406,6 +406,10 @@ basicAuth: expectedValid := []bool{true, true, false} v := createValidator(yamlSpec, nil, nil) + // set shorted syncInterval for test + v.basicAuth.authorizedUsersCache = newHtpasswdUserCache(v.basicAuth.spec.UserFile, 150*time.Millisecond) + go v.basicAuth.authorizedUsersCache.WatchChanges() + for i := 0; i < 3; i++ { ctx, header := prepareCtxAndHeader() b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[i] + ":" + passwords[i])) @@ -421,6 +425,28 @@ basicAuth: } } } + + err = userFile.Truncate(0) + check(err) + _, err = userFile.Seek(0, 0) + check(err) + userFile.Write([]byte("")) // no more authorized users + + tryCount := 5 + for i := 0; i <= tryCount; i++ { + time.Sleep(200 * time.Millisecond) // wait that cache item gets deleted + ctx, header := prepareCtxAndHeader() + b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[0] + ":" + passwords[0])) + header.Set("Authorization", "Basic "+b64creds) + result := v.Handle(ctx) + if result == resultInvalid { + break // successfully unauthorized + } + if i == tryCount && result != resultInvalid { + t.Errorf("should be unauthorized") + } + } + v.Close() }) t.Run("credentials from etcd", func(t *testing.T) { From 2b5fddb3cf51c212ed3cc28775be45596c6cfdba Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 6 Jan 2022 14:51:41 +0800 Subject: [PATCH 12/34] remove unused funcs and add documentation --- doc/cookbook/security.md | 52 +++++++++++++++++++++++++++++-- doc/reference/filters.md | 10 +++++- pkg/filter/validator/basicauth.go | 11 +------ 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/doc/cookbook/security.md b/doc/cookbook/security.md index 5612b6acd7..7e0ff7447c 100644 --- a/doc/cookbook/security.md +++ b/doc/cookbook/security.md @@ -7,11 +7,13 @@ - [JWT](#jwt) - [Signature](#signature) - [OAuth2](#oauth2) + - [Basic Auth](#basic-auth) - [References](#references) - [Header](#header-1) - [JWT](#jwt-1) - [Signature](#signature-1) - [OAuth2](#oauth2-1) + - [Basic Auth](#basic-auth-1) - [Concepts](#concepts) As a production-ready cloud-native traffic orchestrator, Easegress cares about security and provides several features to ensure that. @@ -134,13 +136,35 @@ filters: insecureTls: false - name: proxy kind: Proxy - ``` * The example above uses a token introspection server, which is provided by `endpoint` filed for validation. It also supports `Self-Encoded Access Tokens mode` which will require a JWT related configuration included. Check it out in the Easegress filter doc if needed. [5] * For the full YAML, see [here](#oauth-1) +### Basic Auth + +* Using Basic Auth validation in Easegress. Basic access authentication is the simplest technique for enforcing access control to web resources [6]. You can create .htpasswd file using *apache2-util* `htpasswd` [7] for storing encrypted user credentials. Please note that Basic Auth is not the most secure access control technique and it is not recommended to depend solely to Basic Auth when designing the security features of your environment. + +``` yaml +name: pipeline-reverse-proxy +kind: HTTPPipeline +flow: + - filter: oauth-validator + - filter: proxy +filters: + - kind: Validator + name: oauth-validator + basicAuth: + userFile: '/etc/apache2/.htpasswd' + - name: proxy + kind: Proxy +``` + +* The example above uses credentials defined in `/etc/apache2/.htpasswd` to restrict access. Please check out apache2-utils documentation [7] for more details. + +* For the full YAML, see [here](#basic-auth-1) + ## References ### Header @@ -167,7 +191,6 @@ filters: - url: http://127.0.0.1:9097 loadBalance: policy: roundRobin - ``` ### JWT @@ -247,6 +270,29 @@ filters: policy: roundRobin ``` +### Basic Auth + +``` yaml +name: pipeline-reverse-proxy +kind: HTTPPipeline +flow: + - filter: header-validator + - filter: proxy +filters: + - kind: Validator + name: basic-auth-validator + basicAuth: + userFile: '/etc/apache2/.htpasswd' + - name: proxy + kind: Proxy + mainPool: + servers: + - url: http://127.0.0.1:9095 + - url: http://127.0.0.1:9096 + - url: http://127.0.0.1:9097 + loadBalance: + policy: roundRobin +``` ### Concepts @@ -255,3 +301,5 @@ filters: 3. https://github.com/megaease/easegress/blob/main/doc/filters.md#signerliteral 4. https://oauth.net/2/ 5. https://github.com/megaease/easegress/blob/main/doc/filters.md#validatorOAuth2JWT +6. https://en.wikipedia.org/wiki/Basic_access_authentication +7. https://manpages.debian.org/testing/apache2-utils/htpasswd.1.en.html diff --git a/doc/reference/filters.md b/doc/reference/filters.md index 6dbb06e9f6..31a28b1769 100644 --- a/doc/reference/filters.md +++ b/doc/reference/filters.md @@ -545,7 +545,7 @@ The filter always returns an empty result. ## Validator -The Validator filter validates requests, forwards valid ones, and rejects invalid ones. Four validation methods (`headers`, `jwt`, `signature`, and `oauth2`) are supported up to now, and these methods can either be used together or alone. When two or more methods are used together, a request needs to pass all of them to be forwarded. +The Validator filter validates requests, forwards valid ones, and rejects invalid ones. Four validation methods (`headers`, `jwt`, `signature`, `oauth2` and `basicAuth`) are supported up to now, and these methods can either be used together or alone. When two or more methods are used together, a request needs to pass all of them to be forwarded. Below is an example configuration for the `headers` validation method. Requests which has a header named `Is-Valid` with value `abc` or `goodplan` or matches regular expression `^ok-.+$` are considered to be valid. @@ -592,6 +592,14 @@ oauth2: insecureTls: false ``` +Here's an example for `basicAuth` validation method which uses [Apache2 htpasswd](https://manpages.debian.org/testing/apache2-utils/htpasswd.1.en.html) formatted encrypted password file for validation. +```yaml +kind: Validator +name: basicAuth-validator-example +basicAuth: + userFile: /etc/apache2/.htpasswd +``` + ### Configuration | Name | Type | Description | Required | diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index 24fbc01d6f..b5d33fac30 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -54,7 +54,6 @@ type ( GetUser(string) (string, bool) WatchChanges() error Close() - SetSyncInterval(time.Duration) } htpasswdUserCache struct { @@ -118,7 +117,7 @@ func newHtpasswdUserCache(userFile string, syncInterval time.Duration) *htpasswd return &htpasswdUserCache{ cache: cache, userFile: userFile, - // Removed authorization or updated passwords are updated according syncInterval. + // Removed access or updated passwords are updated according syncInterval. syncInterval: syncInterval, } } @@ -183,10 +182,6 @@ func (huc *htpasswdUserCache) Close() { } } -func (huc *htpasswdUserCache) SetSyncInterval(interval time.Duration) { - huc.syncInterval = interval -} - func newEtcdUserCache(cluster cluster.Cluster) *etcdUserCache { cache, err := lru.New(256) if err != nil { @@ -285,10 +280,6 @@ func (euc *etcdUserCache) Close() { } } -func (euc *etcdUserCache) SetSyncInterval(interval time.Duration) { - euc.syncInterval = interval -} - // NewBasicAuthValidator creates a new Basic Auth validator func NewBasicAuthValidator(spec *BasicAuthValidatorSpec, supervisor *supervisor.Supervisor) *BasicAuthValidator { var cache AuthorizedUsersCache From b188893ec6d54a407cf075824c1fb700694067ed Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 6 Jan 2022 15:40:08 +0800 Subject: [PATCH 13/34] fix race condition in unittest --- pkg/filter/validator/basicauth.go | 43 ++++++++++++++++++++------ pkg/filter/validator/validator_test.go | 3 ++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index b5d33fac30..fe3db183f8 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -22,8 +22,10 @@ import ( "context" "encoding/base64" "fmt" + "io/fs" "os" "strings" + "sync" "time" lru "github.com/hashicorp/golang-lru" @@ -54,20 +56,21 @@ type ( GetUser(string) (string, bool) WatchChanges() error Close() + Lock() // for testing purposes + Unlock() } htpasswdUserCache struct { - cache *lru.Cache - userFile string - + cache *lru.Cache + userFile string + fileMutex sync.RWMutex syncInterval time.Duration cancel context.CancelFunc } etcdUserCache struct { - cache *lru.Cache - cluster cluster.Cluster - + cache *lru.Cache + cluster cluster.Cluster syncInterval time.Duration cancel context.CancelFunc } @@ -127,7 +130,9 @@ func (huc *htpasswdUserCache) GetUser(targetUserID string) (string, bool) { return val.(string), true } + huc.fileMutex.RLock() credentials, err := readPasswordFile(huc.userFile) + huc.fileMutex.RUnlock() if err != nil { logger.Errorf(err.Error()) return "", false @@ -145,23 +150,32 @@ func (huc *htpasswdUserCache) WatchChanges() error { stopCtx, cancel := context.WithCancel(context.Background()) huc.cancel = cancel - initialStat, err := os.Stat(huc.userFile) + getFileStat := func() (fs.FileInfo, error) { + huc.fileMutex.RLock() + stat, err := os.Stat(huc.userFile) + huc.fileMutex.RUnlock() + return stat, err + } + + initialStat, err := getFileStat() if err != nil { return err } for { - stat, err := os.Stat(huc.userFile) + stat, err := getFileStat() if err != nil { return err } if stat.Size() != initialStat.Size() || stat.ModTime() != initialStat.ModTime() { + huc.fileMutex.RLock() credentials, err := readPasswordFile(huc.userFile) + huc.fileMutex.RUnlock() if err != nil { return err } updateCache(credentials, huc.cache) - initialStat, err = os.Stat(huc.userFile) + initialStat, err = getFileStat() if err != nil { return err } @@ -182,6 +196,14 @@ func (huc *htpasswdUserCache) Close() { } } +func (huc *htpasswdUserCache) Lock() { + huc.fileMutex.Lock() +} + +func (huc *htpasswdUserCache) Unlock() { + huc.fileMutex.Unlock() +} + func newEtcdUserCache(cluster cluster.Cluster) *etcdUserCache { cache, err := lru.New(256) if err != nil { @@ -280,6 +302,9 @@ func (euc *etcdUserCache) Close() { } } +func (euc *etcdUserCache) Lock() {} +func (euc *etcdUserCache) Unlock() {} + // NewBasicAuthValidator creates a new Basic Auth validator func NewBasicAuthValidator(spec *BasicAuthValidatorSpec, supervisor *supervisor.Supervisor) *BasicAuthValidator { var cache AuthorizedUsersCache diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 392fcc4665..0b2b6f8573 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -407,6 +407,7 @@ basicAuth: v := createValidator(yamlSpec, nil, nil) // set shorted syncInterval for test + v.basicAuth.authorizedUsersCache.Close() v.basicAuth.authorizedUsersCache = newHtpasswdUserCache(v.basicAuth.spec.UserFile, 150*time.Millisecond) go v.basicAuth.authorizedUsersCache.WatchChanges() @@ -426,11 +427,13 @@ basicAuth: } } + v.basicAuth.authorizedUsersCache.Lock() err = userFile.Truncate(0) check(err) _, err = userFile.Seek(0, 0) check(err) userFile.Write([]byte("")) // no more authorized users + v.basicAuth.authorizedUsersCache.Unlock() tryCount := 5 for i := 0; i <= tryCount; i++ { From 8bf353e6235345da8850b0fd0067234fbd59851f Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 6 Jan 2022 16:53:55 +0800 Subject: [PATCH 14/34] initialize context to fix test --- pkg/filter/validator/basicauth.go | 28 +++++++++++++------------- pkg/filter/validator/validator_test.go | 10 ++++----- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index fe3db183f8..dfa2916479 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -65,6 +65,7 @@ type ( userFile string fileMutex sync.RWMutex syncInterval time.Duration + stopCtx context.Context cancel context.CancelFunc } @@ -72,6 +73,7 @@ type ( cache *lru.Cache cluster cluster.Cluster syncInterval time.Duration + stopCtx context.Context cancel context.CancelFunc } @@ -117,9 +119,12 @@ func newHtpasswdUserCache(userFile string, syncInterval time.Duration) *htpasswd if err != nil { panic(err) } + stopCtx, cancel := context.WithCancel(context.Background()) return &htpasswdUserCache{ cache: cache, userFile: userFile, + stopCtx: stopCtx, + cancel: cancel, // Removed access or updated passwords are updated according syncInterval. syncInterval: syncInterval, } @@ -147,9 +152,6 @@ func (huc *htpasswdUserCache) GetUser(targetUserID string) (string, bool) { } func (huc *htpasswdUserCache) WatchChanges() error { - stopCtx, cancel := context.WithCancel(context.Background()) - huc.cancel = cancel - getFileStat := func() (fs.FileInfo, error) { huc.fileMutex.RLock() stat, err := os.Stat(huc.userFile) @@ -175,6 +177,7 @@ func (huc *htpasswdUserCache) WatchChanges() error { } updateCache(credentials, huc.cache) + // reset initial stat and watch for next modification initialStat, err = getFileStat() if err != nil { return err @@ -183,7 +186,7 @@ func (huc *htpasswdUserCache) WatchChanges() error { select { case <-time.After(huc.syncInterval): continue - case <-stopCtx.Done(): + case <-huc.stopCtx.Done(): return nil } } @@ -191,9 +194,7 @@ func (huc *htpasswdUserCache) WatchChanges() error { } func (huc *htpasswdUserCache) Close() { - if huc.cancel != nil { - huc.cancel() - } + huc.cancel() } func (huc *htpasswdUserCache) Lock() { @@ -209,9 +210,12 @@ func newEtcdUserCache(cluster cluster.Cluster) *etcdUserCache { if err != nil { panic(err) } + stopCtx, cancel := context.WithCancel(context.Background()) return &etcdUserCache{ cache: cache, cluster: cluster, + cancel: cancel, + stopCtx: stopCtx, // cluster.Syncer updates changes (removed access or updated passwords) immediately. // syncInterval defines data consistency check interval. syncInterval: 30 * time.Minute, @@ -258,8 +262,6 @@ func updateCache(kvs map[string]string, cache *lru.Cache) { } func (euc *etcdUserCache) WatchChanges() error { - stopCtx, cancel := context.WithCancel(context.Background()) - euc.cancel = cancel var ( syncer *cluster.Syncer err error @@ -279,7 +281,7 @@ func (euc *etcdUserCache) WatchChanges() error { select { case <-time.After(10 * time.Second): - case <-stopCtx.Done(): + case <-euc.stopCtx.Done(): return nil } } @@ -287,7 +289,7 @@ func (euc *etcdUserCache) WatchChanges() error { for { select { - case <-stopCtx.Done(): + case <-euc.stopCtx.Done(): return nil case kvs := <-ch: updateCache(kvs, euc.cache) @@ -297,9 +299,7 @@ func (euc *etcdUserCache) WatchChanges() error { } func (euc *etcdUserCache) Close() { - if euc.cancel != nil { - euc.cancel() - } + euc.cancel() } func (euc *etcdUserCache) Lock() {} diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 0b2b6f8573..9b6686ff9a 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -394,8 +394,6 @@ func TestBasicAuth(t *testing.T) { userFile, err := os.CreateTemp("", "apache2-htpasswd") check(err) - defer os.Remove(userFile.Name()) - yamlSpec := ` kind: Validator name: validator @@ -407,8 +405,7 @@ basicAuth: v := createValidator(yamlSpec, nil, nil) // set shorted syncInterval for test - v.basicAuth.authorizedUsersCache.Close() - v.basicAuth.authorizedUsersCache = newHtpasswdUserCache(v.basicAuth.spec.UserFile, 150*time.Millisecond) + v.basicAuth.authorizedUsersCache = newHtpasswdUserCache(v.basicAuth.spec.UserFile, 200*time.Millisecond) go v.basicAuth.authorizedUsersCache.WatchChanges() for i := 0; i < 3; i++ { @@ -437,7 +434,7 @@ basicAuth: tryCount := 5 for i := 0; i <= tryCount; i++ { - time.Sleep(200 * time.Millisecond) // wait that cache item gets deleted + time.Sleep(300 * time.Millisecond) // wait that cache item gets deleted ctx, header := prepareCtxAndHeader() b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[0] + ":" + passwords[0])) header.Set("Authorization", "Basic "+b64creds) @@ -450,6 +447,9 @@ basicAuth: } } + v.basicAuth.authorizedUsersCache.Lock() + os.Remove(userFile.Name()) + v.basicAuth.authorizedUsersCache.Unlock() v.Close() }) t.Run("credentials from etcd", func(t *testing.T) { From 5336a8f88fd73d54a7dcfadec30c76a847df537e Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Fri, 7 Jan 2022 15:47:53 +0800 Subject: [PATCH 15/34] simplify test and fix basic auth --- go.mod | 1 + go.sum | 5 ++ pkg/filter/validator/basicauth.go | 117 ++++++++++++------------- pkg/filter/validator/validator_test.go | 36 +++----- 4 files changed, 73 insertions(+), 86 deletions(-) diff --git a/go.mod b/go.mod index b4a4732eb8..d6d86b7807 100644 --- a/go.mod +++ b/go.mod @@ -51,6 +51,7 @@ require ( github.com/spf13/viper v1.8.1 github.com/stretchr/testify v1.7.0 github.com/tcnksm/go-httpstat v0.2.1-0.20191008022543-e866bb274419 + github.com/tg123/go-htpasswd v1.2.0 github.com/tidwall/gjson v1.11.0 github.com/tomasen/realip v0.0.0-20180522021738-f0c99a92ddce github.com/valyala/fasttemplate v1.2.1 diff --git a/go.sum b/go.sum index e8353e5c76..bd27f1c513 100644 --- a/go.sum +++ b/go.sum @@ -107,6 +107,8 @@ github.com/Azure/go-autorest/tracing v0.6.0/go.mod h1:+vhtPC754Xsa23ID7GlGsrdKBp github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ= github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym/WlBOVXweHU+Q+/VP0lqqI8lqeDx9IjBqo= +github.com/GehirnInc/crypt v0.0.0-20200316065508-bb7000b8a962 h1:KeNholpO2xKjgaaSyd+DyQRrsQjhbSeS7qe4nEw8aQw= +github.com/GehirnInc/crypt v0.0.0-20200316065508-bb7000b8a962/go.mod h1:kC29dT1vFpj7py2OvG1khBdQpo3kInWP+6QipLbdngo= github.com/GoogleCloudPlatform/k8s-cloud-provider v0.0.0-20200415212048-7901bc822317/go.mod h1:DF8FZRxMHMGv/vP2lQP6h+dYzzjpuRn24VeRiYn3qjQ= github.com/Masterminds/goutils v1.1.1/go.mod h1:8cTjp+g8YejhMuvIA5y2vz3BpJxksy863GQaJW2MFNU= github.com/Masterminds/semver v1.4.2/go.mod h1:MB6lktGJrhw8PrUyiEoblNEGEQ+RzHPF078ddwwvV3Y= @@ -1266,6 +1268,8 @@ github.com/tcnksm/go-httpstat v0.2.1-0.20191008022543-e866bb274419 h1:elOIj31UL4 github.com/tcnksm/go-httpstat v0.2.1-0.20191008022543-e866bb274419/go.mod h1:s3JVJFtQxtBEBC9dwcdTTXS9xFnM3SXAZwPG41aurT8= github.com/tebeka/strftime v0.1.3 h1:5HQXOqWKYRFfNyBMNVc9z5+QzuBtIXy03psIhtdJYto= github.com/tebeka/strftime v0.1.3/go.mod h1:7wJm3dZlpr4l/oVK0t1HYIc4rMzQ2XJlOMIUJUJH6XQ= +github.com/tg123/go-htpasswd v1.2.0 h1:UKp34m9H467/xklxUxU15wKRru7fwXoTojtxg25ITF0= +github.com/tg123/go-htpasswd v1.2.0/go.mod h1:h7IzlfpvIWnVJhNZ0nQ9HaFxHb7pn5uFJYLlEUJa2sM= github.com/tidwall/gjson v1.11.0 h1:C16pk7tQNiH6VlCrtIXL1w8GaOsi1X3W8KDkE1BuYd4= github.com/tidwall/gjson v1.11.0/go.mod h1:/wbyibRr2FHMks5tjHJ5F8dMZh3AcwJEMf5vlfC0lxk= github.com/tidwall/match v1.1.1 h1:+Ho715JplO36QYgwN9PGYNhgZvoUSc9X2c80KVTi+GA= @@ -1426,6 +1430,7 @@ golang.org/x/crypto v0.0.0-20181029021203-45a5f77698d3/go.mod h1:6SG95UA2DQfeDnf golang.org/x/crypto v0.0.0-20181030102418-4d3f4d9ffa16/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= golang.org/x/crypto v0.0.0-20190211182817-74369b46fc67/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4= +golang.org/x/crypto v0.0.0-20190228161510-8dd112bcdc25/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190313024323-a1f597ede03a/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20190320223903-b7391e95e576/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index dfa2916479..f147bde769 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -18,9 +18,10 @@ package validator import ( - "bufio" "context" + "crypto/sha256" "encoding/base64" + "encoding/hex" "fmt" "io/fs" "os" @@ -29,6 +30,7 @@ import ( "time" lru "github.com/hashicorp/golang-lru" + "github.com/tg123/go-htpasswd" "github.com/megaease/easegress/pkg/cluster" httpcontext "github.com/megaease/easegress/pkg/context" @@ -53,20 +55,20 @@ type ( // AuthorizedUsersCache provides cached lookup for authorized users. AuthorizedUsersCache interface { - GetUser(string) (string, bool) + Match(string, string) bool WatchChanges() error + Refresh() error Close() - Lock() // for testing purposes - Unlock() } htpasswdUserCache struct { - cache *lru.Cache - userFile string - fileMutex sync.RWMutex - syncInterval time.Duration - stopCtx context.Context - cancel context.CancelFunc + cache *lru.Cache + userFile string + userFileObject *htpasswd.File + fileMutex sync.RWMutex + syncInterval time.Duration + stopCtx context.Context + cancel context.CancelFunc } etcdUserCache struct { @@ -94,24 +96,9 @@ func parseCredentials(creds string) (string, string, error) { return parts[0], parts[1], nil } -func readPasswordFile(userFile string) (map[string]string, error) { - credentials := make(map[string]string) - file, err := os.OpenFile(userFile, os.O_RDONLY, os.ModePerm) - if err != nil { - return credentials, err - } - defer file.Close() - - sc := bufio.NewScanner(file) - for sc.Scan() { - line := sc.Text() - userID, password, err := parseCredentials(line) - if err != nil { - return credentials, err - } - credentials[userID] = password - } - return credentials, nil +func sha256Sum(data []byte) string { + sha256Bytes := sha256.Sum256(data) + return hex.EncodeToString(sha256Bytes[:]) } func newHtpasswdUserCache(userFile string, syncInterval time.Duration) *htpasswdUserCache { @@ -120,35 +107,27 @@ func newHtpasswdUserCache(userFile string, syncInterval time.Duration) *htpasswd panic(err) } stopCtx, cancel := context.WithCancel(context.Background()) + userFileObject, err := htpasswd.New(userFile, htpasswd.DefaultSystems, nil) + if err != nil { + panic(err) + } return &htpasswdUserCache{ - cache: cache, - userFile: userFile, - stopCtx: stopCtx, - cancel: cancel, + cache: cache, + userFile: userFile, + stopCtx: stopCtx, + cancel: cancel, + userFileObject: userFileObject, // Removed access or updated passwords are updated according syncInterval. syncInterval: syncInterval, } } -func (huc *htpasswdUserCache) GetUser(targetUserID string) (string, bool) { - if val, ok := huc.cache.Get(targetUserID); ok { - return val.(string), true - } - +// Refresh reloads users from userFile. +func (huc *htpasswdUserCache) Refresh() error { huc.fileMutex.RLock() - credentials, err := readPasswordFile(huc.userFile) + err := huc.userFileObject.Reload(nil) huc.fileMutex.RUnlock() - if err != nil { - logger.Errorf(err.Error()) - return "", false - } - if password, ok := credentials[targetUserID]; ok { - huc.cache.Add(targetUserID, password) - return password, true - } - - logger.Errorf("unauthorized") - return "", false + return err } func (huc *htpasswdUserCache) WatchChanges() error { @@ -169,13 +148,10 @@ func (huc *htpasswdUserCache) WatchChanges() error { return err } if stat.Size() != initialStat.Size() || stat.ModTime() != initialStat.ModTime() { - huc.fileMutex.RLock() - credentials, err := readPasswordFile(huc.userFile) - huc.fileMutex.RUnlock() + err := huc.Refresh() if err != nil { return err } - updateCache(credentials, huc.cache) // reset initial stat and watch for next modification initialStat, err = getFileStat() @@ -197,12 +173,8 @@ func (huc *htpasswdUserCache) Close() { huc.cancel() } -func (huc *htpasswdUserCache) Lock() { - huc.fileMutex.Lock() -} - -func (huc *htpasswdUserCache) Unlock() { - huc.fileMutex.Unlock() +func (huc *htpasswdUserCache) Match(username string, password string) bool { + return huc.userFileObject.Match(username, password) } func newEtcdUserCache(cluster cluster.Cluster) *etcdUserCache { @@ -302,8 +274,27 @@ func (euc *etcdUserCache) Close() { euc.cancel() } -func (euc *etcdUserCache) Lock() {} -func (euc *etcdUserCache) Unlock() {} +func (euc *etcdUserCache) Refresh() error { return nil } + +func (euc *etcdUserCache) Match(targetUserID string, targetPassword string) bool { + var password string + if val, ok := euc.cache.Get(targetUserID); ok { + password = val.(string) + } else { + result, err := euc.cluster.Get(credsPrefix + targetUserID) + if err != nil { + logger.Errorf(err.Error()) + return false + } + if result == nil { + logger.Errorf("unauthorized") + return false + } + password = *result + euc.cache.Add(targetUserID, password) + } + return password == targetPassword +} // NewBasicAuthValidator creates a new Basic Auth validator func NewBasicAuthValidator(spec *BasicAuthValidatorSpec, supervisor *supervisor.Supervisor) *BasicAuthValidator { @@ -350,12 +341,12 @@ func (bav *BasicAuthValidator) Validate(req httpcontext.HTTPRequest) error { return fmt.Errorf("error occured during base64 decode: %s", err.Error()) } credentials := string(credentialBytes) - userID, token, err := parseCredentials(credentials) + userID, password, err := parseCredentials(credentials) if err != nil { return fmt.Errorf("unauthorized") } - if expectedToken, ok := bav.authorizedUsersCache.GetUser(userID); ok && expectedToken == token { + if bav.authorizedUsersCache.Match(userID, password) { return nil } return fmt.Errorf("unauthorized") diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 9b6686ff9a..7b0e54638f 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -388,8 +388,11 @@ func TestBasicAuth(t *testing.T) { "userY", "userZ", "nonExistingUser", } passwords := []string{ - "md5-encrypted-pw-1", "md5-encrypted-pw-2", "md5-encrypted-pw3", + sha256Sum([]byte("userpasswordY")), + sha256Sum([]byte("userpasswordZ")), + sha256Sum([]byte("userpasswordX")), } + t.Run("credentials from userFile", func(t *testing.T) { userFile, err := os.CreateTemp("", "apache2-htpasswd") check(err) @@ -404,10 +407,6 @@ basicAuth: expectedValid := []bool{true, true, false} v := createValidator(yamlSpec, nil, nil) - // set shorted syncInterval for test - v.basicAuth.authorizedUsersCache = newHtpasswdUserCache(v.basicAuth.spec.UserFile, 200*time.Millisecond) - go v.basicAuth.authorizedUsersCache.WatchChanges() - for i := 0; i < 3; i++ { ctx, header := prepareCtxAndHeader() b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[i] + ":" + passwords[i])) @@ -424,32 +423,22 @@ basicAuth: } } - v.basicAuth.authorizedUsersCache.Lock() err = userFile.Truncate(0) check(err) _, err = userFile.Seek(0, 0) check(err) userFile.Write([]byte("")) // no more authorized users - v.basicAuth.authorizedUsersCache.Unlock() - - tryCount := 5 - for i := 0; i <= tryCount; i++ { - time.Sleep(300 * time.Millisecond) // wait that cache item gets deleted - ctx, header := prepareCtxAndHeader() - b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[0] + ":" + passwords[0])) - header.Set("Authorization", "Basic "+b64creds) - result := v.Handle(ctx) - if result == resultInvalid { - break // successfully unauthorized - } - if i == tryCount && result != resultInvalid { - t.Errorf("should be unauthorized") - } + v.basicAuth.authorizedUsersCache.Refresh() + + ctx, header := prepareCtxAndHeader() + b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[0] + ":" + passwords[0])) + header.Set("Authorization", "Basic "+b64creds) + result := v.Handle(ctx) + if result != resultInvalid { + t.Errorf("should be unauthorized") } - v.basicAuth.authorizedUsersCache.Lock() os.Remove(userFile.Name()) - v.basicAuth.authorizedUsersCache.Unlock() v.Close() }) t.Run("credentials from etcd", func(t *testing.T) { @@ -490,6 +479,7 @@ basicAuth: } clusterInstance.Delete("/credentials/" + userIds[0]) // first user is not authorized anymore + tryCount := 5 for i := 0; i <= tryCount; i++ { time.Sleep(200 * time.Millisecond) // wait that cache item gets deleted From 26bd8eff08a32f8c17a07ec7fe3170271a76a410 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Mon, 10 Jan 2022 15:49:03 +0800 Subject: [PATCH 16/34] make etcd yaml password format configurable and use go-htpasswd for both password file and etcd --- pkg/filter/validator/basicauth.go | 143 ++++++++++++------------- pkg/filter/validator/validator_test.go | 60 ++++------- 2 files changed, 84 insertions(+), 119 deletions(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index f147bde769..f23d2d8ed3 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -23,34 +23,43 @@ import ( "encoding/base64" "encoding/hex" "fmt" + "io" "io/fs" "os" "strings" "sync" "time" - lru "github.com/hashicorp/golang-lru" "github.com/tg123/go-htpasswd" + "golang.org/x/crypto/bcrypt" "github.com/megaease/easegress/pkg/cluster" httpcontext "github.com/megaease/easegress/pkg/context" "github.com/megaease/easegress/pkg/logger" "github.com/megaease/easegress/pkg/supervisor" "github.com/megaease/easegress/pkg/util/httpheader" + "github.com/megaease/easegress/pkg/util/yamltool" ) type ( + // EtcdYamlFormatSpec defines which yaml entry is the password. For example spec + // passwordKey: "pw" + // expects following yaml + // pw: {encrypted password} + EtcdYamlFormatSpec struct { + PasswordKey string `yaml:"passwordKey" jsonschema:"omitempty"` + } // BasicAuthValidatorSpec defines the configuration of Basic Auth validator. - // Only one of UserFile or UseEtcd should be defined. + // Only one of UserFile or EtcdYamlFormat should be defined. BasicAuthValidatorSpec struct { // UserFile is path to file containing encrypted user credentials in apache2-utils/htpasswd format. // To add user `userY`, use `sudo htpasswd /etc/apache2/.htpasswd userY` // Reference: https://manpages.debian.org/testing/apache2-utils/htpasswd.1.en.html#EXAMPLES UserFile string `yaml:"userFile" jsonschema:"omitempty"` - // When UseEtcd is true, verify user credentials from etcd. Etcd stores them: - // key: /credentials/{user id} - // value: {encrypted password} - UseEtcd bool `yaml:"useEtcd" jsonschema:"omitempty"` + // When etcdYamlFormat is specified, verify user credentials from etcd. Etcd stores them: + // key: /credentials/{username} + // value: {yaml string in format of etcdYamlFormat} + EtcdYamlFormat *EtcdYamlFormatSpec `yaml:"etcdYamlFormat" jsonschema:"omitempty"` } // AuthorizedUsersCache provides cached lookup for authorized users. @@ -62,7 +71,6 @@ type ( } htpasswdUserCache struct { - cache *lru.Cache userFile string userFileObject *htpasswd.File fileMutex sync.RWMutex @@ -72,11 +80,12 @@ type ( } etcdUserCache struct { - cache *lru.Cache - cluster cluster.Cluster - syncInterval time.Duration - stopCtx context.Context - cancel context.CancelFunc + userFileObject *htpasswd.File + cluster cluster.Cluster + passwordKey string + syncInterval time.Duration + stopCtx context.Context + cancel context.CancelFunc } // BasicAuthValidator defines the Basic Auth validator @@ -101,18 +110,18 @@ func sha256Sum(data []byte) string { return hex.EncodeToString(sha256Bytes[:]) } +func bcryptHash(data []byte) (string, error) { + pw, err := bcrypt.GenerateFromPassword(data, bcrypt.DefaultCost) + return string(pw), err +} + func newHtpasswdUserCache(userFile string, syncInterval time.Duration) *htpasswdUserCache { - cache, err := lru.New(256) - if err != nil { - panic(err) - } stopCtx, cancel := context.WithCancel(context.Background()) userFileObject, err := htpasswd.New(userFile, htpasswd.DefaultSystems, nil) if err != nil { panic(err) } return &htpasswdUserCache{ - cache: cache, userFile: userFile, stopCtx: stopCtx, cancel: cancel, @@ -177,60 +186,52 @@ func (huc *htpasswdUserCache) Match(username string, password string) bool { return huc.userFileObject.Match(username, password) } -func newEtcdUserCache(cluster cluster.Cluster) *etcdUserCache { - cache, err := lru.New(256) +func newEtcdUserCache(cluster cluster.Cluster, etcdYamlFormat *EtcdYamlFormatSpec) *etcdUserCache { + kvs, err := cluster.GetPrefix(credsPrefix) + if err != nil { + panic(err) + } + passwordKey := etcdYamlFormat.PasswordKey + pwReader, err := mapToReader(kvs, passwordKey) + if err != nil { + panic(err) + } + userFileObject, err := htpasswd.NewFromReader(pwReader, htpasswd.DefaultSystems, nil) if err != nil { panic(err) } stopCtx, cancel := context.WithCancel(context.Background()) return &etcdUserCache{ - cache: cache, - cluster: cluster, - cancel: cancel, - stopCtx: stopCtx, + userFileObject: userFileObject, + cluster: cluster, + passwordKey: passwordKey, + cancel: cancel, + stopCtx: stopCtx, // cluster.Syncer updates changes (removed access or updated passwords) immediately. // syncInterval defines data consistency check interval. syncInterval: 30 * time.Minute, } } -func (euc *etcdUserCache) GetUser(targetUserID string) (string, bool) { - if val, ok := euc.cache.Get(targetUserID); ok { - return val.(string), true - } - - password, err := euc.cluster.Get(credsPrefix + targetUserID) - if err != nil { - logger.Errorf(err.Error()) - return "", false - } - if password == nil { - logger.Errorf("unauthorized") - return "", false - } - - euc.cache.Add(targetUserID, *password) - return *password, true +func parseYamlPW(entry string, key string) (string, bool) { + credentials := make(map[string]string) + yamltool.Unmarshal([]byte(entry), &credentials) + value, ok := credentials[key] + return value, ok } -// updateCache updates the intersection of kvs map and cache and removes other keys from cache. -func updateCache(kvs map[string]string, cache *lru.Cache) { - intersection := make(map[string]string) - for key, password := range kvs { - userID := strings.TrimPrefix(key, credsPrefix) - if oldPassword, ok := cache.Peek(userID); ok { - intersection[userID] = password - if password != oldPassword { - cache.Add(userID, password) // update password - } - } - } - // delete cache items that were not in kvs - for _, cacheKey := range cache.Keys() { - if _, exists := intersection[cacheKey.(string)]; !exists { - cache.Remove(cacheKey) +func mapToReader(kvs map[string]string, yamlKey string) (io.Reader, error) { + pwStrSlice := make([]string, 0, len(kvs)) + for key, yaml := range kvs { + pw, ok := parseYamlPW(yaml, yamlKey) + if !ok { + return nil, fmt.Errorf("parsing pw updates failed") } + username := strings.TrimPrefix(key, credsPrefix) + pwStrSlice = append(pwStrSlice, username+":"+pw) } + stringData := strings.Join(pwStrSlice, "\n") + return strings.NewReader(stringData), nil } func (euc *etcdUserCache) WatchChanges() error { @@ -264,7 +265,11 @@ func (euc *etcdUserCache) WatchChanges() error { case <-euc.stopCtx.Done(): return nil case kvs := <-ch: - updateCache(kvs, euc.cache) + reader, err := mapToReader(kvs, euc.passwordKey) + if err != nil { + logger.Errorf(err.Error()) + } + euc.userFileObject.ReloadFromReader(reader, nil) } } return nil @@ -276,34 +281,18 @@ func (euc *etcdUserCache) Close() { func (euc *etcdUserCache) Refresh() error { return nil } -func (euc *etcdUserCache) Match(targetUserID string, targetPassword string) bool { - var password string - if val, ok := euc.cache.Get(targetUserID); ok { - password = val.(string) - } else { - result, err := euc.cluster.Get(credsPrefix + targetUserID) - if err != nil { - logger.Errorf(err.Error()) - return false - } - if result == nil { - logger.Errorf("unauthorized") - return false - } - password = *result - euc.cache.Add(targetUserID, password) - } - return password == targetPassword +func (euc *etcdUserCache) Match(username string, password string) bool { + return euc.userFileObject.Match(username, password) } // NewBasicAuthValidator creates a new Basic Auth validator func NewBasicAuthValidator(spec *BasicAuthValidatorSpec, supervisor *supervisor.Supervisor) *BasicAuthValidator { var cache AuthorizedUsersCache - if spec.UseEtcd { + if spec.EtcdYamlFormat != nil { if supervisor == nil || supervisor.Cluster() == nil { logger.Errorf("BasicAuth validator : failed to read data from etcd") } else { - cache = newEtcdUserCache(supervisor.Cluster()) + cache = newEtcdUserCache(supervisor.Cluster(), spec.EtcdYamlFormat) } } else if spec.UserFile != "" { cache = newHtpasswdUserCache(spec.UserFile, 1*time.Minute) diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 7b0e54638f..dd7ab52fa9 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -29,7 +29,6 @@ import ( "testing" "time" - lru "github.com/hashicorp/golang-lru" "github.com/phayes/freeport" cluster "github.com/megaease/easegress/pkg/cluster" @@ -304,40 +303,6 @@ signature: } } -func TestBasicAuthUpdateCache(t *testing.T) { - kvs1 := make(map[string]string) - cache, _ := lru.New(16) - for i := 0; i < 30; i++ { - kvs1[fmt.Sprintf("u%d", i)] = fmt.Sprintf("md5-%d", i) - } - updateCache(kvs1, cache) // does nothing - if cache.Len() != 0 { - t.Errorf("cache should not be modified") - } - - cache.Add("u0", "md5-0") // present in kvs1 - cache.Add("u743", "md5-743") // not present in kvs1 - cache.Add("u744", "md5-744") // not present in kvs1 - - if cache.Len() != 3 { - t.Errorf("cache should have 3 items") - } - updateCache(kvs1, cache) // removes u743 and u744 - if cache.Len() != 1 { - t.Errorf("cache should have 1 item") - } - - cache.Add("u7", "md5-old-password") // present in kvs1 but different value - updateCache(kvs1, cache) // updates u7 - if val, ok := cache.Get("u7"); ok { - if val.(string) != "md5-7" { - t.Errorf("cache value should be updated") - } - } else { - t.Errorf("cache should contain u7") - } -} - func check(e error) { if e != nil { panic(e) @@ -388,9 +353,15 @@ func TestBasicAuth(t *testing.T) { "userY", "userZ", "nonExistingUser", } passwords := []string{ - sha256Sum([]byte("userpasswordY")), - sha256Sum([]byte("userpasswordZ")), - sha256Sum([]byte("userpasswordX")), + "userpasswordY", "userpasswordZ", "userpasswordX", + } + encrypt := func(pw string) string { + encPw, err := bcryptHash([]byte(pw)) + check(err) + return encPw + } + encryptedPasswords := []string{ + encrypt("userpasswordY"), encrypt("userpasswordZ"), encrypt("userpasswordX"), } t.Run("credentials from userFile", func(t *testing.T) { @@ -403,7 +374,8 @@ name: validator basicAuth: userFile: ` + userFile.Name() - userFile.Write([]byte(userIds[0] + ":" + passwords[0] + "\n" + userIds[1] + ":" + passwords[1])) + userFile.Write( + []byte(userIds[0] + ":" + encryptedPasswords[0] + "\n" + userIds[1] + ":" + encryptedPasswords[1])) expectedValid := []bool{true, true, false} v := createValidator(yamlSpec, nil, nil) @@ -447,8 +419,11 @@ basicAuth: defer os.RemoveAll(etcdDirName) clusterInstance := createCluster(etcdDirName) - clusterInstance.Put("/credentials/"+userIds[0], passwords[0]) - clusterInstance.Put("/credentials/"+userIds[2], passwords[2]) + pwToYaml := func(pw string) string { + return fmt.Sprintf("password: %s", pw) + } + clusterInstance.Put("/credentials/"+userIds[0], pwToYaml(encryptedPasswords[0])) + clusterInstance.Put("/credentials/"+userIds[2], pwToYaml(encryptedPasswords[2])) var mockMap sync.Map supervisor := supervisor.NewMock( @@ -458,7 +433,8 @@ basicAuth: kind: Validator name: validator basicAuth: - useEtcd: true` + etcdYamlFormat: + passwordKey: "password"` expectedValid := []bool{true, false, true} v := createValidator(yamlSpec, nil, supervisor) From 03c435fcd35e965daeb2cccd0b6bcdda3058172d Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Mon, 10 Jan 2022 17:24:25 +0800 Subject: [PATCH 17/34] better error handling --- pkg/filter/validator/basicauth.go | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index f23d2d8ed3..8c95691117 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -18,6 +18,7 @@ package validator import ( + "bytes" "context" "crypto/sha256" "encoding/base64" @@ -194,7 +195,7 @@ func newEtcdUserCache(cluster cluster.Cluster, etcdYamlFormat *EtcdYamlFormatSpe passwordKey := etcdYamlFormat.PasswordKey pwReader, err := mapToReader(kvs, passwordKey) if err != nil { - panic(err) + logger.Errorf(err.Error()) } userFileObject, err := htpasswd.NewFromReader(pwReader, htpasswd.DefaultSystems, nil) if err != nil { @@ -214,9 +215,17 @@ func newEtcdUserCache(cluster cluster.Cluster, etcdYamlFormat *EtcdYamlFormatSpe } func parseYamlPW(entry string, key string) (string, bool) { + ok := false + value := "" + defer func() { + if err := recover(); err != nil { + logger.Errorf("Could not marshal credentials. Ensure that credentials are valid yaml.") + ok = false + } + }() credentials := make(map[string]string) yamltool.Unmarshal([]byte(entry), &credentials) - value, ok := credentials[key] + value, ok = credentials[key] return value, ok } @@ -225,7 +234,8 @@ func mapToReader(kvs map[string]string, yamlKey string) (io.Reader, error) { for key, yaml := range kvs { pw, ok := parseYamlPW(yaml, yamlKey) if !ok { - return nil, fmt.Errorf("parsing pw updates failed") + return bytes.NewReader([]byte("")), + fmt.Errorf("Parsing password updates failed. Make sure that '" + yamlKey + "' is a valid yaml entry.") } username := strings.TrimPrefix(key, credsPrefix) pwStrSlice = append(pwStrSlice, username+":"+pw) @@ -265,11 +275,11 @@ func (euc *etcdUserCache) WatchChanges() error { case <-euc.stopCtx.Done(): return nil case kvs := <-ch: - reader, err := mapToReader(kvs, euc.passwordKey) + pwReader, err := mapToReader(kvs, euc.passwordKey) if err != nil { logger.Errorf(err.Error()) } - euc.userFileObject.ReloadFromReader(reader, nil) + euc.userFileObject.ReloadFromReader(pwReader, nil) } } return nil From 0d23ac72998391d748f183a623d68993f63a7736 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Mon, 10 Jan 2022 17:57:32 +0800 Subject: [PATCH 18/34] add test case --- pkg/filter/validator/basicauth.go | 15 ++++----------- pkg/filter/validator/validator_test.go | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index 8c95691117..db45cb5949 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -20,9 +20,7 @@ package validator import ( "bytes" "context" - "crypto/sha256" "encoding/base64" - "encoding/hex" "fmt" "io" "io/fs" @@ -106,11 +104,6 @@ func parseCredentials(creds string) (string, string, error) { return parts[0], parts[1], nil } -func sha256Sum(data []byte) string { - sha256Bytes := sha256.Sum256(data) - return hex.EncodeToString(sha256Bytes[:]) -} - func bcryptHash(data []byte) (string, error) { pw, err := bcrypt.GenerateFromPassword(data, bcrypt.DefaultCost) return string(pw), err @@ -216,17 +209,17 @@ func newEtcdUserCache(cluster cluster.Cluster, etcdYamlFormat *EtcdYamlFormatSpe func parseYamlPW(entry string, key string) (string, bool) { ok := false - value := "" + var value interface{} defer func() { if err := recover(); err != nil { logger.Errorf("Could not marshal credentials. Ensure that credentials are valid yaml.") ok = false } }() - credentials := make(map[string]string) + credentials := make(map[string]interface{}) yamltool.Unmarshal([]byte(entry), &credentials) value, ok = credentials[key] - return value, ok + return value.(string), ok } func mapToReader(kvs map[string]string, yamlKey string) (io.Reader, error) { @@ -252,7 +245,7 @@ func (euc *etcdUserCache) WatchChanges() error { ) for { - syncer, err = euc.cluster.Syncer(20 * time.Minute) + syncer, err = euc.cluster.Syncer(euc.syncInterval) if err != nil { logger.Errorf("failed to create syncer: %v", err) } else if ch, err = syncer.SyncPrefix(credsPrefix); err != nil { diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index dd7ab52fa9..9866b2c9a2 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -455,6 +455,14 @@ basicAuth: } clusterInstance.Delete("/credentials/" + userIds[0]) // first user is not authorized anymore + clusterInstance.Put("/credentials/doge", + ` +randomEntry1: 21 +nestedEntry: + key1: val1 +password: doge +lastEntry: "byebye" +`) tryCount := 5 for i := 0; i <= tryCount; i++ { @@ -471,6 +479,14 @@ basicAuth: } } + ctx, header := prepareCtxAndHeader() + b64creds := base64.StdEncoding.EncodeToString([]byte("doge:doge")) + header.Set("Authorization", "Basic "+b64creds) + result := v.Handle(ctx) + if result == resultInvalid { + t.Errorf("should be authorized") + } + v.Close() wg := &sync.WaitGroup{} wg.Add(1) From a5fd957b39d2ee988bb531d9a92ca1015884df5a Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Mon, 10 Jan 2022 18:33:22 +0800 Subject: [PATCH 19/34] make etcd prefix configuratble --- pkg/filter/validator/basicauth.go | 37 +++++++++++++++----------- pkg/filter/validator/validator_test.go | 3 ++- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index db45cb5949..dec61c4184 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -41,11 +41,13 @@ import ( ) type ( - // EtcdYamlFormatSpec defines which yaml entry is the password. For example spec + // EtcdSpec defines etcd prefix and which yaml entry is the password. For example spec + // prefix: "/creds/" // passwordKey: "pw" - // expects following yaml + // expects the yaml to be stored with key /creds/{username} in following yaml // pw: {encrypted password} - EtcdYamlFormatSpec struct { + EtcdSpec struct { + Prefix string `yaml:"prefix" jsonschema:"onitempty"` PasswordKey string `yaml:"passwordKey" jsonschema:"omitempty"` } // BasicAuthValidatorSpec defines the configuration of Basic Auth validator. @@ -57,8 +59,8 @@ type ( UserFile string `yaml:"userFile" jsonschema:"omitempty"` // When etcdYamlFormat is specified, verify user credentials from etcd. Etcd stores them: // key: /credentials/{username} - // value: {yaml string in format of etcdYamlFormat} - EtcdYamlFormat *EtcdYamlFormatSpec `yaml:"etcdYamlFormat" jsonschema:"omitempty"` + // value: {yaml string in format of etcd} + Etcd *EtcdSpec `yaml:"etcd" jsonschema:"omitempty"` } // AuthorizedUsersCache provides cached lookup for authorized users. @@ -81,6 +83,7 @@ type ( etcdUserCache struct { userFileObject *htpasswd.File cluster cluster.Cluster + prefix string passwordKey string syncInterval time.Duration stopCtx context.Context @@ -180,13 +183,16 @@ func (huc *htpasswdUserCache) Match(username string, password string) bool { return huc.userFileObject.Match(username, password) } -func newEtcdUserCache(cluster cluster.Cluster, etcdYamlFormat *EtcdYamlFormatSpec) *etcdUserCache { - kvs, err := cluster.GetPrefix(credsPrefix) +func newEtcdUserCache(cluster cluster.Cluster, etcdConfig *EtcdSpec) *etcdUserCache { + prefix := etcdConfig.Prefix + if prefix == "" { + prefix = "/custom-data/credentials/" + } + kvs, err := cluster.GetPrefix(prefix) if err != nil { panic(err) } - passwordKey := etcdYamlFormat.PasswordKey - pwReader, err := mapToReader(kvs, passwordKey) + pwReader, err := mapToReader(kvs, etcdConfig.PasswordKey, prefix) if err != nil { logger.Errorf(err.Error()) } @@ -198,7 +204,8 @@ func newEtcdUserCache(cluster cluster.Cluster, etcdYamlFormat *EtcdYamlFormatSpe return &etcdUserCache{ userFileObject: userFileObject, cluster: cluster, - passwordKey: passwordKey, + prefix: prefix, + passwordKey: etcdConfig.PasswordKey, cancel: cancel, stopCtx: stopCtx, // cluster.Syncer updates changes (removed access or updated passwords) immediately. @@ -222,7 +229,7 @@ func parseYamlPW(entry string, key string) (string, bool) { return value.(string), ok } -func mapToReader(kvs map[string]string, yamlKey string) (io.Reader, error) { +func mapToReader(kvs map[string]string, yamlKey string, prefix string) (io.Reader, error) { pwStrSlice := make([]string, 0, len(kvs)) for key, yaml := range kvs { pw, ok := parseYamlPW(yaml, yamlKey) @@ -230,7 +237,7 @@ func mapToReader(kvs map[string]string, yamlKey string) (io.Reader, error) { return bytes.NewReader([]byte("")), fmt.Errorf("Parsing password updates failed. Make sure that '" + yamlKey + "' is a valid yaml entry.") } - username := strings.TrimPrefix(key, credsPrefix) + username := strings.TrimPrefix(key, prefix) pwStrSlice = append(pwStrSlice, username+":"+pw) } stringData := strings.Join(pwStrSlice, "\n") @@ -268,7 +275,7 @@ func (euc *etcdUserCache) WatchChanges() error { case <-euc.stopCtx.Done(): return nil case kvs := <-ch: - pwReader, err := mapToReader(kvs, euc.passwordKey) + pwReader, err := mapToReader(kvs, euc.passwordKey, euc.prefix) if err != nil { logger.Errorf(err.Error()) } @@ -291,11 +298,11 @@ func (euc *etcdUserCache) Match(username string, password string) bool { // NewBasicAuthValidator creates a new Basic Auth validator func NewBasicAuthValidator(spec *BasicAuthValidatorSpec, supervisor *supervisor.Supervisor) *BasicAuthValidator { var cache AuthorizedUsersCache - if spec.EtcdYamlFormat != nil { + if spec.Etcd != nil { if supervisor == nil || supervisor.Cluster() == nil { logger.Errorf("BasicAuth validator : failed to read data from etcd") } else { - cache = newEtcdUserCache(supervisor.Cluster(), spec.EtcdYamlFormat) + cache = newEtcdUserCache(supervisor.Cluster(), spec.Etcd) } } else if spec.UserFile != "" { cache = newHtpasswdUserCache(spec.UserFile, 1*time.Minute) diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 9866b2c9a2..6344134c19 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -433,7 +433,8 @@ basicAuth: kind: Validator name: validator basicAuth: - etcdYamlFormat: + etcd: + prefix: /credentials/ passwordKey: "password"` expectedValid := []bool{true, false, true} From caa1a4974bc4d4f0c67ca2c7aa0f7e610c381ea5 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Tue, 11 Jan 2022 10:16:56 +0800 Subject: [PATCH 20/34] make username yaml entry configurable --- pkg/filter/validator/basicauth.go | 52 ++++++++++++++++---------- pkg/filter/validator/validator_test.go | 12 +++--- 2 files changed, 40 insertions(+), 24 deletions(-) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index dec61c4184..dcc162c816 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -41,13 +41,16 @@ import ( ) type ( - // EtcdSpec defines etcd prefix and which yaml entry is the password. For example spec + // EtcdSpec defines etcd prefix and which yaml entries are the username and password. For example spec // prefix: "/creds/" + // usernameKey: "user" // passwordKey: "pw" - // expects the yaml to be stored with key /creds/{username} in following yaml - // pw: {encrypted password} + // expects the yaml to be stored with key /creds/{id} in following yaml (extra keys are allowed) + // user: doge + // pw: {encrypted password} EtcdSpec struct { Prefix string `yaml:"prefix" jsonschema:"onitempty"` + UsernameKey string `yaml:"usernameKey" jsonschema:"omitempty"` PasswordKey string `yaml:"passwordKey" jsonschema:"omitempty"` } // BasicAuthValidatorSpec defines the configuration of Basic Auth validator. @@ -84,6 +87,7 @@ type ( userFileObject *htpasswd.File cluster cluster.Cluster prefix string + usernameKey string passwordKey string syncInterval time.Duration stopCtx context.Context @@ -192,7 +196,7 @@ func newEtcdUserCache(cluster cluster.Cluster, etcdConfig *EtcdSpec) *etcdUserCa if err != nil { panic(err) } - pwReader, err := mapToReader(kvs, etcdConfig.PasswordKey, prefix) + pwReader, err := kvsToReader(kvs, etcdConfig.UsernameKey, etcdConfig.PasswordKey) if err != nil { logger.Errorf(err.Error()) } @@ -205,6 +209,7 @@ func newEtcdUserCache(cluster cluster.Cluster, etcdConfig *EtcdSpec) *etcdUserCa userFileObject: userFileObject, cluster: cluster, prefix: prefix, + usernameKey: etcdConfig.UsernameKey, passwordKey: etcdConfig.PasswordKey, cancel: cancel, stopCtx: stopCtx, @@ -214,31 +219,40 @@ func newEtcdUserCache(cluster cluster.Cluster, etcdConfig *EtcdSpec) *etcdUserCa } } -func parseYamlPW(entry string, key string) (string, bool) { - ok := false - var value interface{} +func parseYamlCreds(entry string) (map[string]interface{}, error) { + var err error defer func() { if err := recover(); err != nil { - logger.Errorf("Could not marshal credentials. Ensure that credentials are valid yaml.") - ok = false + err = fmt.Errorf("could not marshal credentials, ensure that credentials are valid yaml") } }() credentials := make(map[string]interface{}) yamltool.Unmarshal([]byte(entry), &credentials) - value, ok = credentials[key] - return value.(string), ok + return credentials, err } -func mapToReader(kvs map[string]string, yamlKey string, prefix string) (io.Reader, error) { +func kvsToReader(kvs map[string]string, usernameKey string, passwordKey string) (io.Reader, error) { + reader := bytes.NewReader([]byte("")) pwStrSlice := make([]string, 0, len(kvs)) - for key, yaml := range kvs { - pw, ok := parseYamlPW(yaml, yamlKey) + for _, yaml := range kvs { + credentials, err := parseYamlCreds(yaml) + if err != nil { + return reader, err + } + var ok bool + username, ok := credentials[usernameKey] + if !ok { + return reader, + fmt.Errorf("Parsing password updates failed. Make sure that '" + + usernameKey + "' is a valid yaml entry.") + } + password, ok := credentials[passwordKey] if !ok { - return bytes.NewReader([]byte("")), - fmt.Errorf("Parsing password updates failed. Make sure that '" + yamlKey + "' is a valid yaml entry.") + return reader, + fmt.Errorf("Parsing password updates failed. Make sure that '" + + passwordKey + "' is a valid yaml entry.") } - username := strings.TrimPrefix(key, prefix) - pwStrSlice = append(pwStrSlice, username+":"+pw) + pwStrSlice = append(pwStrSlice, username.(string)+":"+password.(string)) } stringData := strings.Join(pwStrSlice, "\n") return strings.NewReader(stringData), nil @@ -275,7 +289,7 @@ func (euc *etcdUserCache) WatchChanges() error { case <-euc.stopCtx.Done(): return nil case kvs := <-ch: - pwReader, err := mapToReader(kvs, euc.passwordKey, euc.prefix) + pwReader, err := kvsToReader(kvs, euc.usernameKey, euc.passwordKey) if err != nil { logger.Errorf(err.Error()) } diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 6344134c19..78a04e04ad 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -419,11 +419,11 @@ basicAuth: defer os.RemoveAll(etcdDirName) clusterInstance := createCluster(etcdDirName) - pwToYaml := func(pw string) string { - return fmt.Sprintf("password: %s", pw) + pwToYaml := func(user string, pw string) string { + return fmt.Sprintf("username: %s\npassword: %s", user, pw) } - clusterInstance.Put("/credentials/"+userIds[0], pwToYaml(encryptedPasswords[0])) - clusterInstance.Put("/credentials/"+userIds[2], pwToYaml(encryptedPasswords[2])) + clusterInstance.Put("/credentials/1", pwToYaml(userIds[0], encryptedPasswords[0])) + clusterInstance.Put("/credentials/2", pwToYaml(userIds[2], encryptedPasswords[2])) var mockMap sync.Map supervisor := supervisor.NewMock( @@ -435,6 +435,7 @@ name: validator basicAuth: etcd: prefix: /credentials/ + usernameKey: "username" passwordKey: "password"` expectedValid := []bool{true, false, true} @@ -455,13 +456,14 @@ basicAuth: } } - clusterInstance.Delete("/credentials/" + userIds[0]) // first user is not authorized anymore + clusterInstance.Delete("/credentials/1") // first user is not authorized anymore clusterInstance.Put("/credentials/doge", ` randomEntry1: 21 nestedEntry: key1: val1 password: doge +username: doge lastEntry: "byebye" `) From 616719652fbfca97aedf5769fa3407a7f3cf8982 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Tue, 11 Jan 2022 11:14:41 +0800 Subject: [PATCH 21/34] set x-auth-user header --- pkg/filter/validator/basicauth.go | 1 + pkg/filter/validator/validator_test.go | 3 +++ 2 files changed, 4 insertions(+) diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index dcc162c816..6a292141f0 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -360,6 +360,7 @@ func (bav *BasicAuthValidator) Validate(req httpcontext.HTTPRequest) error { } if bav.authorizedUsersCache.Match(userID, password) { + req.Header().Set("X-AUTH-USER", userID) return nil } return fmt.Errorf("unauthorized") diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 78a04e04ad..eb619278cf 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -489,6 +489,9 @@ lastEntry: "byebye" if result == resultInvalid { t.Errorf("should be authorized") } + if header.Get("X-AUTH-USER") != "doge" { + t.Errorf("x-auth-user header not set") + } v.Close() wg := &sync.WaitGroup{} From eeed6e98b739d02cbf799be2cc842ab98f9aac0e Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Tue, 11 Jan 2022 17:01:39 +0800 Subject: [PATCH 22/34] headerlookup filter --- pkg/cluster/test_util.go | 64 +++++++ pkg/filter/headerlookup/headerlookup.go | 179 +++++++++++++++++++ pkg/filter/headerlookup/headerlookup_test.go | 172 ++++++++++++++++++ pkg/filter/validator/validator_test.go | 36 +--- 4 files changed, 416 insertions(+), 35 deletions(-) create mode 100644 pkg/cluster/test_util.go create mode 100644 pkg/filter/headerlookup/headerlookup.go create mode 100644 pkg/filter/headerlookup/headerlookup_test.go diff --git a/pkg/cluster/test_util.go b/pkg/cluster/test_util.go new file mode 100644 index 0000000000..4a6d44b0b8 --- /dev/null +++ b/pkg/cluster/test_util.go @@ -0,0 +1,64 @@ +/* +* Copyright (c) 2017, MegaEase +* All rights reserved. +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +package cluster + +import ( + "fmt" + + "github.com/phayes/freeport" + + "github.com/megaease/easegress/pkg/env" + "github.com/megaease/easegress/pkg/option" +) + +func check(e error) { + if e != nil { + panic(e) + } +} + +// CreateClusterForTest creates a cluster for testing purposes. +func CreateClusterForTest(tempDir string) Cluster { + ports, err := freeport.GetFreePorts(3) + check(err) + name := fmt.Sprintf("test-member-x") + opt := option.New() + opt.Name = name + opt.ClusterName = "test-cluster" + opt.ClusterRole = "primary" + opt.ClusterRequestTimeout = "10s" + opt.Cluster.ListenClientURLs = []string{fmt.Sprintf("http://localhost:%d", ports[0])} + opt.Cluster.AdvertiseClientURLs = opt.Cluster.ListenClientURLs + opt.Cluster.ListenPeerURLs = []string{fmt.Sprintf("http://localhost:%d", ports[1])} + opt.Cluster.InitialAdvertisePeerURLs = opt.Cluster.ListenPeerURLs + opt.Cluster.InitialCluster = make(map[string]string) + opt.Cluster.InitialCluster[name] = opt.Cluster.InitialAdvertisePeerURLs[0] + opt.APIAddr = fmt.Sprintf("localhost:%d", ports[2]) + opt.DataDir = fmt.Sprintf("%s/data", tempDir) + opt.LogDir = fmt.Sprintf("%s/log", tempDir) + opt.MemberDir = fmt.Sprintf("%s/member", tempDir) + + _, err = opt.Parse() + check(err) + + env.InitServerDir(opt) + + clusterInstance, err := New(opt) + check(err) + return clusterInstance +} diff --git a/pkg/filter/headerlookup/headerlookup.go b/pkg/filter/headerlookup/headerlookup.go new file mode 100644 index 0000000000..45ca76a71f --- /dev/null +++ b/pkg/filter/headerlookup/headerlookup.go @@ -0,0 +1,179 @@ +/* +* Copyright (c) 2017, MegaEase +* All rights reserved. +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. + */ + +package headerlookup + +import ( + "fmt" + + "github.com/megaease/easegress/pkg/cluster" + "github.com/megaease/easegress/pkg/context" + "github.com/megaease/easegress/pkg/logger" + "github.com/megaease/easegress/pkg/object/httppipeline" + "github.com/megaease/easegress/pkg/util/yamltool" +) + +const ( + // Kind is the kind of HeaderLookup. + Kind = "HeaderLookup" +) + +var results = []string{} + +func init() { + httppipeline.Register(&HeaderLookup{}) +} + +type ( + // HeaderLookup retrieves values from etcd to headers. + HeaderLookup struct { + filterSpec *httppipeline.FilterSpec + spec *Spec + + cluster cluster.Cluster + } + + // HeaderSetterSpec defines etcd source key and request destination header. + HeaderSetterSpec struct { + EtcdKey string `yaml:"etcdKey,omitempty" jsonschema:"omitempty"` + HeaderKey string `yaml:"headerKey,omitempty" jsonschema:"omitempty"` + } + + // Spec defines header key and etcd prefix that form etcd key like {etcdPrefix}/{headerKey's value}. + // This {etcdPrefix}/{headerKey's value} is retrieved from etcd and HeaderSetters extract keys from the + // from the retrieved etcd item. + Spec struct { + HeaderKey string `yaml:"headerKey" jsonschema:"required"` + EtcdPrefix string `yaml:"etcdPrefix" jsonschema:"required"` + HeaderSetters []*HeaderSetterSpec `yaml:"headerSetters" jsonschema:"required"` + } +) + +// Validate validates spec. +func (spec Spec) Validate() error { + if spec.HeaderKey == "" { + return fmt.Errorf("headerKey is required") + } + if spec.EtcdPrefix == "" { + return fmt.Errorf("etcdPrefix is required") + } + if len(spec.HeaderSetters) < 1 { + return fmt.Errorf("at least one headerSetter is required") + } + for _, hs := range spec.HeaderSetters { + if hs.EtcdKey == "" { + return fmt.Errorf("headerSetters[i].etcdKey is required") + } + if hs.HeaderKey == "" { + return fmt.Errorf("headerSetters[i].headerKey is required") + } + } + return nil +} + +// Kind returns the kind of HeaderLookup. +func (hl *HeaderLookup) Kind() string { + return Kind +} + +// DefaultSpec returns the default spec of HeaderLookup. +func (hl *HeaderLookup) DefaultSpec() interface{} { + return &Spec{} +} + +// Description returns the description of HeaderLookup. +func (hl *HeaderLookup) Description() string { + return "HeaderLookup enriches request headers per request, looking up values from etcd." +} + +// Results returns the results of HeaderLookup. +func (hl *HeaderLookup) Results() []string { + return results +} + +// Init initializes HeaderLookup. +func (hl *HeaderLookup) Init(filterSpec *httppipeline.FilterSpec) { + hl.filterSpec, hl.spec = filterSpec, filterSpec.FilterSpec().(*Spec) + hl.cluster = filterSpec.Super().Cluster() +} + +// Inherit inherits previous generation of HeaderLookup. +func (hl *HeaderLookup) Inherit(filterSpec *httppipeline.FilterSpec, previousGeneration httppipeline.Filter) { + previousGeneration.Close() + hl.Init(filterSpec) +} + +func parseYamlCreds(entry string) (map[string]string, error) { + var err error + defer func() { + if err := recover(); err != nil { + err = fmt.Errorf("could not marshal custom-data, ensure that it's valid yaml") + } + }() + yamlMap := make(map[string]string) + yamltool.Unmarshal([]byte(entry), &yamlMap) + return yamlMap, err +} + +func (hl *HeaderLookup) lookup(headerVal string) (map[string]string, error) { + etcdKey := hl.spec.EtcdPrefix + headerVal + fmt.Println(etcdKey) + etcdVal, err := hl.cluster.Get(etcdKey) + if err != nil { + return nil, err + } + if etcdVal == nil { + return nil, fmt.Errorf("no data found") + } + result := make(map[string]string, len(hl.spec.HeaderSetters)) + etcdValues, err := parseYamlCreds(*etcdVal) + if err != nil { + return nil, err + } + for _, setter := range hl.spec.HeaderSetters { + if val, ok := etcdValues[setter.EtcdKey]; ok { + result[setter.HeaderKey] = val + } + } + return result, nil +} + +// Handle retrieves header values and sets request headers. +func (hl *HeaderLookup) Handle(ctx context.HTTPContext) string { + header := ctx.Request().Header() + headerVal := header.Get(hl.spec.HeaderKey) + if headerVal == "" { + logger.Warnf("request does not have header '%s'", hl.spec.HeaderKey) + return "" + } + headersToAdd, err := hl.lookup(headerVal) + if err != nil { + logger.Errorf(err.Error()) + return "" + } + fmt.Println(len(headersToAdd)) + for hk, hv := range headersToAdd { + header.Set(hk, hv) + } + return "" +} + +// Status returns status. +func (hl *HeaderLookup) Status() interface{} { return nil } + +// Close closes RequestAdaptor. +func (hl *HeaderLookup) Close() {} diff --git a/pkg/filter/headerlookup/headerlookup_test.go b/pkg/filter/headerlookup/headerlookup_test.go new file mode 100644 index 0000000000..2a4f15b545 --- /dev/null +++ b/pkg/filter/headerlookup/headerlookup_test.go @@ -0,0 +1,172 @@ +/* +* Copyright (c) 2017, MegaEase +* All rights reserved. +* +* Licensed under the Apache License, Version 2.0 (the "License"); +* you may not use this file except in compliance with the License. +* You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. + */ + +package headerlookup + +import ( + "io/ioutil" + "net/http" + "os" + "sync" + "testing" + + cluster "github.com/megaease/easegress/pkg/cluster" + "github.com/megaease/easegress/pkg/context/contexttest" + "github.com/megaease/easegress/pkg/logger" + "github.com/megaease/easegress/pkg/object/httppipeline" + "github.com/megaease/easegress/pkg/supervisor" + "github.com/megaease/easegress/pkg/util/httpheader" + "github.com/megaease/easegress/pkg/util/yamltool" +) + +func TestMain(m *testing.M) { + logger.InitNop() + code := m.Run() + os.Exit(code) +} + +func createHeaderLookup(yamlSpec string, supervisor *supervisor.Supervisor) (*HeaderLookup, error) { + rawSpec := make(map[string]interface{}) + yamltool.Unmarshal([]byte(yamlSpec), &rawSpec) + spec, err := httppipeline.NewFilterSpec(rawSpec, supervisor) + if err != nil { + return nil, err + } + hl := &HeaderLookup{} + hl.Init(spec) + hl.Inherit(spec, hl) + return hl, nil +} + +func check(e error) { + if e != nil { + panic(e) + } +} + +func TestValidate(t *testing.T) { + etcdDirName, err := ioutil.TempDir("", "etcd-headerlookup-test") + check(err) + defer os.RemoveAll(etcdDirName) + clusterInstance := cluster.CreateClusterForTest(etcdDirName) + var mockMap sync.Map + supervisor := supervisor.NewMock( + nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil) + + const validYaml = ` +name: headerLookup +kind: HeaderLookup +headerKey: "X-AUTH-USER" +etcdPrefix: "/credentials/" +headerSetters: + - etcdKey: "ext-id" + headerKey: "user-ext-id" +` + unvalidYamls := []string{ + ` +name: headerLookup +kind: HeaderLookup +`, + ` +name: headerLookup +kind: HeaderLookup +headerKey: "X-AUTH-USER" +`, + ` +name: headerLookup +kind: HeaderLookup +headerKey: "X-AUTH-USER" +etcdPrefix: "/credentials/" +`, + ` +name: headerLookup +kind: HeaderLookup +headerKey: "X-AUTH-USER" +etcdPrefix: "/credentials/" +headerSetters: + - etcdKey: "ext-id" +`, + } + + for _, unvalidYaml := range unvalidYamls { + if _, err := createHeaderLookup(unvalidYaml, supervisor); err == nil { + t.Errorf("validate should return error") + } + } + + if _, err := createHeaderLookup(validYaml, supervisor); err != nil { + t.Errorf("validate should not return error: %s", err.Error()) + } +} + +func TestHandle(t *testing.T) { + etcdDirName, err := ioutil.TempDir("", "etcd-headerlookup-test") + check(err) + defer os.RemoveAll(etcdDirName) + clusterInstance := cluster.CreateClusterForTest(etcdDirName) + var mockMap sync.Map + supervisor := supervisor.NewMock( + nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil) + + // let's put data to 'foobar' + clusterInstance.Put("/credentials/foobar", + ` +ext-id: 123456789 +extra-entry: "extra" +`) + + const config = ` +name: headerLookup +kind: HeaderLookup +headerKey: "X-AUTH-USER" +etcdPrefix: "/credentials/" +headerSetters: + - etcdKey: "ext-id" + headerKey: "user-ext-id" +` + hl, err := createHeaderLookup(config, supervisor) + check(err) + + // 'foobar' is the id + ctx := &contexttest.MockedHTTPContext{} + header := httpheader.New(http.Header{}) + ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { + return header + } + + hl.Handle(ctx) // does nothing as header missing + + if header.Get("user-ext-id") != "" { + t.Errorf("header should not be set") + } + + header.Set("X-AUTH-USER", "unknown-user") + + hl.Handle(ctx) // does nothing as user is missing + + if header.Get("user-ext-id") != "" { + t.Errorf("header should be set") + } + + header.Set("X-AUTH-USER", "foobar") + + hl.Handle(ctx) // now updates header + + if header.Get("user-ext-id") != "123456789" { + t.Errorf("header should be set") + } +} diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index eb619278cf..c4c0d1c40c 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -29,14 +29,10 @@ import ( "testing" "time" - "github.com/phayes/freeport" - cluster "github.com/megaease/easegress/pkg/cluster" "github.com/megaease/easegress/pkg/context/contexttest" - "github.com/megaease/easegress/pkg/env" "github.com/megaease/easegress/pkg/logger" "github.com/megaease/easegress/pkg/object/httppipeline" - "github.com/megaease/easegress/pkg/option" "github.com/megaease/easegress/pkg/supervisor" "github.com/megaease/easegress/pkg/util/httpheader" "github.com/megaease/easegress/pkg/util/yamltool" @@ -309,36 +305,6 @@ func check(e error) { } } -func createCluster(tempDir string) cluster.Cluster { - ports, err := freeport.GetFreePorts(3) - check(err) - name := fmt.Sprintf("test-member-x") - opt := option.New() - opt.Name = name - opt.ClusterName = "test-cluster" - opt.ClusterRole = "primary" - opt.ClusterRequestTimeout = "10s" - opt.Cluster.ListenClientURLs = []string{fmt.Sprintf("http://localhost:%d", ports[0])} - opt.Cluster.AdvertiseClientURLs = opt.Cluster.ListenClientURLs - opt.Cluster.ListenPeerURLs = []string{fmt.Sprintf("http://localhost:%d", ports[1])} - opt.Cluster.InitialAdvertisePeerURLs = opt.Cluster.ListenPeerURLs - opt.Cluster.InitialCluster = make(map[string]string) - opt.Cluster.InitialCluster[name] = opt.Cluster.InitialAdvertisePeerURLs[0] - opt.APIAddr = fmt.Sprintf("localhost:%d", ports[2]) - opt.DataDir = fmt.Sprintf("%s/data", tempDir) - opt.LogDir = fmt.Sprintf("%s/log", tempDir) - opt.MemberDir = fmt.Sprintf("%s/member", tempDir) - - _, err = opt.Parse() - check(err) - - env.InitServerDir(opt) - - clusterInstance, err := cluster.New(opt) - check(err) - return clusterInstance -} - func prepareCtxAndHeader() (*contexttest.MockedHTTPContext, http.Header) { ctx := &contexttest.MockedHTTPContext{} header := http.Header{} @@ -417,7 +383,7 @@ basicAuth: etcdDirName, err := ioutil.TempDir("", "etcd-validator-test") check(err) defer os.RemoveAll(etcdDirName) - clusterInstance := createCluster(etcdDirName) + clusterInstance := cluster.CreateClusterForTest(etcdDirName) pwToYaml := func(user string, pw string) string { return fmt.Sprintf("username: %s\npassword: %s", user, pw) From 4f2c3397d9ee9438212d1732d2f716636831ec84 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Tue, 11 Jan 2022 17:03:46 +0800 Subject: [PATCH 23/34] fix typo --- pkg/cluster/test_util.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cluster/test_util.go b/pkg/cluster/test_util.go index 4a6d44b0b8..cae77c79c0 100644 --- a/pkg/cluster/test_util.go +++ b/pkg/cluster/test_util.go @@ -13,7 +13,7 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. -*/ + */ package cluster From ccf9ccacea8f1a2e7b369c40a5c149b5dfbadfbd Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Tue, 11 Jan 2022 18:05:49 +0800 Subject: [PATCH 24/34] fix filter --- pkg/filter/headerlookup/headerlookup.go | 10 ++++++++-- pkg/registry/registry.go | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/pkg/filter/headerlookup/headerlookup.go b/pkg/filter/headerlookup/headerlookup.go index 45ca76a71f..b299af7560 100644 --- a/pkg/filter/headerlookup/headerlookup.go +++ b/pkg/filter/headerlookup/headerlookup.go @@ -108,7 +108,9 @@ func (hl *HeaderLookup) Results() []string { // Init initializes HeaderLookup. func (hl *HeaderLookup) Init(filterSpec *httppipeline.FilterSpec) { hl.filterSpec, hl.spec = filterSpec, filterSpec.FilterSpec().(*Spec) - hl.cluster = filterSpec.Super().Cluster() + if filterSpec.Super() != nil && filterSpec.Super().Cluster() != nil { + hl.cluster = filterSpec.Super().Cluster() + } } // Inherit inherits previous generation of HeaderLookup. @@ -154,6 +156,11 @@ func (hl *HeaderLookup) lookup(headerVal string) (map[string]string, error) { // Handle retrieves header values and sets request headers. func (hl *HeaderLookup) Handle(ctx context.HTTPContext) string { + result := hl.handle(ctx) + return ctx.CallNextHandler(result) +} + +func (hl *HeaderLookup) handle(ctx context.HTTPContext) string { header := ctx.Request().Header() headerVal := header.Get(hl.spec.HeaderKey) if headerVal == "" { @@ -165,7 +172,6 @@ func (hl *HeaderLookup) Handle(ctx context.HTTPContext) string { logger.Errorf(err.Error()) return "" } - fmt.Println(len(headersToAdd)) for hk, hv := range headersToAdd { header.Set(hk, hv) } diff --git a/pkg/registry/registry.go b/pkg/registry/registry.go index 0b962e4112..0277ee5758 100644 --- a/pkg/registry/registry.go +++ b/pkg/registry/registry.go @@ -26,6 +26,7 @@ import ( _ "github.com/megaease/easegress/pkg/filter/connectcontrol" _ "github.com/megaease/easegress/pkg/filter/corsadaptor" _ "github.com/megaease/easegress/pkg/filter/fallback" + _ "github.com/megaease/easegress/pkg/filter/headerlookup" _ "github.com/megaease/easegress/pkg/filter/kafka" _ "github.com/megaease/easegress/pkg/filter/meshadaptor" _ "github.com/megaease/easegress/pkg/filter/mock" From 82796cba7be9208c373e42aaca73bbb5246efca7 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Wed, 12 Jan 2022 08:33:04 +0800 Subject: [PATCH 25/34] sanitize header keys --- pkg/filter/headerlookup/headerlookup.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/filter/headerlookup/headerlookup.go b/pkg/filter/headerlookup/headerlookup.go index b299af7560..35c5c48a2e 100644 --- a/pkg/filter/headerlookup/headerlookup.go +++ b/pkg/filter/headerlookup/headerlookup.go @@ -19,6 +19,7 @@ package headerlookup import ( "fmt" + "net/http" "github.com/megaease/easegress/pkg/cluster" "github.com/megaease/easegress/pkg/context" @@ -162,7 +163,7 @@ func (hl *HeaderLookup) Handle(ctx context.HTTPContext) string { func (hl *HeaderLookup) handle(ctx context.HTTPContext) string { header := ctx.Request().Header() - headerVal := header.Get(hl.spec.HeaderKey) + headerVal := header.Get(http.CanonicalHeaderKey(hl.spec.HeaderKey)) if headerVal == "" { logger.Warnf("request does not have header '%s'", hl.spec.HeaderKey) return "" @@ -173,7 +174,7 @@ func (hl *HeaderLookup) handle(ctx context.HTTPContext) string { return "" } for hk, hv := range headersToAdd { - header.Set(hk, hv) + header.Set(http.CanonicalHeaderKey(hk), hv) } return "" } From 4bbe9aeb46b0c0e5ddcd10b073161ab2e9767182 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Wed, 12 Jan 2022 11:13:52 +0800 Subject: [PATCH 26/34] force basicAuth and headerLookup to use /custom-data/ etcd prefix --- pkg/filter/headerlookup/headerlookup.go | 10 +++++--- pkg/filter/headerlookup/headerlookup_test.go | 6 ++--- pkg/filter/validator/basicauth.go | 24 ++++++++++-------- pkg/filter/validator/validator_test.go | 26 ++++++++++++++++---- 4 files changed, 44 insertions(+), 22 deletions(-) diff --git a/pkg/filter/headerlookup/headerlookup.go b/pkg/filter/headerlookup/headerlookup.go index 35c5c48a2e..6985cd8d5a 100644 --- a/pkg/filter/headerlookup/headerlookup.go +++ b/pkg/filter/headerlookup/headerlookup.go @@ -20,6 +20,7 @@ package headerlookup import ( "fmt" "net/http" + "strings" "github.com/megaease/easegress/pkg/cluster" "github.com/megaease/easegress/pkg/context" @@ -31,6 +32,8 @@ import ( const ( // Kind is the kind of HeaderLookup. Kind = "HeaderLookup" + // customDataPrefix is prefix for lookup data. + customDataPrefix = "/custom-data/" ) var results = []string{} @@ -54,8 +57,8 @@ type ( HeaderKey string `yaml:"headerKey,omitempty" jsonschema:"omitempty"` } - // Spec defines header key and etcd prefix that form etcd key like {etcdPrefix}/{headerKey's value}. - // This {etcdPrefix}/{headerKey's value} is retrieved from etcd and HeaderSetters extract keys from the + // Spec defines header key and etcd prefix that form etcd key like /custom-data/{etcdPrefix}/{headerKey's value}. + // This /custom-data/{etcdPrefix}/{headerKey's value} is retrieved from etcd and HeaderSetters extract keys from the // from the retrieved etcd item. Spec struct { HeaderKey string `yaml:"headerKey" jsonschema:"required"` @@ -133,8 +136,7 @@ func parseYamlCreds(entry string) (map[string]string, error) { } func (hl *HeaderLookup) lookup(headerVal string) (map[string]string, error) { - etcdKey := hl.spec.EtcdPrefix + headerVal - fmt.Println(etcdKey) + etcdKey := customDataPrefix + strings.TrimPrefix(hl.spec.EtcdPrefix, "/") + headerVal etcdVal, err := hl.cluster.Get(etcdKey) if err != nil { return nil, err diff --git a/pkg/filter/headerlookup/headerlookup_test.go b/pkg/filter/headerlookup/headerlookup_test.go index 2a4f15b545..ea4d3c602a 100644 --- a/pkg/filter/headerlookup/headerlookup_test.go +++ b/pkg/filter/headerlookup/headerlookup_test.go @@ -71,7 +71,7 @@ func TestValidate(t *testing.T) { name: headerLookup kind: HeaderLookup headerKey: "X-AUTH-USER" -etcdPrefix: "/credentials/" +etcdPrefix: "credentials/" headerSetters: - etcdKey: "ext-id" headerKey: "user-ext-id" @@ -123,7 +123,7 @@ func TestHandle(t *testing.T) { nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil) // let's put data to 'foobar' - clusterInstance.Put("/credentials/foobar", + clusterInstance.Put("/custom-data/credentials/foobar", ` ext-id: 123456789 extra-entry: "extra" @@ -133,7 +133,7 @@ extra-entry: "extra" name: headerLookup kind: HeaderLookup headerKey: "X-AUTH-USER" -etcdPrefix: "/credentials/" +etcdPrefix: "credentials/" headerSetters: - etcdKey: "ext-id" headerKey: "user-ext-id" diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index 6a292141f0..6ebb994568 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -45,23 +45,23 @@ type ( // prefix: "/creds/" // usernameKey: "user" // passwordKey: "pw" - // expects the yaml to be stored with key /creds/{id} in following yaml (extra keys are allowed) + // expects the yaml to be stored with key /custom-data/creds/{id} in following yaml (extra keys are allowed) // user: doge - // pw: {encrypted password} + // pw: {encrypted or plain text password} EtcdSpec struct { Prefix string `yaml:"prefix" jsonschema:"onitempty"` UsernameKey string `yaml:"usernameKey" jsonschema:"omitempty"` PasswordKey string `yaml:"passwordKey" jsonschema:"omitempty"` } // BasicAuthValidatorSpec defines the configuration of Basic Auth validator. - // Only one of UserFile or EtcdYamlFormat should be defined. + // Only one of UserFile or Etcd should be defined. BasicAuthValidatorSpec struct { // UserFile is path to file containing encrypted user credentials in apache2-utils/htpasswd format. // To add user `userY`, use `sudo htpasswd /etc/apache2/.htpasswd userY` // Reference: https://manpages.debian.org/testing/apache2-utils/htpasswd.1.en.html#EXAMPLES UserFile string `yaml:"userFile" jsonschema:"omitempty"` - // When etcdYamlFormat is specified, verify user credentials from etcd. Etcd stores them: - // key: /credentials/{username} + // When etcd is specified, verify user credentials from etcd. Etcd stores them: + // key: /custom-data/{etcd.prefix}/{username} // value: {yaml string in format of etcd} Etcd *EtcdSpec `yaml:"etcd" jsonschema:"omitempty"` } @@ -101,7 +101,7 @@ type ( } ) -const credsPrefix = "/credentials/" +const customDataPrefix = "/custom-data/" func parseCredentials(creds string) (string, string, error) { parts := strings.Split(creds, ":") @@ -188,10 +188,13 @@ func (huc *htpasswdUserCache) Match(username string, password string) bool { } func newEtcdUserCache(cluster cluster.Cluster, etcdConfig *EtcdSpec) *etcdUserCache { - prefix := etcdConfig.Prefix - if prefix == "" { - prefix = "/custom-data/credentials/" + prefix := customDataPrefix + if etcdConfig.Prefix == "" { + prefix += "credentials/" + } else { + prefix += strings.TrimPrefix(etcdConfig.Prefix, "/") } + logger.Infof("credentials etcd prefix %s", prefix) kvs, err := cluster.GetPrefix(prefix) if err != nil { panic(err) @@ -269,7 +272,7 @@ func (euc *etcdUserCache) WatchChanges() error { syncer, err = euc.cluster.Syncer(euc.syncInterval) if err != nil { logger.Errorf("failed to create syncer: %v", err) - } else if ch, err = syncer.SyncPrefix(credsPrefix); err != nil { + } else if ch, err = syncer.SyncPrefix(euc.prefix); err != nil { logger.Errorf("failed to sync prefix: %v", err) syncer.Close() } else { @@ -289,6 +292,7 @@ func (euc *etcdUserCache) WatchChanges() error { case <-euc.stopCtx.Done(): return nil case kvs := <-ch: + logger.Infof("basic auth credentials update") pwReader, err := kvsToReader(kvs, euc.usernameKey, euc.passwordKey) if err != nil { logger.Errorf(err.Error()) diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index c4c0d1c40c..eba5ee0cee 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -379,17 +379,33 @@ basicAuth: os.Remove(userFile.Name()) v.Close() }) + t.Run("credentials from etcd", func(t *testing.T) { etcdDirName, err := ioutil.TempDir("", "etcd-validator-test") check(err) defer os.RemoveAll(etcdDirName) clusterInstance := cluster.CreateClusterForTest(etcdDirName) + // Test newEtcdUserCache + if euc := newEtcdUserCache(clusterInstance, &EtcdSpec{ + UsernameKey: "user", + PasswordKey: "pass", + }); euc.prefix != "/custom-data/credentials/" { + t.Errorf("newEtcdUserCache failed") + } + if euc := newEtcdUserCache(clusterInstance, &EtcdSpec{ + Prefix: "/extra-slash/", + UsernameKey: "user", + PasswordKey: "pass", + }); euc.prefix != "/custom-data/extra-slash/" { + t.Errorf("newEtcdUserCache failed") + } + pwToYaml := func(user string, pw string) string { return fmt.Sprintf("username: %s\npassword: %s", user, pw) } - clusterInstance.Put("/credentials/1", pwToYaml(userIds[0], encryptedPasswords[0])) - clusterInstance.Put("/credentials/2", pwToYaml(userIds[2], encryptedPasswords[2])) + clusterInstance.Put("/custom-data/credentials/1", pwToYaml(userIds[0], encryptedPasswords[0])) + clusterInstance.Put("/custom-data/credentials/2", pwToYaml(userIds[2], encryptedPasswords[2])) var mockMap sync.Map supervisor := supervisor.NewMock( @@ -400,7 +416,7 @@ kind: Validator name: validator basicAuth: etcd: - prefix: /credentials/ + prefix: credentials/ usernameKey: "username" passwordKey: "password"` @@ -422,8 +438,8 @@ basicAuth: } } - clusterInstance.Delete("/credentials/1") // first user is not authorized anymore - clusterInstance.Put("/credentials/doge", + clusterInstance.Delete("/custom-data/credentials/1") // first user is not authorized anymore + clusterInstance.Put("/custom-data/credentials/doge", ` randomEntry1: 21 nestedEntry: From bc7ff35cfd0b86cafe901ad6a3845a6527c67846 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Wed, 12 Jan 2022 15:52:50 +0800 Subject: [PATCH 27/34] simplify configuration by fixing key and password entries; allow skipping unvalid etcd entries --- doc/cookbook/security.md | 2 + doc/reference/filters.md | 2 + pkg/filter/validator/basicauth.go | 109 +++++++++++++------------ pkg/filter/validator/validator_test.go | 24 ++---- 4 files changed, 70 insertions(+), 67 deletions(-) diff --git a/doc/cookbook/security.md b/doc/cookbook/security.md index 7e0ff7447c..dee108b45f 100644 --- a/doc/cookbook/security.md +++ b/doc/cookbook/security.md @@ -156,6 +156,7 @@ filters: - kind: Validator name: oauth-validator basicAuth: + mode: "FILE" userFile: '/etc/apache2/.htpasswd' - name: proxy kind: Proxy @@ -282,6 +283,7 @@ filters: - kind: Validator name: basic-auth-validator basicAuth: + mode: "FILE" userFile: '/etc/apache2/.htpasswd' - name: proxy kind: Proxy diff --git a/doc/reference/filters.md b/doc/reference/filters.md index 31a28b1769..8975c80d87 100644 --- a/doc/reference/filters.md +++ b/doc/reference/filters.md @@ -597,6 +597,7 @@ Here's an example for `basicAuth` validation method which uses [Apache2 htpasswd kind: Validator name: basicAuth-validator-example basicAuth: + mode: "FILE" userFile: /etc/apache2/.htpasswd ``` @@ -608,6 +609,7 @@ basicAuth: | jwt | [validator.JWTValidatorSpec](#validatorJWTValidatorSpec) | JWT validation rule, validates JWT token string from the `Authorization` header or cookies | No | | signature | [signer.Spec](#signerSpec) | Signature validation rule, implements an [Amazon Signature V4](https://docs.aws.amazon.com/general/latest/gr/sigv4_signing.html) compatible signature validation validator, with customizable literal strings | No | | oauth2 | [validator.OAuth2ValidatorSpec](#validatorOAuth2ValidatorSpec) | The `OAuth/2` method support `Token Introspection` mode and `Self-Encoded Access Tokens` mode, only one mode can be configured at a time | No | +| basicAuth | [basicauth.BasicAuthValidatorSpec](#basicauthBasicAuthValidatorSpec) | The `BasicAuth` method support `FILE` mode and `ETCD` mode, only one mode can be configured at a time. | No | ### Results diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index 6ebb994568..39be67d72f 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -41,29 +41,22 @@ import ( ) type ( - // EtcdSpec defines etcd prefix and which yaml entries are the username and password. For example spec - // prefix: "/creds/" - // usernameKey: "user" - // passwordKey: "pw" - // expects the yaml to be stored with key /custom-data/creds/{id} in following yaml (extra keys are allowed) - // user: doge - // pw: {encrypted or plain text password} - EtcdSpec struct { - Prefix string `yaml:"prefix" jsonschema:"onitempty"` - UsernameKey string `yaml:"usernameKey" jsonschema:"omitempty"` - PasswordKey string `yaml:"passwordKey" jsonschema:"omitempty"` - } // BasicAuthValidatorSpec defines the configuration of Basic Auth validator. - // Only one of UserFile or Etcd should be defined. + // There are 'file' and 'etcd' modes. BasicAuthValidatorSpec struct { + Mode string `yaml:"mode" jsonschema:"omitempty,enum=FILE,enum=ETCD"` + // Required for 'FILE' mode. // UserFile is path to file containing encrypted user credentials in apache2-utils/htpasswd format. // To add user `userY`, use `sudo htpasswd /etc/apache2/.htpasswd userY` // Reference: https://manpages.debian.org/testing/apache2-utils/htpasswd.1.en.html#EXAMPLES UserFile string `yaml:"userFile" jsonschema:"omitempty"` - // When etcd is specified, verify user credentials from etcd. Etcd stores them: - // key: /custom-data/{etcd.prefix}/{username} - // value: {yaml string in format of etcd} - Etcd *EtcdSpec `yaml:"etcd" jsonschema:"omitempty"` + // Required for 'ETCD' mode. + // When EtcdPrefix is specified, verify user credentials from etcd. Etcd should store them: + // key: /custom-data/{etcdPrefix}/{$key} + // value: + // key: "$key" + // password: "$password" + EtcdPrefix string `yaml:"etcdPrefix" jsonschema:"omitempty"` } // AuthorizedUsersCache provides cached lookup for authorized users. @@ -87,8 +80,6 @@ type ( userFileObject *htpasswd.File cluster cluster.Cluster prefix string - usernameKey string - passwordKey string syncInterval time.Duration stopCtx context.Context cancel context.CancelFunc @@ -101,7 +92,11 @@ type ( } ) -const customDataPrefix = "/custom-data/" +const ( + customDataPrefix = "/custom-data/" + etcdUsernameKey = "key" + etcdPasswordKey = "password" +) func parseCredentials(creds string) (string, string, error) { parts := strings.Split(creds, ":") @@ -117,6 +112,9 @@ func bcryptHash(data []byte) (string, error) { } func newHtpasswdUserCache(userFile string, syncInterval time.Duration) *htpasswdUserCache { + if userFile == "" { + userFile = "/etc/apache2/.htpasswd" + } stopCtx, cancel := context.WithCancel(context.Background()) userFileObject, err := htpasswd.New(userFile, htpasswd.DefaultSystems, nil) if err != nil { @@ -187,33 +185,30 @@ func (huc *htpasswdUserCache) Match(username string, password string) bool { return huc.userFileObject.Match(username, password) } -func newEtcdUserCache(cluster cluster.Cluster, etcdConfig *EtcdSpec) *etcdUserCache { +func newEtcdUserCache(cluster cluster.Cluster, etcdPrefix string) *etcdUserCache { prefix := customDataPrefix - if etcdConfig.Prefix == "" { + if etcdPrefix == "" { prefix += "credentials/" } else { - prefix += strings.TrimPrefix(etcdConfig.Prefix, "/") + prefix = customDataPrefix + strings.TrimPrefix(etcdPrefix, "/") } logger.Infof("credentials etcd prefix %s", prefix) kvs, err := cluster.GetPrefix(prefix) - if err != nil { - panic(err) - } - pwReader, err := kvsToReader(kvs, etcdConfig.UsernameKey, etcdConfig.PasswordKey) if err != nil { logger.Errorf(err.Error()) + return &etcdUserCache{} } + pwReader := kvsToReader(kvs) userFileObject, err := htpasswd.NewFromReader(pwReader, htpasswd.DefaultSystems, nil) if err != nil { - panic(err) + logger.Errorf(err.Error()) + return &etcdUserCache{} } stopCtx, cancel := context.WithCancel(context.Background()) return &etcdUserCache{ userFileObject: userFileObject, cluster: cluster, prefix: prefix, - usernameKey: etcdConfig.UsernameKey, - passwordKey: etcdConfig.PasswordKey, cancel: cancel, stopCtx: stopCtx, // cluster.Syncer updates changes (removed access or updated passwords) immediately. @@ -234,34 +229,42 @@ func parseYamlCreds(entry string) (map[string]interface{}, error) { return credentials, err } -func kvsToReader(kvs map[string]string, usernameKey string, passwordKey string) (io.Reader, error) { - reader := bytes.NewReader([]byte("")) +func kvsToReader(kvs map[string]string) io.Reader { pwStrSlice := make([]string, 0, len(kvs)) for _, yaml := range kvs { credentials, err := parseYamlCreds(yaml) if err != nil { - return reader, err + logger.Errorf(err.Error()) + continue } var ok bool - username, ok := credentials[usernameKey] + username, ok := credentials[etcdUsernameKey] if !ok { - return reader, - fmt.Errorf("Parsing password updates failed. Make sure that '" + - usernameKey + "' is a valid yaml entry.") + logger.Errorf("Parsing credential updates failed. Make sure that credentials contains '" + + etcdUsernameKey + "' entry.") + continue } - password, ok := credentials[passwordKey] + password, ok := credentials[etcdPasswordKey] if !ok { - return reader, - fmt.Errorf("Parsing password updates failed. Make sure that '" + - passwordKey + "' is a valid yaml entry.") + logger.Errorf("Parsing credential updates failed. Make sure that credentials contains '" + + etcdPasswordKey + "' entry.") + continue } pwStrSlice = append(pwStrSlice, username.(string)+":"+password.(string)) } + if len(pwStrSlice) == 0 { + // no credentials found, let's return empty reader + return bytes.NewReader([]byte("")) + } stringData := strings.Join(pwStrSlice, "\n") - return strings.NewReader(stringData), nil + return strings.NewReader(stringData) } func (euc *etcdUserCache) WatchChanges() error { + if euc.prefix == "" { + logger.Errorf("missing etcd prefix, skip watching changes") + return nil + } var ( syncer *cluster.Syncer err error @@ -293,10 +296,7 @@ func (euc *etcdUserCache) WatchChanges() error { return nil case kvs := <-ch: logger.Infof("basic auth credentials update") - pwReader, err := kvsToReader(kvs, euc.usernameKey, euc.passwordKey) - if err != nil { - logger.Errorf(err.Error()) - } + pwReader := kvsToReader(kvs) euc.userFileObject.ReloadFromReader(pwReader, nil) } } @@ -304,27 +304,34 @@ func (euc *etcdUserCache) WatchChanges() error { } func (euc *etcdUserCache) Close() { + if euc.prefix == "" { + return + } euc.cancel() } func (euc *etcdUserCache) Refresh() error { return nil } func (euc *etcdUserCache) Match(username string, password string) bool { + if euc.prefix == "" { + return false + } return euc.userFileObject.Match(username, password) } // NewBasicAuthValidator creates a new Basic Auth validator func NewBasicAuthValidator(spec *BasicAuthValidatorSpec, supervisor *supervisor.Supervisor) *BasicAuthValidator { var cache AuthorizedUsersCache - if spec.Etcd != nil { + switch spec.Mode { + case "ETCD": if supervisor == nil || supervisor.Cluster() == nil { logger.Errorf("BasicAuth validator : failed to read data from etcd") - } else { - cache = newEtcdUserCache(supervisor.Cluster(), spec.Etcd) + return nil } - } else if spec.UserFile != "" { + cache = newEtcdUserCache(supervisor.Cluster(), spec.EtcdPrefix) + case "FILE": cache = newHtpasswdUserCache(spec.UserFile, 1*time.Minute) - } else { + default: logger.Errorf("BasicAuth validator spec unvalid.") return nil } diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index eba5ee0cee..7984f3ae9f 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -338,6 +338,7 @@ func TestBasicAuth(t *testing.T) { kind: Validator name: validator basicAuth: + mode: FILE userFile: ` + userFile.Name() userFile.Write( @@ -387,22 +388,15 @@ basicAuth: clusterInstance := cluster.CreateClusterForTest(etcdDirName) // Test newEtcdUserCache - if euc := newEtcdUserCache(clusterInstance, &EtcdSpec{ - UsernameKey: "user", - PasswordKey: "pass", - }); euc.prefix != "/custom-data/credentials/" { + if euc := newEtcdUserCache(clusterInstance, ""); euc.prefix != "/custom-data/credentials/" { t.Errorf("newEtcdUserCache failed") } - if euc := newEtcdUserCache(clusterInstance, &EtcdSpec{ - Prefix: "/extra-slash/", - UsernameKey: "user", - PasswordKey: "pass", - }); euc.prefix != "/custom-data/extra-slash/" { + if euc := newEtcdUserCache(clusterInstance, "/extra-slash/"); euc.prefix != "/custom-data/extra-slash/" { t.Errorf("newEtcdUserCache failed") } pwToYaml := func(user string, pw string) string { - return fmt.Sprintf("username: %s\npassword: %s", user, pw) + return fmt.Sprintf("key: %s\npassword: %s", user, pw) } clusterInstance.Put("/custom-data/credentials/1", pwToYaml(userIds[0], encryptedPasswords[0])) clusterInstance.Put("/custom-data/credentials/2", pwToYaml(userIds[2], encryptedPasswords[2])) @@ -415,11 +409,9 @@ basicAuth: kind: Validator name: validator basicAuth: - etcd: - prefix: credentials/ - usernameKey: "username" - passwordKey: "password"` - + mode: ETCD + etcdPrefix: credentials/ +` expectedValid := []bool{true, false, true} v := createValidator(yamlSpec, nil, supervisor) for i := 0; i < 3; i++ { @@ -445,7 +437,7 @@ randomEntry1: 21 nestedEntry: key1: val1 password: doge -username: doge +key: doge lastEntry: "byebye" `) From 8cd84219a39ef7f7e680bf7bcb8f0c41c99ea77c Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 13 Jan 2022 13:17:43 +0800 Subject: [PATCH 28/34] use fsnotify for basic auth userfile updates --- go.mod | 1 + pkg/filter/validator/basicauth.go | 124 ++++++++++++------------- pkg/filter/validator/validator_test.go | 73 ++++++++++++--- 3 files changed, 118 insertions(+), 80 deletions(-) diff --git a/go.mod b/go.mod index d6d86b7807..c28e872bc1 100644 --- a/go.mod +++ b/go.mod @@ -13,6 +13,7 @@ require ( github.com/facebookgo/stack v0.0.0-20160209184415-751773369052 // indirect github.com/facebookgo/subset v0.0.0-20200203212716-c811ad88dec4 // indirect github.com/fatih/color v1.12.0 + github.com/fsnotify/fsnotify v1.4.9 // indirect github.com/ghodss/yaml v1.0.0 github.com/go-chi/chi/v5 v5.0.3 github.com/go-zookeeper/zk v1.0.2 diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index 39be67d72f..17bf27debc 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -23,12 +23,10 @@ import ( "encoding/base64" "fmt" "io" - "io/fs" - "os" "strings" - "sync" "time" + "github.com/fsnotify/fsnotify" "github.com/tg123/go-htpasswd" "golang.org/x/crypto/bcrypt" @@ -62,15 +60,14 @@ type ( // AuthorizedUsersCache provides cached lookup for authorized users. AuthorizedUsersCache interface { Match(string, string) bool - WatchChanges() error - Refresh() error + WatchChanges() Close() } htpasswdUserCache struct { userFile string userFileObject *htpasswd.File - fileMutex sync.RWMutex + watcher *fsnotify.Watcher syncInterval time.Duration stopCtx context.Context cancel context.CancelFunc @@ -118,67 +115,59 @@ func newHtpasswdUserCache(userFile string, syncInterval time.Duration) *htpasswd stopCtx, cancel := context.WithCancel(context.Background()) userFileObject, err := htpasswd.New(userFile, htpasswd.DefaultSystems, nil) if err != nil { - panic(err) + logger.Errorf(err.Error()) + userFileObject = nil + } + watcher, err := fsnotify.NewWatcher() + if err != nil { + logger.Errorf(err.Error()) + watcher = nil } return &htpasswdUserCache{ userFile: userFile, stopCtx: stopCtx, cancel: cancel, + watcher: watcher, userFileObject: userFileObject, // Removed access or updated passwords are updated according syncInterval. syncInterval: syncInterval, } } -// Refresh reloads users from userFile. -func (huc *htpasswdUserCache) Refresh() error { - huc.fileMutex.RLock() - err := huc.userFileObject.Reload(nil) - huc.fileMutex.RUnlock() - return err -} - -func (huc *htpasswdUserCache) WatchChanges() error { - getFileStat := func() (fs.FileInfo, error) { - huc.fileMutex.RLock() - stat, err := os.Stat(huc.userFile) - huc.fileMutex.RUnlock() - return stat, err - } - - initialStat, err := getFileStat() - if err != nil { - return err +func (huc *htpasswdUserCache) WatchChanges() { + if huc.userFileObject == nil || huc.watcher == nil { + return } - for { - stat, err := getFileStat() - if err != nil { - return err - } - if stat.Size() != initialStat.Size() || stat.ModTime() != initialStat.ModTime() { - err := huc.Refresh() - if err != nil { - return err + go func() { + for { + select { + case _, ok := <-huc.watcher.Events: + if !ok { + return + } + err := huc.userFileObject.Reload(nil) + if err != nil { + logger.Errorf(err.Error()) + } + case err, ok := <-huc.watcher.Errors: + if !ok { + return + } + logger.Errorf(err.Error()) } - - // reset initial stat and watch for next modification - initialStat, err = getFileStat() - if err != nil { - return err - } - } - select { - case <-time.After(huc.syncInterval): - continue - case <-huc.stopCtx.Done(): - return nil } + }() + err := huc.watcher.Add(huc.userFile) + if err != nil { + logger.Errorf(err.Error()) } - return nil + return } func (huc *htpasswdUserCache) Close() { - huc.cancel() + if huc.watcher != nil { + huc.watcher.Close() + } } func (huc *htpasswdUserCache) Match(username string, password string) bool { @@ -260,10 +249,10 @@ func kvsToReader(kvs map[string]string) io.Reader { return strings.NewReader(stringData) } -func (euc *etcdUserCache) WatchChanges() error { +func (euc *etcdUserCache) WatchChanges() { if euc.prefix == "" { logger.Errorf("missing etcd prefix, skip watching changes") - return nil + return } var ( syncer *cluster.Syncer @@ -285,22 +274,25 @@ func (euc *etcdUserCache) WatchChanges() error { select { case <-time.After(10 * time.Second): case <-euc.stopCtx.Done(): - return nil + return } } - defer syncer.Close() - - for { - select { - case <-euc.stopCtx.Done(): - return nil - case kvs := <-ch: - logger.Infof("basic auth credentials update") - pwReader := kvsToReader(kvs) - euc.userFileObject.ReloadFromReader(pwReader, nil) + // start listening in background + go func() { + defer syncer.Close() + + for { + select { + case <-euc.stopCtx.Done(): + return + case kvs := <-ch: + logger.Infof("basic auth credentials update") + pwReader := kvsToReader(kvs) + euc.userFileObject.ReloadFromReader(pwReader, nil) + } } - } - return nil + }() + return } func (euc *etcdUserCache) Close() { @@ -310,8 +302,6 @@ func (euc *etcdUserCache) Close() { euc.cancel() } -func (euc *etcdUserCache) Refresh() error { return nil } - func (euc *etcdUserCache) Match(username string, password string) bool { if euc.prefix == "" { return false @@ -335,7 +325,7 @@ func NewBasicAuthValidator(spec *BasicAuthValidatorSpec, supervisor *supervisor. logger.Errorf("BasicAuth validator spec unvalid.") return nil } - go cache.WatchChanges() + cache.WatchChanges() bav := &BasicAuthValidator{ spec: spec, authorizedUsersCache: cache, diff --git a/pkg/filter/validator/validator_test.go b/pkg/filter/validator/validator_test.go index 7984f3ae9f..0d569b082e 100644 --- a/pkg/filter/validator/validator_test.go +++ b/pkg/filter/validator/validator_test.go @@ -314,6 +314,14 @@ func prepareCtxAndHeader() (*contexttest.MockedHTTPContext, http.Header) { return ctx, header } +func cleanFile(userFile *os.File) { + err := userFile.Truncate(0) + check(err) + _, err = userFile.Seek(0, 0) + check(err) + userFile.Write([]byte("")) +} + func TestBasicAuth(t *testing.T) { userIds := []string{ "userY", "userZ", "nonExistingUser", @@ -330,6 +338,19 @@ func TestBasicAuth(t *testing.T) { encrypt("userpasswordY"), encrypt("userpasswordZ"), encrypt("userpasswordX"), } + t.Run("unexisting userFile", func(t *testing.T) { + yamlSpec := ` +kind: Validator +name: validator +basicAuth: + mode: FILE + userFile: unexisting-file` + v := createValidator(yamlSpec, nil, nil) + ctx, _ := prepareCtxAndHeader() + if v.Handle(ctx) != resultInvalid { + t.Errorf("should be invalid") + } + }) t.Run("credentials from userFile", func(t *testing.T) { userFile, err := os.CreateTemp("", "apache2-htpasswd") check(err) @@ -341,11 +362,21 @@ basicAuth: mode: FILE userFile: ` + userFile.Name() + // test invalid format + userFile.Write([]byte("keypass")) + v := createValidator(yamlSpec, nil, nil) + ctx, _ := prepareCtxAndHeader() + if v.Handle(ctx) != resultInvalid { + t.Errorf("should be invalid") + } + + // now proper format + cleanFile(userFile) userFile.Write( []byte(userIds[0] + ":" + encryptedPasswords[0] + "\n" + userIds[1] + ":" + encryptedPasswords[1])) expectedValid := []bool{true, true, false} - v := createValidator(yamlSpec, nil, nil) + v = createValidator(yamlSpec, nil, nil) for i := 0; i < 3; i++ { ctx, header := prepareCtxAndHeader() b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[i] + ":" + passwords[i])) @@ -362,25 +393,41 @@ basicAuth: } } - err = userFile.Truncate(0) - check(err) - _, err = userFile.Seek(0, 0) - check(err) - userFile.Write([]byte("")) // no more authorized users - v.basicAuth.authorizedUsersCache.Refresh() + cleanFile(userFile) // no more authorized users - ctx, header := prepareCtxAndHeader() - b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[0] + ":" + passwords[0])) - header.Set("Authorization", "Basic "+b64creds) - result := v.Handle(ctx) - if result != resultInvalid { - t.Errorf("should be unauthorized") + tryCount := 5 + for i := 0; i <= tryCount; i++ { + time.Sleep(200 * time.Millisecond) // wait that cache item gets deleted + ctx, header := prepareCtxAndHeader() + b64creds := base64.StdEncoding.EncodeToString([]byte(userIds[0] + ":" + passwords[0])) + header.Set("Authorization", "Basic "+b64creds) + result := v.Handle(ctx) + if result == resultInvalid { + break // successfully unauthorized + } + if i == tryCount && result != resultInvalid { + t.Errorf("should be unauthorized") + } } os.Remove(userFile.Name()) v.Close() }) + t.Run("test kvsToReader", func(t *testing.T) { + kvs := make(map[string]string) + kvs["/creds/key1"] = "key: key1\npass: pw" // invalid + kvs["/creds/key2"] = "ky: key2\npassword: pw" // invalid + kvs["/creds/key3"] = "key: key3\npassword: pw" // valid + reader := kvsToReader(kvs) + b, err := io.ReadAll(reader) + check(err) + s := string(b) + if s != "key3:pw" { + t.Errorf("parsing failed, %s", s) + } + }) + t.Run("credentials from etcd", func(t *testing.T) { etcdDirName, err := ioutil.TempDir("", "etcd-validator-test") check(err) From 68d7cf85345bbff96c0deb0e5bbc5fe525086342 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 13 Jan 2022 13:39:47 +0800 Subject: [PATCH 29/34] go mod tidy --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index c28e872bc1..989923a879 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/facebookgo/stack v0.0.0-20160209184415-751773369052 // indirect github.com/facebookgo/subset v0.0.0-20200203212716-c811ad88dec4 // indirect github.com/fatih/color v1.12.0 - github.com/fsnotify/fsnotify v1.4.9 // indirect + github.com/fsnotify/fsnotify v1.4.9 github.com/ghodss/yaml v1.0.0 github.com/go-chi/chi/v5 v5.0.3 github.com/go-zookeeper/zk v1.0.2 From b4442d6b19f674d4d85d14bf4a742d25f23cbda2 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 13 Jan 2022 15:18:46 +0800 Subject: [PATCH 30/34] address review comments --- pkg/filter/headerlookup/headerlookup.go | 27 +++----- pkg/filter/headerlookup/headerlookup_test.go | 71 ++++++++++---------- pkg/filter/validator/basicauth.go | 44 +++++------- 3 files changed, 61 insertions(+), 81 deletions(-) diff --git a/pkg/filter/headerlookup/headerlookup.go b/pkg/filter/headerlookup/headerlookup.go index 6985cd8d5a..899c761d43 100644 --- a/pkg/filter/headerlookup/headerlookup.go +++ b/pkg/filter/headerlookup/headerlookup.go @@ -22,11 +22,12 @@ import ( "net/http" "strings" + yaml "gopkg.in/yaml.v2" + "github.com/megaease/easegress/pkg/cluster" "github.com/megaease/easegress/pkg/context" "github.com/megaease/easegress/pkg/logger" "github.com/megaease/easegress/pkg/object/httppipeline" - "github.com/megaease/easegress/pkg/util/yamltool" ) const ( @@ -47,6 +48,8 @@ type ( HeaderLookup struct { filterSpec *httppipeline.FilterSpec spec *Spec + etcdPrefix string + headerKey string cluster cluster.Cluster } @@ -115,6 +118,8 @@ func (hl *HeaderLookup) Init(filterSpec *httppipeline.FilterSpec) { if filterSpec.Super() != nil && filterSpec.Super().Cluster() != nil { hl.cluster = filterSpec.Super().Cluster() } + hl.etcdPrefix = customDataPrefix + strings.TrimPrefix(hl.spec.EtcdPrefix, "/") + hl.headerKey = http.CanonicalHeaderKey(hl.spec.HeaderKey) } // Inherit inherits previous generation of HeaderLookup. @@ -123,21 +128,8 @@ func (hl *HeaderLookup) Inherit(filterSpec *httppipeline.FilterSpec, previousGen hl.Init(filterSpec) } -func parseYamlCreds(entry string) (map[string]string, error) { - var err error - defer func() { - if err := recover(); err != nil { - err = fmt.Errorf("could not marshal custom-data, ensure that it's valid yaml") - } - }() - yamlMap := make(map[string]string) - yamltool.Unmarshal([]byte(entry), &yamlMap) - return yamlMap, err -} - func (hl *HeaderLookup) lookup(headerVal string) (map[string]string, error) { - etcdKey := customDataPrefix + strings.TrimPrefix(hl.spec.EtcdPrefix, "/") + headerVal - etcdVal, err := hl.cluster.Get(etcdKey) + etcdVal, err := hl.cluster.Get(hl.etcdPrefix + headerVal) if err != nil { return nil, err } @@ -145,7 +137,8 @@ func (hl *HeaderLookup) lookup(headerVal string) (map[string]string, error) { return nil, fmt.Errorf("no data found") } result := make(map[string]string, len(hl.spec.HeaderSetters)) - etcdValues, err := parseYamlCreds(*etcdVal) + etcdValues := make(map[string]string) + err = yaml.Unmarshal([]byte(*etcdVal), etcdValues) if err != nil { return nil, err } @@ -165,7 +158,7 @@ func (hl *HeaderLookup) Handle(ctx context.HTTPContext) string { func (hl *HeaderLookup) handle(ctx context.HTTPContext) string { header := ctx.Request().Header() - headerVal := header.Get(http.CanonicalHeaderKey(hl.spec.HeaderKey)) + headerVal := header.Get(hl.headerKey) if headerVal == "" { logger.Warnf("request does not have header '%s'", hl.spec.HeaderKey) return "" diff --git a/pkg/filter/headerlookup/headerlookup_test.go b/pkg/filter/headerlookup/headerlookup_test.go index ea4d3c602a..24564c84a1 100644 --- a/pkg/filter/headerlookup/headerlookup_test.go +++ b/pkg/filter/headerlookup/headerlookup_test.go @@ -117,18 +117,6 @@ func TestHandle(t *testing.T) { etcdDirName, err := ioutil.TempDir("", "etcd-headerlookup-test") check(err) defer os.RemoveAll(etcdDirName) - clusterInstance := cluster.CreateClusterForTest(etcdDirName) - var mockMap sync.Map - supervisor := supervisor.NewMock( - nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil) - - // let's put data to 'foobar' - clusterInstance.Put("/custom-data/credentials/foobar", - ` -ext-id: 123456789 -extra-entry: "extra" -`) - const config = ` name: headerLookup kind: HeaderLookup @@ -138,35 +126,48 @@ headerSetters: - etcdKey: "ext-id" headerKey: "user-ext-id" ` - hl, err := createHeaderLookup(config, supervisor) - check(err) - - // 'foobar' is the id - ctx := &contexttest.MockedHTTPContext{} - header := httpheader.New(http.Header{}) - ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { - return header - } + t.Run("handle", func(t *testing.T) { + clusterInstance := cluster.CreateClusterForTest(etcdDirName) + var mockMap sync.Map + supervisor := supervisor.NewMock( + nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil) + + // let's put data to 'foobar' + clusterInstance.Put("/custom-data/credentials/foobar", + ` +ext-id: 123456789 +extra-entry: "extra" +`) + hl, err := createHeaderLookup(config, supervisor) + check(err) + + // 'foobar' is the id + ctx := &contexttest.MockedHTTPContext{} + header := httpheader.New(http.Header{}) + ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { + return header + } - hl.Handle(ctx) // does nothing as header missing + hl.Handle(ctx) // does nothing as header missing - if header.Get("user-ext-id") != "" { - t.Errorf("header should not be set") - } + if header.Get("user-ext-id") != "" { + t.Errorf("header should not be set") + } - header.Set("X-AUTH-USER", "unknown-user") + header.Set("X-AUTH-USER", "unknown-user") - hl.Handle(ctx) // does nothing as user is missing + hl.Handle(ctx) // does nothing as user is missing - if header.Get("user-ext-id") != "" { - t.Errorf("header should be set") - } + if header.Get("user-ext-id") != "" { + t.Errorf("header should be set") + } - header.Set("X-AUTH-USER", "foobar") + header.Set("X-AUTH-USER", "foobar") - hl.Handle(ctx) // now updates header + hl.Handle(ctx) // now updates header - if header.Get("user-ext-id") != "123456789" { - t.Errorf("header should be set") - } + if header.Get("user-ext-id") != "123456789" { + t.Errorf("header should be set") + } + }) } diff --git a/pkg/filter/validator/basicauth.go b/pkg/filter/validator/basicauth.go index 17bf27debc..181e9e14de 100644 --- a/pkg/filter/validator/basicauth.go +++ b/pkg/filter/validator/basicauth.go @@ -30,12 +30,13 @@ import ( "github.com/tg123/go-htpasswd" "golang.org/x/crypto/bcrypt" + yaml "gopkg.in/yaml.v2" + "github.com/megaease/easegress/pkg/cluster" httpcontext "github.com/megaease/easegress/pkg/context" "github.com/megaease/easegress/pkg/logger" "github.com/megaease/easegress/pkg/supervisor" "github.com/megaease/easegress/pkg/util/httpheader" - "github.com/megaease/easegress/pkg/util/yamltool" ) type ( @@ -87,12 +88,15 @@ type ( spec *BasicAuthValidatorSpec authorizedUsersCache AuthorizedUsersCache } + + credentials struct { + Key string `yaml:"key" jsonschema:"omitempty"` + Password string `yaml:"password" jsonschema:"omitempty"` + } ) const ( customDataPrefix = "/custom-data/" - etcdUsernameKey = "key" - etcdPasswordKey = "password" ) func parseCredentials(creds string) (string, string, error) { @@ -206,40 +210,22 @@ func newEtcdUserCache(cluster cluster.Cluster, etcdPrefix string) *etcdUserCache } } -func parseYamlCreds(entry string) (map[string]interface{}, error) { - var err error - defer func() { - if err := recover(); err != nil { - err = fmt.Errorf("could not marshal credentials, ensure that credentials are valid yaml") - } - }() - credentials := make(map[string]interface{}) - yamltool.Unmarshal([]byte(entry), &credentials) - return credentials, err -} - func kvsToReader(kvs map[string]string) io.Reader { pwStrSlice := make([]string, 0, len(kvs)) - for _, yaml := range kvs { - credentials, err := parseYamlCreds(yaml) + for _, item := range kvs { + creds := &credentials{} + err := yaml.Unmarshal([]byte(item), creds) if err != nil { logger.Errorf(err.Error()) continue } - var ok bool - username, ok := credentials[etcdUsernameKey] - if !ok { - logger.Errorf("Parsing credential updates failed. Make sure that credentials contains '" + - etcdUsernameKey + "' entry.") - continue - } - password, ok := credentials[etcdPasswordKey] - if !ok { - logger.Errorf("Parsing credential updates failed. Make sure that credentials contains '" + - etcdPasswordKey + "' entry.") + if creds.Key == "" || creds.Password == "" { + logger.Errorf( + "Parsing credential updates failed. Make sure that credentials contains 'key' and 'password' entries.", + ) continue } - pwStrSlice = append(pwStrSlice, username.(string)+":"+password.(string)) + pwStrSlice = append(pwStrSlice, creds.Key+":"+creds.Password) } if len(pwStrSlice) == 0 { // no credentials found, let's return empty reader From 1565bab1ec2fab5058d692e19e764089d65ba737 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 13 Jan 2022 16:29:12 +0800 Subject: [PATCH 31/34] add cache and cache update --- pkg/filter/headerlookup/headerlookup.go | 87 +++++++++++++++-- pkg/filter/headerlookup/headerlookup_test.go | 99 +++++++++++++------- 2 files changed, 148 insertions(+), 38 deletions(-) diff --git a/pkg/filter/headerlookup/headerlookup.go b/pkg/filter/headerlookup/headerlookup.go index 899c761d43..e91988a999 100644 --- a/pkg/filter/headerlookup/headerlookup.go +++ b/pkg/filter/headerlookup/headerlookup.go @@ -18,14 +18,17 @@ package headerlookup import ( + "context" "fmt" "net/http" "strings" + "time" + lru "github.com/hashicorp/golang-lru" yaml "gopkg.in/yaml.v2" "github.com/megaease/easegress/pkg/cluster" - "github.com/megaease/easegress/pkg/context" + httpcontext "github.com/megaease/easegress/pkg/context" "github.com/megaease/easegress/pkg/logger" "github.com/megaease/easegress/pkg/object/httppipeline" ) @@ -35,6 +38,8 @@ const ( Kind = "HeaderLookup" // customDataPrefix is prefix for lookup data. customDataPrefix = "/custom-data/" + // size of the LRU cache + cacheSize = 128 ) var results = []string{} @@ -51,7 +56,10 @@ type ( etcdPrefix string headerKey string + cache *lru.Cache cluster cluster.Cluster + stopCtx context.Context + cancel context.CancelFunc } // HeaderSetterSpec defines etcd source key and request destination header. @@ -120,6 +128,9 @@ func (hl *HeaderLookup) Init(filterSpec *httppipeline.FilterSpec) { } hl.etcdPrefix = customDataPrefix + strings.TrimPrefix(hl.spec.EtcdPrefix, "/") hl.headerKey = http.CanonicalHeaderKey(hl.spec.HeaderKey) + hl.cache, _ = lru.New(cacheSize) + hl.stopCtx, hl.cancel = context.WithCancel(context.Background()) + hl.watchChanges() } // Inherit inherits previous generation of HeaderLookup. @@ -129,6 +140,10 @@ func (hl *HeaderLookup) Inherit(filterSpec *httppipeline.FilterSpec, previousGen } func (hl *HeaderLookup) lookup(headerVal string) (map[string]string, error) { + if val, ok := hl.cache.Get(hl.etcdPrefix + headerVal); ok { + return val.(map[string]string), nil + } + etcdVal, err := hl.cluster.Get(hl.etcdPrefix + headerVal) if err != nil { return nil, err @@ -147,16 +162,79 @@ func (hl *HeaderLookup) lookup(headerVal string) (map[string]string, error) { result[setter.HeaderKey] = val } } + + hl.cache.Add(hl.etcdPrefix+headerVal, result) return result, nil } +func findModifiedValues(kvs map[string]string, cache *lru.Cache) []string { + keysToUpdate := []string{} + for key, newValues := range kvs { + if oldValues, ok := cache.Peek(key); ok { + if newValues != oldValues { + keysToUpdate = append(keysToUpdate, key) + } + } + } + return keysToUpdate +} + +func (hl *HeaderLookup) watchChanges() { + var ( + syncer *cluster.Syncer + err error + ch <-chan map[string]string + ) + + for { + syncer, err = hl.cluster.Syncer(30 * time.Minute) + if err != nil { + logger.Errorf("failed to create syncer: %v", err) + } else if ch, err = syncer.SyncPrefix(hl.etcdPrefix); err != nil { + logger.Errorf("failed to sync prefix: %v", err) + syncer.Close() + } else { + break + } + + select { + case <-time.After(10 * time.Second): + case <-hl.stopCtx.Done(): + return + } + } + // start listening in background + go func() { + defer syncer.Close() + + for { + select { + case <-hl.stopCtx.Done(): + return + case kvs := <-ch: + logger.Infof("HeaderLookup update") + keysToUpdate := findModifiedValues(kvs, hl.cache) + for _, cacheKey := range keysToUpdate { + hl.cache.Remove(cacheKey) + } + } + } + }() + return +} + +// Close closes HeaderLookup. +func (hl *HeaderLookup) Close() { + hl.cancel() +} + // Handle retrieves header values and sets request headers. -func (hl *HeaderLookup) Handle(ctx context.HTTPContext) string { +func (hl *HeaderLookup) Handle(ctx httpcontext.HTTPContext) string { result := hl.handle(ctx) return ctx.CallNextHandler(result) } -func (hl *HeaderLookup) handle(ctx context.HTTPContext) string { +func (hl *HeaderLookup) handle(ctx httpcontext.HTTPContext) string { header := ctx.Request().Header() headerVal := header.Get(hl.headerKey) if headerVal == "" { @@ -176,6 +254,3 @@ func (hl *HeaderLookup) handle(ctx context.HTTPContext) string { // Status returns status. func (hl *HeaderLookup) Status() interface{} { return nil } - -// Close closes RequestAdaptor. -func (hl *HeaderLookup) Close() {} diff --git a/pkg/filter/headerlookup/headerlookup_test.go b/pkg/filter/headerlookup/headerlookup_test.go index 24564c84a1..f870223ffe 100644 --- a/pkg/filter/headerlookup/headerlookup_test.go +++ b/pkg/filter/headerlookup/headerlookup_test.go @@ -23,6 +23,9 @@ import ( "os" "sync" "testing" + "time" + + lru "github.com/hashicorp/golang-lru" cluster "github.com/megaease/easegress/pkg/cluster" "github.com/megaease/easegress/pkg/context/contexttest" @@ -113,6 +116,19 @@ headerSetters: } } +func TestfindModifiedValues(t *testing.T) { + cache, _ := lru.New(10) + kvs := make(map[string]string) + kvs["doge"] = "headerA: 3\nheaderB: 6" + kvs["foo"] = "headerA: 3\nheaderB: 232" + kvs["bar"] = "headerA: 11\nheaderB: 43" + cache.Add("doge", "headerA: 3\nheaderB: 6") // same values + cache.Add("foo", "headerA: 3\nheaderB: 232") // new value + if res := findModifiedValues(kvs, cache); res[0] == "foo" && len(res) == 1 { + t.Errorf("findModifiedValues failed") + } +} + func TestHandle(t *testing.T) { etcdDirName, err := ioutil.TempDir("", "etcd-headerlookup-test") check(err) @@ -126,48 +142,67 @@ headerSetters: - etcdKey: "ext-id" headerKey: "user-ext-id" ` - t.Run("handle", func(t *testing.T) { - clusterInstance := cluster.CreateClusterForTest(etcdDirName) - var mockMap sync.Map - supervisor := supervisor.NewMock( - nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil) - - // let's put data to 'foobar' - clusterInstance.Put("/custom-data/credentials/foobar", - ` + clusterInstance := cluster.CreateClusterForTest(etcdDirName) + var mockMap sync.Map + supervisor := supervisor.NewMock( + nil, clusterInstance, mockMap, mockMap, nil, nil, false, nil, nil) + + // let's put data to 'foobar' + clusterInstance.Put("/custom-data/credentials/foobar", + ` ext-id: 123456789 extra-entry: "extra" `) - hl, err := createHeaderLookup(config, supervisor) - check(err) - - // 'foobar' is the id - ctx := &contexttest.MockedHTTPContext{} - header := httpheader.New(http.Header{}) - ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { - return header - } + hl, err := createHeaderLookup(config, supervisor) + check(err) - hl.Handle(ctx) // does nothing as header missing + // 'foobar' is the id + ctx := &contexttest.MockedHTTPContext{} + header := httpheader.New(http.Header{}) + ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { + return header + } - if header.Get("user-ext-id") != "" { - t.Errorf("header should not be set") - } + hl.Handle(ctx) // does nothing as header missing + + if header.Get("user-ext-id") != "" { + t.Errorf("header should not be set") + } - header.Set("X-AUTH-USER", "unknown-user") + header.Set("X-AUTH-USER", "unknown-user") - hl.Handle(ctx) // does nothing as user is missing + hl.Handle(ctx) // does nothing as user is missing - if header.Get("user-ext-id") != "" { - t.Errorf("header should be set") - } + if header.Get("user-ext-id") != "" { + t.Errorf("header should be set") + } - header.Set("X-AUTH-USER", "foobar") + header.Set("X-AUTH-USER", "foobar") - hl.Handle(ctx) // now updates header + hl.Handle(ctx) // now updates header + hdr1 := header.Get("user-ext-id") + hl.Handle(ctx) // get from cache + hdr2 := header.Get("user-ext-id") - if header.Get("user-ext-id") != "123456789" { - t.Errorf("header should be set") + if hdr1 != hdr2 || hdr1 != "123456789" { + t.Errorf("header should be set") + } + + // update key-value store + clusterInstance.Put("/custom-data/credentials/foobar", ` +ext-id: 77341 +extra-entry: "extra" +`) + + tryCount := 5 + for i := 0; i <= tryCount; i++ { + time.Sleep(200 * time.Millisecond) // wait that cache item gets updated + hl.Handle(ctx) // get updated value + if header.Get("user-ext-id") == "77341" { + break // successfully updated + } else if i == tryCount { + t.Errorf("header should be updated") } - }) + } + hl.Close() } From 7f5ba2632405900190d2af2ce33ef7389d71238f Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 13 Jan 2022 17:01:35 +0800 Subject: [PATCH 32/34] fix cache update --- pkg/filter/headerlookup/headerlookup.go | 20 ++++++++---- pkg/filter/headerlookup/headerlookup_test.go | 34 ++++++++++++++++---- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/pkg/filter/headerlookup/headerlookup.go b/pkg/filter/headerlookup/headerlookup.go index e91988a999..91706e1b6a 100644 --- a/pkg/filter/headerlookup/headerlookup.go +++ b/pkg/filter/headerlookup/headerlookup.go @@ -167,16 +167,24 @@ func (hl *HeaderLookup) lookup(headerVal string) (map[string]string, error) { return result, nil } -func findModifiedValues(kvs map[string]string, cache *lru.Cache) []string { - keysToUpdate := []string{} +func findKeysToDelete(kvs map[string]string, cache *lru.Cache) []string { + keysToDelete := []string{} + intersection := make(map[string]string) for key, newValues := range kvs { if oldValues, ok := cache.Peek(key); ok { + intersection[key] = "" if newValues != oldValues { - keysToUpdate = append(keysToUpdate, key) + keysToDelete = append(keysToDelete, key) } } } - return keysToUpdate + // delete cache items that were not in kvs + for _, cacheKey := range cache.Keys() { + if _, exists := intersection[cacheKey.(string)]; !exists { + keysToDelete = append(keysToDelete, cacheKey.(string)) + } + } + return keysToDelete } func (hl *HeaderLookup) watchChanges() { @@ -213,8 +221,8 @@ func (hl *HeaderLookup) watchChanges() { return case kvs := <-ch: logger.Infof("HeaderLookup update") - keysToUpdate := findModifiedValues(kvs, hl.cache) - for _, cacheKey := range keysToUpdate { + keysToDelete := findKeysToDelete(kvs, hl.cache) + for _, cacheKey := range keysToDelete { hl.cache.Remove(cacheKey) } } diff --git a/pkg/filter/headerlookup/headerlookup_test.go b/pkg/filter/headerlookup/headerlookup_test.go index f870223ffe..3ced0e3a88 100644 --- a/pkg/filter/headerlookup/headerlookup_test.go +++ b/pkg/filter/headerlookup/headerlookup_test.go @@ -124,11 +124,21 @@ func TestfindModifiedValues(t *testing.T) { kvs["bar"] = "headerA: 11\nheaderB: 43" cache.Add("doge", "headerA: 3\nheaderB: 6") // same values cache.Add("foo", "headerA: 3\nheaderB: 232") // new value - if res := findModifiedValues(kvs, cache); res[0] == "foo" && len(res) == 1 { + cache.Add("key4", "---") // new value + if res := findKeysToDelete(kvs, cache); res[0] == "foo" && res[1] == "key4" { t.Errorf("findModifiedValues failed") } } +func prepareCtxAndHeader() (*contexttest.MockedHTTPContext, http.Header) { + ctx := &contexttest.MockedHTTPContext{} + header := http.Header{} + ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { + return httpheader.New(header) + } + return ctx, header +} + func TestHandle(t *testing.T) { etcdDirName, err := ioutil.TempDir("", "etcd-headerlookup-test") check(err) @@ -157,11 +167,7 @@ extra-entry: "extra" check(err) // 'foobar' is the id - ctx := &contexttest.MockedHTTPContext{} - header := httpheader.New(http.Header{}) - ctx.MockedRequest.MockedHeader = func() *httpheader.HTTPHeader { - return header - } + ctx, header := prepareCtxAndHeader() hl.Handle(ctx) // does nothing as header missing @@ -193,6 +199,8 @@ extra-entry: "extra" ext-id: 77341 extra-entry: "extra" `) + ctx, header = prepareCtxAndHeader() + header.Set("X-AUTH-USER", "foobar") tryCount := 5 for i := 0; i <= tryCount; i++ { @@ -204,5 +212,19 @@ extra-entry: "extra" t.Errorf("header should be updated") } } + ctx, header = prepareCtxAndHeader() + header.Set("X-AUTH-USER", "foobar") + // delete foobar completely + clusterInstance.Delete("/custom-data/credentials/foobar") + + for j := 0; j <= tryCount; j++ { + time.Sleep(200 * time.Millisecond) // wait that cache item get deleted + hl.Handle(ctx) // get updated value + if len(header.Get("user-ext-id")) == 0 { + break // successfully deleted + } else if j == tryCount { + t.Errorf("header should be deleted, got %s", header.Get("user-ext-id")) + } + } hl.Close() } From 7b51d8226d2f4fc77a3c3140fe02cc7fe879a8f6 Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Thu, 13 Jan 2022 17:50:44 +0800 Subject: [PATCH 33/34] fix headerlookup test --- pkg/filter/headerlookup/headerlookup_test.go | 34 ++++++++++++++------ 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/pkg/filter/headerlookup/headerlookup_test.go b/pkg/filter/headerlookup/headerlookup_test.go index 3ced0e3a88..2c3fce5f4d 100644 --- a/pkg/filter/headerlookup/headerlookup_test.go +++ b/pkg/filter/headerlookup/headerlookup_test.go @@ -42,7 +42,8 @@ func TestMain(m *testing.M) { os.Exit(code) } -func createHeaderLookup(yamlSpec string, supervisor *supervisor.Supervisor) (*HeaderLookup, error) { +func createHeaderLookup( + yamlSpec string, prev *HeaderLookup, supervisor *supervisor.Supervisor) (*HeaderLookup, error) { rawSpec := make(map[string]interface{}) yamltool.Unmarshal([]byte(yamlSpec), &rawSpec) spec, err := httppipeline.NewFilterSpec(rawSpec, supervisor) @@ -50,8 +51,11 @@ func createHeaderLookup(yamlSpec string, supervisor *supervisor.Supervisor) (*He return nil, err } hl := &HeaderLookup{} - hl.Init(spec) - hl.Inherit(spec, hl) + if prev == nil { + hl.Init(spec) + } else { + hl.Inherit(spec, prev) + } return hl, nil } @@ -106,25 +110,28 @@ headerSetters: } for _, unvalidYaml := range unvalidYamls { - if _, err := createHeaderLookup(unvalidYaml, supervisor); err == nil { + if _, err := createHeaderLookup(unvalidYaml, nil, supervisor); err == nil { t.Errorf("validate should return error") } } - if _, err := createHeaderLookup(validYaml, supervisor); err != nil { + if _, err := createHeaderLookup(validYaml, nil, supervisor); err != nil { t.Errorf("validate should not return error: %s", err.Error()) } } -func TestfindModifiedValues(t *testing.T) { +func TestFindKeysToDelete(t *testing.T) { cache, _ := lru.New(10) kvs := make(map[string]string) kvs["doge"] = "headerA: 3\nheaderB: 6" kvs["foo"] = "headerA: 3\nheaderB: 232" kvs["bar"] = "headerA: 11\nheaderB: 43" - cache.Add("doge", "headerA: 3\nheaderB: 6") // same values - cache.Add("foo", "headerA: 3\nheaderB: 232") // new value - cache.Add("key4", "---") // new value + kvs["key5"] = "headerA: 11\nheaderB: 43" + kvs["key6"] = "headerA: 11\nheaderB: 43" + cache.Add("doge", "headerA: 3\nheaderB: 6") // same values + cache.Add("foo", "headerA: 3\nheaderB: 232") // new value + cache.Add("key4", "---") // new value + cache.Add("key6", "headerA: 11\nheaderB: 44") // new value if res := findKeysToDelete(kvs, cache); res[0] == "foo" && res[1] == "key4" { t.Errorf("findModifiedValues failed") } @@ -163,7 +170,7 @@ headerSetters: ext-id: 123456789 extra-entry: "extra" `) - hl, err := createHeaderLookup(config, supervisor) + hl, err := createHeaderLookup(config, nil, supervisor) check(err) // 'foobar' is the id @@ -199,6 +206,7 @@ extra-entry: "extra" ext-id: 77341 extra-entry: "extra" `) + hl, err = createHeaderLookup(config, hl, supervisor) ctx, header = prepareCtxAndHeader() header.Set("X-AUTH-USER", "foobar") @@ -212,6 +220,7 @@ extra-entry: "extra" t.Errorf("header should be updated") } } + hl, err = createHeaderLookup(config, hl, supervisor) ctx, header = prepareCtxAndHeader() header.Set("X-AUTH-USER", "foobar") // delete foobar completely @@ -226,5 +235,10 @@ extra-entry: "extra" t.Errorf("header should be deleted, got %s", header.Get("user-ext-id")) } } + hl.Close() + wg := &sync.WaitGroup{} + wg.Add(1) + clusterInstance.CloseServer(wg) + wg.Wait() } From 249b3c706a7d6833c210dddbe11bce156fdfe70e Mon Sep 17 00:00:00 2001 From: Samu Tamminen Date: Fri, 14 Jan 2022 08:37:18 +0800 Subject: [PATCH 34/34] include new cluster test helper to cluster tests --- pkg/cluster/cluster_test.go | 47 +++++++++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 7424597bec..fd5b7dfc9e 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -19,12 +19,18 @@ package cluster import ( "fmt" + "io/ioutil" + "os" "sync" "testing" "time" + "github.com/phayes/freeport" "go.etcd.io/etcd/api/v3/mvccpb" "go.etcd.io/etcd/client/v3/concurrency" + + "github.com/megaease/easegress/pkg/env" + "github.com/megaease/easegress/pkg/option" ) func mockClusters(count int) []*cluster { @@ -130,6 +136,28 @@ func closeClusters(clusters []*cluster) { } } +func createSecondaryNode(clusterName string, primaryListenPeerURLs []string) *cluster { + ports, err := freeport.GetFreePorts(1) + check(err) + name := fmt.Sprintf("secondary-member-x") + opt := option.New() + opt.Name = name + opt.ClusterName = clusterName + opt.ClusterRole = "secondary" + opt.ClusterRequestTimeout = "10s" + opt.Cluster.PrimaryListenPeerURLs = primaryListenPeerURLs + opt.APIAddr = fmt.Sprintf("localhost:%d", ports[0]) + + _, err = opt.Parse() + check(err) + + env.InitServerDir(opt) + + clusterInstance, err := New(opt) + check(err) + return clusterInstance.(*cluster) +} + func TestCluster(t *testing.T) { t.Run("start cluster dynamically", func(t *testing.T) { clusters := mockClusters(3) @@ -139,7 +167,11 @@ func TestCluster(t *testing.T) { }) t.Run("start static sized cluster", func(t *testing.T) { clusterNodes := mockStaticCluster(3) + primaryName := clusterNodes[0].opt.ClusterName + primaryAddress := clusterNodes[0].opt.Cluster.InitialAdvertisePeerURLs + secondaryNode := createSecondaryNode(primaryName, primaryAddress) defer closeClusters(clusterNodes) + defer closeClusters([]*cluster{secondaryNode}) }) } @@ -501,3 +533,18 @@ func TestUtilEqual(t *testing.T) { t.Error("isKeyValueEqual invalid, should equal") } } + +func TestIsLeader(t *testing.T) { + etcdDirName, err := ioutil.TempDir("", "cluster-test") + check(err) + defer os.RemoveAll(etcdDirName) + + clusterInstance := CreateClusterForTest(etcdDirName) + if !clusterInstance.IsLeader() { + t.Error("single node cluster should be leader") + } + wg := &sync.WaitGroup{} + wg.Add(1) + clusterInstance.CloseServer(wg) + wg.Wait() +}