simplify configuration by fixing key and password entries; allow skip…

…ping unvalid etcd entries

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

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
 

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,38 +296,42 @@ 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)
  }
  }
  return nil
 }
 
 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
  }

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"
 `)