From 8409d7b83e6d499469b9ff50f8b3ce9b3dfa53aa Mon Sep 17 00:00:00 2001 From: akshya96 Date: Thu, 15 Sep 2022 13:37:25 -0700 Subject: [PATCH 01/24] config file changes --- command/server/config.go | 5 ++ go.mod | 1 + go.sum | 2 + internalshared/configutil/config.go | 24 ++++++ internalshared/configutil/merge.go | 7 ++ internalshared/configutil/userlockout.go | 101 +++++++++++++++++++++++ 6 files changed, 140 insertions(+) create mode 100644 internalshared/configutil/userlockout.go diff --git a/command/server/config.go b/command/server/config.go index 5d985db69d08..f30cc073866f 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -20,6 +20,7 @@ import ( "github.com/hashicorp/vault/helper/osutil" "github.com/hashicorp/vault/internalshared/configutil" "github.com/hashicorp/vault/sdk/helper/consts" + "github.com/ryboe/q" ) const ( @@ -505,6 +506,10 @@ func LoadConfigFile(path string) (*Config, error) { } } } + q.Q("Reading from config file") + q.Q(conf.Listeners) + q.Q("user lockout configs") + q.Q(conf.UserLockoutConfigs) return conf, nil } diff --git a/go.mod b/go.mod index 56d2bfe5981a..a7dc43754514 100644 --- a/go.mod +++ b/go.mod @@ -381,6 +381,7 @@ require ( github.com/prometheus/procfs v0.6.0 // indirect github.com/renier/xmlrpc v0.0.0-20170708154548-ce4a1a486c03 // indirect github.com/rogpeppe/go-internal v1.8.1 // indirect + github.com/ryboe/q v1.0.17 // indirect github.com/sirupsen/logrus v1.8.1 // indirect github.com/snowflakedb/gosnowflake v1.6.3 // indirect github.com/softlayer/softlayer-go v0.0.0-20180806151055-260589d94c7d // indirect diff --git a/go.sum b/go.sum index 8469a94fb43a..865ab431a02c 100644 --- a/go.sum +++ b/go.sum @@ -1660,6 +1660,8 @@ github.com/ryanuber/columnize v2.1.0+incompatible h1:j1Wcmh8OrK4Q7GXY+V7SVSY8nUW github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIHxIXzX/Yc= +github.com/ryboe/q v1.0.17 h1:Ap34VxlzBbjFHdApe1RzvBwrYmoLa4hC5J7P643ENtU= +github.com/ryboe/q v1.0.17/go.mod h1:7wNegax8bjSyGxm9Pnsy6i8z+Uy9X8hkm7pAId9PDdg= github.com/safchain/ethtool v0.0.0-20190326074333-42ed695e3de8/go.mod h1:Z0q5wiBQGYcxhMZ6gUqHn6pYNLypFAvaL3UvgZLR0U4= github.com/samuel/go-zookeeper v0.0.0-20190923202752-2cc03de413da h1:p3Vo3i64TCLY7gIfzeQaUJ+kppEO5WQG3cL8iE8tGHU= github.com/samuel/go-zookeeper v0.0.0-20190923202752-2cc03de413da/go.mod h1:gi+0XIa01GRL2eRQVjQkKGqKF3SF9vZR/HnPullcV2E= diff --git a/internalshared/configutil/config.go b/internalshared/configutil/config.go index 5c12e03b949f..f94eeaf438a8 100644 --- a/internalshared/configutil/config.go +++ b/internalshared/configutil/config.go @@ -20,6 +20,7 @@ type SharedConfig struct { EntSharedConfig Listeners []*Listener `hcl:"-"` + UserLockoutConfigs []*UserLockoutConfig `hcl:"-"` Seals []*KMS `hcl:"-"` Entropy *Entropy `hcl:"-"` @@ -134,6 +135,13 @@ func ParseConfig(d string) (*SharedConfig, error) { } } + if o := list.Filter("user_lockout"); len(o.Items) > 0 { + result.found("user_lockout", "UserLockout") + if err := ParseUserLockouts(&result, o); err != nil { + return nil, fmt.Errorf("error parsing 'user_lockout': %w", err) + } + } + if o := list.Filter("telemetry"); len(o.Items) > 0 { result.found("telemetry", "Telemetry") if err := parseTelemetry(&result, o); err != nil { @@ -194,6 +202,22 @@ func (c *SharedConfig) Sanitized() map[string]interface{} { result["listeners"] = sanitizedListeners } + // Sanitize user lockout stanza + if len(c.UserLockoutConfigs) != 0 { + var sanitizedUserLockouts []interface{} + for _, userlockout := range c.UserLockoutConfigs { + cleanUserLockout := map[string]interface{}{ + "type": userlockout.Type, + "lockout_threshold": userlockout.LockoutThreshold, + "lockout_duration": userlockout.LockoutDuration, + "lockout_counter_reset": userlockout.LockoutCounterReset, + "disable_lockout": userlockout.DisableLockout, + } + sanitizedUserLockouts = append(sanitizedUserLockouts, cleanUserLockout) + } + result["user_lockout_configs"] = sanitizedUserLockouts + } + // Sanitize seals stanza if len(c.Seals) != 0 { var sanitizedSeals []interface{} diff --git a/internalshared/configutil/merge.go b/internalshared/configutil/merge.go index 8ae99ca4879d..17e20d26e70b 100644 --- a/internalshared/configutil/merge.go +++ b/internalshared/configutil/merge.go @@ -14,6 +14,13 @@ func (c *SharedConfig) Merge(c2 *SharedConfig) *SharedConfig { result.Listeners = append(result.Listeners, l) } + for _, userlockout := range c.UserLockoutConfigs { + result.UserLockoutConfigs = append(result.UserLockoutConfigs, userlockout) + } + for _, userlockout := range c2.UserLockoutConfigs { + result.UserLockoutConfigs = append(result.UserLockoutConfigs, userlockout) + } + result.HCPLinkConf = c.HCPLinkConf if c2.HCPLinkConf != nil { result.HCPLinkConf = c2.HCPLinkConf diff --git a/internalshared/configutil/userlockout.go b/internalshared/configutil/userlockout.go new file mode 100644 index 000000000000..4f6bac581060 --- /dev/null +++ b/internalshared/configutil/userlockout.go @@ -0,0 +1,101 @@ +package configutil + +import ( + "errors" + "fmt" + "strings" + "time" + + "github.com/hashicorp/go-multierror" + "github.com/hashicorp/go-secure-stdlib/parseutil" + "github.com/hashicorp/hcl" + "github.com/hashicorp/hcl/hcl/ast" +) + +type UserLockoutConfig struct{ + Type string + LockoutThreshold int64 `hcl:"-"` + LockoutThresholdRaw interface{} `hcl:"lockout_threshold"` + LockoutDuration time.Duration `hcl:"-"` + LockoutDurationRaw interface{} `hcl:"lockout_duration"` + LockoutCounterReset time.Duration `hcl:"-"` + LockoutCounterResetRaw interface{} `hcl:"lockout_counter_reset"` + DisableLockout bool `hcl:"-"` + DisableLockoutRaw interface{} `hcl:"disable_lockout"` +} + +func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { + var err error + result.UserLockoutConfigs = make([]*UserLockoutConfig, 0, len(list.Items)) + for i, item := range list.Items { + var userLockoutConfig UserLockoutConfig + if err := hcl.DecodeObject(&userLockoutConfig, item.Val); err != nil { + return multierror.Prefix(err, fmt.Sprintf("userLockouts.%d:", i)) + } + + // Base values + { + switch { + case userLockoutConfig.Type != "": + case len(item.Keys) == 1: + userLockoutConfig.Type = strings.ToLower(item.Keys[0].Token.Value().(string)) + default: + return multierror.Prefix(errors.New("auth type for user lockout must be specified, if it applies to all auth methods specify \"all\" "), fmt.Sprintf("user_lockouts.%d:", i)) + } + + userLockoutConfig.Type = strings.ToLower(userLockoutConfig.Type) + switch userLockoutConfig.Type { + case "all", "ldap", "approle", "userpass": + result.found(userLockoutConfig.Type, userLockoutConfig.Type) + default: + return multierror.Prefix(fmt.Errorf("unsupported auth type %q", userLockoutConfig.Type), fmt.Sprintf("user_lockouts.%d:", i)) + } + } + + // Lockout Parameters + { + if userLockoutConfig.LockoutThresholdRaw != nil { + if userLockoutConfig.LockoutThreshold, err = parseutil.ParseInt(userLockoutConfig.LockoutThresholdRaw); err != nil { + return multierror.Prefix(fmt.Errorf("error parsing lockout_threshold: %w", err), fmt.Sprintf("user_lockouts.%d", i)) + } + + userLockoutConfig.LockoutThresholdRaw = nil + } + + if userLockoutConfig.LockoutDurationRaw != nil { + if userLockoutConfig.LockoutDuration, err = parseutil.ParseDurationSecond(userLockoutConfig.LockoutDurationRaw); err != nil { + return multierror.Prefix(fmt.Errorf("error parsing lockout_duration: %w", err), fmt.Sprintf("user_lockouts.%d", i)) + } + if userLockoutConfig.LockoutDuration < 0 { + return multierror.Prefix(errors.New("lockout_duration cannot be negative"), fmt.Sprintf("user_lockouts.%d", i)) + } + + userLockoutConfig.LockoutDurationRaw = nil + } + + if userLockoutConfig.LockoutCounterResetRaw != nil { + if userLockoutConfig.LockoutCounterReset, err = parseutil.ParseDurationSecond(userLockoutConfig.LockoutCounterResetRaw); err != nil { + return multierror.Prefix(fmt.Errorf("error parsing lockout_counter_reset: %w", err), fmt.Sprintf("user_lockouts.%d", i)) + } + if userLockoutConfig.LockoutCounterReset < 0 { + return multierror.Prefix(errors.New("lockout_counter_reset cannot be negative"), fmt.Sprintf("user_lockouts.%d", i)) + } + + userLockoutConfig.LockoutCounterResetRaw = nil + } + + if userLockoutConfig.DisableLockoutRaw != nil { + if userLockoutConfig.DisableLockout, err = parseutil.ParseBool(userLockoutConfig.DisableLockoutRaw); err != nil { + return multierror.Prefix(fmt.Errorf("invalid value for disable_lockout: %w", err), fmt.Sprintf("user_lockouts.%d", i)) + } + + userLockoutConfig.DisableLockoutRaw = nil + } + + } + + result.UserLockoutConfigs = append(result.UserLockoutConfigs, &userLockoutConfig) + } + + return nil +} From f4503aebe3d5374133b0d571c6d73662835b5cb2 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Wed, 21 Sep 2022 16:47:37 -0700 Subject: [PATCH 02/24] lockout config changes --- api/sys_mounts.go | 67 ++++--- command/auth_tune.go | 54 ++++++ command/server/config.go | 8 + internalshared/configutil/config.go | 10 +- internalshared/configutil/userlockout.go | 109 +++++++++-- sdk/helper/consts/consts.go | 2 + vault/logical_system.go | 225 ++++++++++++++++++++++- vault/logical_system_paths.go | 4 + vault/mount.go | 39 +++- vault/request_handling.go | 25 +++ 10 files changed, 490 insertions(+), 53 deletions(-) diff --git a/api/sys_mounts.go b/api/sys_mounts.go index 522f87e6eae5..fa414162f3bb 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -8,6 +8,7 @@ import ( "time" "github.com/mitchellh/mapstructure" + "github.com/ryboe/q" ) func (c *Sys) ListMounts() (map[string]*MountOutput, error) { @@ -199,6 +200,9 @@ func (c *Sys) TuneMountWithContext(ctx context.Context, path string, config Moun return err } + + q.Q("request") + q.Q(r) resp, err := c.c.rawRequestWithContext(ctx, r) if err == nil { defer resp.Body.Close() @@ -255,18 +259,19 @@ type MountInput struct { } type MountConfigInput struct { - Options map[string]string `json:"options" mapstructure:"options"` - DefaultLeaseTTL string `json:"default_lease_ttl" mapstructure:"default_lease_ttl"` - Description *string `json:"description,omitempty" mapstructure:"description"` - MaxLeaseTTL string `json:"max_lease_ttl" mapstructure:"max_lease_ttl"` - ForceNoCache bool `json:"force_no_cache" mapstructure:"force_no_cache"` - AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" mapstructure:"audit_non_hmac_request_keys"` - AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" mapstructure:"audit_non_hmac_response_keys"` - ListingVisibility string `json:"listing_visibility,omitempty" mapstructure:"listing_visibility"` - PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" mapstructure:"passthrough_request_headers"` - AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` - TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` - AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` + Options map[string]string `json:"options" mapstructure:"options"` + DefaultLeaseTTL string `json:"default_lease_ttl" mapstructure:"default_lease_ttl"` + Description *string `json:"description,omitempty" mapstructure:"description"` + MaxLeaseTTL string `json:"max_lease_ttl" mapstructure:"max_lease_ttl"` + ForceNoCache bool `json:"force_no_cache" mapstructure:"force_no_cache"` + AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" mapstructure:"audit_non_hmac_request_keys"` + AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" mapstructure:"audit_non_hmac_response_keys"` + ListingVisibility string `json:"listing_visibility,omitempty" mapstructure:"listing_visibility"` + PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" mapstructure:"passthrough_request_headers"` + AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` + TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` + AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` + UserLockoutConfig UserLockoutInputConfig `json:"user_lockout_config"` // Deprecated: This field will always be blank for newer server responses. PluginName string `json:"plugin_name,omitempty" mapstructure:"plugin_name"` @@ -290,21 +295,37 @@ type MountOutput struct { } type MountConfigOutput struct { - DefaultLeaseTTL int `json:"default_lease_ttl" mapstructure:"default_lease_ttl"` - MaxLeaseTTL int `json:"max_lease_ttl" mapstructure:"max_lease_ttl"` - ForceNoCache bool `json:"force_no_cache" mapstructure:"force_no_cache"` - AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" mapstructure:"audit_non_hmac_request_keys"` - AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" mapstructure:"audit_non_hmac_response_keys"` - ListingVisibility string `json:"listing_visibility,omitempty" mapstructure:"listing_visibility"` - PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" mapstructure:"passthrough_request_headers"` - AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` - TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` - AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` - + DefaultLeaseTTL int `json:"default_lease_ttl" mapstructure:"default_lease_ttl"` + MaxLeaseTTL int `json:"max_lease_ttl" mapstructure:"max_lease_ttl"` + ForceNoCache bool `json:"force_no_cache" mapstructure:"force_no_cache"` + AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" mapstructure:"audit_non_hmac_request_keys"` + AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" mapstructure:"audit_non_hmac_response_keys"` + ListingVisibility string `json:"listing_visibility,omitempty" mapstructure:"listing_visibility"` + PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" mapstructure:"passthrough_request_headers"` + AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` + TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` + AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` + UserLockoutConfig UserLockoutOutputConfig `json:"user_lockout_config"` // Deprecated: This field will always be blank for newer server responses. PluginName string `json:"plugin_name,omitempty" mapstructure:"plugin_name"` } +type UserLockoutInputConfig struct { + LockoutThreshold string `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` // Override for global default + LockoutDuration string `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` // Override for global default + LockoutCounterResetDuration string `json:"lockout_counter_reset_duration,omitempty" structs:"lockout_counter_reset_duration" mapstructure:"lockout_counter_reset_duration"` // Override for global default + DisableLockout bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` // Override for global default + +} + +type UserLockoutOutputConfig struct { + LockoutThreshold int `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` // Override for global default + LockoutDuration int `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` // Override for global default + LockoutCounterReset int `json:"lockout_counter_reset,omitempty" structs:"lockout_counter_reset" mapstructure:"lockout_counter_reset"` // Override for global default + DisableLockout bool `json:"disable_lockout,omitempty" structs:"disable_lockout" mapstructure:"disable_lockout"` // Override for global default + +} + type MountMigrationOutput struct { MigrationID string `mapstructure:"migration_id"` } diff --git a/command/auth_tune.go b/command/auth_tune.go index 9c3a963efc37..c61af282f4f0 100644 --- a/command/auth_tune.go +++ b/command/auth_tune.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/vault/api" "github.com/mitchellh/cli" "github.com/posener/complete" + "github.com/ryboe/q" ) var ( @@ -31,6 +32,10 @@ type AuthTuneCommand struct { flagOptions map[string]string flagTokenType string flagVersion int + flagUserLockoutThreshold int + flagUserLockoutDuration time.Duration + flagUserLockoutCounterResetDuration time.Duration + flagUserLockoutDisable bool } func (c *AuthTuneCommand) Synopsis() string { @@ -144,6 +149,40 @@ func (c *AuthTuneCommand) Flags() *FlagSets { Usage: "Select the version of the auth method to run. Not supported by all auth methods.", }) + f.IntVar(&IntVar{ + Name: "user-lockout-threshold", + Target: &c.flagUserLockoutThreshold, + Usage: "The threshold for user lockout for this auth method. If unspecified, this " + + "defaults to the Vault server's globally configured user lockout threshold, " + + "or a previously configured value for the auth method.", + }) + + f.DurationVar(&DurationVar{ + Name: "user-lockout-duration", + Target: &c.flagUserLockoutDuration, + Completion: complete.PredictAnything, + Usage: "The user lockout duration for this auth method. If unspecified, this " + + "defaults to the Vault server's globally configured user lockout duration, " + + "or a previously configured value for the auth method.", + }) + + f.DurationVar(&DurationVar{ + Name: "user-lockout-counter-reset-duration", + Target: &c.flagUserLockoutCounterResetDuration, + Completion: complete.PredictAnything, + Usage: "The user lockout counter reset duration for this auth method. If unspecified, this " + + "defaults to the Vault server's globally configured user lockout counter reset duration, " + + "or a previously configured value for the auth method.", + }) + + f.BoolVar(&BoolVar{ + Name: "user-lockout-disable", + Target: &c.flagUserLockoutDisable, + Usage: "Disable user lockout for this auth method. If unspecified, this " + + "defaults to the Vault server's globally configured user lockout disable, " + + "or a previously configured value for the auth method.", + }) + return set } @@ -221,8 +260,23 @@ func (c *AuthTuneCommand) Run(args []string) int { if fl.Name == flagNameTokenType { mountConfigInput.TokenType = c.flagTokenType } + if fl.Name == "user-lockout-threshold" { + if c.flagUserLockoutThreshold > 0 { + mountConfigInput.UserLockoutConfig.LockoutThreshold = strconv.Itoa(c.flagUserLockoutThreshold) + } + } + if fl.Name == "user-lockout-duration" { + mountConfigInput.UserLockoutConfig.LockoutDuration = ttlToAPI(c.flagUserLockoutDuration) + } + if fl.Name == "user-lockout-counter-reset" { + mountConfigInput.UserLockoutConfig.LockoutCounterResetDuration = ttlToAPI(c.flagUserLockoutCounterResetDuration) + } + if fl.Name == "user-lockout-disable" { + mountConfigInput.UserLockoutConfig.DisableLockout = c.flagUserLockoutDisable + } }) + q.Q(mountConfigInput) // Append /auth (since that's where auths live) and a trailing slash to // indicate it's a path in output mountPath := ensureTrailingSlash(sanitizePath(args[0])) diff --git a/command/server/config.go b/command/server/config.go index f30cc073866f..115398d2ed41 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -510,6 +510,14 @@ func LoadConfigFile(path string) (*Config, error) { q.Q(conf.Listeners) q.Q("user lockout configs") q.Q(conf.UserLockoutConfigs) + + //update the mount entries with the configuration + + // get the mount entry + + + + return conf, nil } diff --git a/internalshared/configutil/config.go b/internalshared/configutil/config.go index f94eeaf438a8..0877338dc6f4 100644 --- a/internalshared/configutil/config.go +++ b/internalshared/configutil/config.go @@ -19,7 +19,7 @@ type SharedConfig struct { EntSharedConfig - Listeners []*Listener `hcl:"-"` + Listeners []*Listener `hcl:"-"` UserLockoutConfigs []*UserLockoutConfig `hcl:"-"` Seals []*KMS `hcl:"-"` @@ -207,11 +207,11 @@ func (c *SharedConfig) Sanitized() map[string]interface{} { var sanitizedUserLockouts []interface{} for _, userlockout := range c.UserLockoutConfigs { cleanUserLockout := map[string]interface{}{ - "type": userlockout.Type, - "lockout_threshold": userlockout.LockoutThreshold, - "lockout_duration": userlockout.LockoutDuration, + "type": userlockout.Type, + "lockout_threshold": userlockout.LockoutThreshold, + "lockout_duration": userlockout.LockoutDuration, "lockout_counter_reset": userlockout.LockoutCounterReset, - "disable_lockout": userlockout.DisableLockout, + "disable_lockout": userlockout.DisableLockout, } sanitizedUserLockouts = append(sanitizedUserLockouts, cleanUserLockout) } diff --git a/internalshared/configutil/userlockout.go b/internalshared/configutil/userlockout.go index 4f6bac581060..1f4a96672fd8 100644 --- a/internalshared/configutil/userlockout.go +++ b/internalshared/configutil/userlockout.go @@ -12,27 +12,35 @@ import ( "github.com/hashicorp/hcl/hcl/ast" ) -type UserLockoutConfig struct{ - Type string - LockoutThreshold int64 `hcl:"-"` - LockoutThresholdRaw interface{} `hcl:"lockout_threshold"` - LockoutDuration time.Duration `hcl:"-"` - LockoutDurationRaw interface{} `hcl:"lockout_duration"` - LockoutCounterReset time.Duration `hcl:"-"` - LockoutCounterResetRaw interface{} `hcl:"lockout_counter_reset"` - DisableLockout bool `hcl:"-"` - DisableLockoutRaw interface{} `hcl:"disable_lockout"` +const ( + UserLockoutThresholdDefault = 5 + UserLockoutDurationDefault = 15 * time.Minute + UserLockoutCounterResetDefault = 15 * time.Minute + DisableUserLockoutDefault = false +) + +type UserLockoutConfig struct { + Type string + LockoutThreshold int64 `hcl:"-"` + LockoutThresholdRaw interface{} `hcl:"lockout_threshold"` + LockoutDuration time.Duration `hcl:"-"` + LockoutDurationRaw interface{} `hcl:"lockout_duration"` + LockoutCounterReset time.Duration `hcl:"-"` + LockoutCounterResetRaw interface{} `hcl:"lockout_counter_reset"` + DisableLockout bool `hcl:"-"` + DisableLockoutRaw interface{} `hcl:"disable_lockout"` } func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { var err error result.UserLockoutConfigs = make([]*UserLockoutConfig, 0, len(list.Items)) + userLockoutConfigsMap := make(map[string]*UserLockoutConfig) for i, item := range list.Items { var userLockoutConfig UserLockoutConfig if err := hcl.DecodeObject(&userLockoutConfig, item.Val); err != nil { return multierror.Prefix(err, fmt.Sprintf("userLockouts.%d:", i)) } - + // Base values { switch { @@ -58,8 +66,6 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { if userLockoutConfig.LockoutThreshold, err = parseutil.ParseInt(userLockoutConfig.LockoutThresholdRaw); err != nil { return multierror.Prefix(fmt.Errorf("error parsing lockout_threshold: %w", err), fmt.Sprintf("user_lockouts.%d", i)) } - - userLockoutConfig.LockoutThresholdRaw = nil } if userLockoutConfig.LockoutDurationRaw != nil { @@ -70,7 +76,6 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { return multierror.Prefix(errors.New("lockout_duration cannot be negative"), fmt.Sprintf("user_lockouts.%d", i)) } - userLockoutConfig.LockoutDurationRaw = nil } if userLockoutConfig.LockoutCounterResetRaw != nil { @@ -81,21 +86,85 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { return multierror.Prefix(errors.New("lockout_counter_reset cannot be negative"), fmt.Sprintf("user_lockouts.%d", i)) } - userLockoutConfig.LockoutCounterResetRaw = nil } - if userLockoutConfig.DisableLockoutRaw != nil { if userLockoutConfig.DisableLockout, err = parseutil.ParseBool(userLockoutConfig.DisableLockoutRaw); err != nil { return multierror.Prefix(fmt.Errorf("invalid value for disable_lockout: %w", err), fmt.Sprintf("user_lockouts.%d", i)) } - - userLockoutConfig.DisableLockoutRaw = nil } + } + userLockoutConfigsMap[userLockoutConfig.Type] = &userLockoutConfig + } + // userLockoutConfigsMap = SetDefaultUserLockoutValuesInMap(userLockoutConfigsMap) + userLockoutConfigsMap = SetMissingUserLockoutValuesInMap(userLockoutConfigsMap) + for _, userLockoutValues := range userLockoutConfigsMap { + result.UserLockoutConfigs = append(result.UserLockoutConfigs, userLockoutValues) + } + return nil +} +// setDefaultUserLockoutValuesInMap sets user lockout default values for key "all" for user lockout fields thats are not configured +func setDefaultUserLockoutValuesInMap(userLockoutConfigsMap map[string]*UserLockoutConfig) map[string]*UserLockoutConfig { + if userLockoutAll, ok := userLockoutConfigsMap["all"]; !ok { + var tmpUserLockoutConfig UserLockoutConfig + tmpUserLockoutConfig.LockoutThreshold = UserLockoutThresholdDefault + tmpUserLockoutConfig.LockoutDuration = UserLockoutDurationDefault + tmpUserLockoutConfig.LockoutCounterReset = UserLockoutCounterResetDefault + tmpUserLockoutConfig.DisableLockout = DisableUserLockoutDefault + userLockoutConfigsMap["all"] = &tmpUserLockoutConfig + + } else { + if userLockoutAll.LockoutThresholdRaw == nil { + userLockoutAll.LockoutThreshold = UserLockoutThresholdDefault + } + if userLockoutAll.LockoutDurationRaw == nil { + userLockoutAll.LockoutDuration = UserLockoutDurationDefault + } + if userLockoutAll.LockoutCounterResetRaw == nil { + userLockoutAll.LockoutCounterReset = UserLockoutCounterResetDefault + } + if userLockoutAll.DisableLockoutRaw == nil { + userLockoutAll.DisableLockout = DisableUserLockoutDefault + } + userLockoutConfigsMap[userLockoutAll.Type] = userLockoutAll + } + return userLockoutConfigsMap +} + +// setDefaultUserLockoutValuesInMap sets missing user lockout fields for other auth types with default values (from key "all") +func SetMissingUserLockoutValuesInMap(userLockoutConfigsMap map[string]*UserLockoutConfig) map[string]*UserLockoutConfig { + userLockoutConfigsMap = setDefaultUserLockoutValuesInMap(userLockoutConfigsMap) + for _, userLockoutAuth := range userLockoutConfigsMap { + // set missing values + if userLockoutAuth.LockoutThresholdRaw == nil { + userLockoutAuth.LockoutThreshold = userLockoutConfigsMap["all"].LockoutThreshold + } + if userLockoutAuth.LockoutDurationRaw == nil { + userLockoutAuth.LockoutDuration = userLockoutConfigsMap["all"].LockoutDuration + } + if userLockoutAuth.LockoutCounterResetRaw == nil { + userLockoutAuth.LockoutCounterReset = userLockoutConfigsMap["all"].LockoutCounterReset + } + if userLockoutAuth.DisableLockoutRaw == nil { + userLockoutAuth.DisableLockout = userLockoutConfigsMap["all"].DisableLockout } - result.UserLockoutConfigs = append(result.UserLockoutConfigs, &userLockoutConfig) + // set nil values to Raw fields + if userLockoutAuth.LockoutThresholdRaw != nil { + userLockoutAuth.LockoutThresholdRaw = nil + } + if userLockoutAuth.LockoutDurationRaw != nil { + userLockoutAuth.LockoutDurationRaw = nil + } + if userLockoutAuth.LockoutCounterResetRaw != nil { + userLockoutAuth.LockoutCounterResetRaw = nil + } + if userLockoutAuth.DisableLockoutRaw != nil { + userLockoutAuth.DisableLockoutRaw = nil + } + + userLockoutConfigsMap[userLockoutAuth.Type] = userLockoutAuth } - return nil + return userLockoutConfigsMap } diff --git a/sdk/helper/consts/consts.go b/sdk/helper/consts/consts.go index c431e2e59419..a4b7c5040422 100644 --- a/sdk/helper/consts/consts.go +++ b/sdk/helper/consts/consts.go @@ -34,4 +34,6 @@ const ( ReplicationResolverALPN = "replication_resolver_v1" VaultEnableFilePermissionsCheckEnv = "VAULT_ENABLE_FILE_PERMISSIONS_CHECK" + + VaultDisableUserLockout = "VAULT_DISABLE_USER_LOCKOUT" ) diff --git a/vault/logical_system.go b/vault/logical_system.go index 4373083b47cb..a7bbd17b1263 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -30,6 +30,7 @@ import ( "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/go-secure-stdlib/strutil" semver "github.com/hashicorp/go-version" + "github.com/hashicorp/vault/command/server" "github.com/hashicorp/vault/helper/hostutil" "github.com/hashicorp/vault/helper/identity" "github.com/hashicorp/vault/helper/metricsutil" @@ -44,6 +45,7 @@ import ( "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/version" "github.com/mitchellh/mapstructure" + "github.com/ryboe/q" ) const ( @@ -1517,10 +1519,14 @@ func (b *SystemBackend) handleTuneReadCommon(ctx context.Context, path string) ( resp := &logical.Response{ Data: map[string]interface{}{ - "description": mountEntry.Description, - "default_lease_ttl": int(sysView.DefaultLeaseTTL().Seconds()), - "max_lease_ttl": int(sysView.MaxLeaseTTL().Seconds()), - "force_no_cache": mountEntry.Config.ForceNoCache, + "description": mountEntry.Description, + "default_lease_ttl": int(sysView.DefaultLeaseTTL().Seconds()), + "max_lease_ttl": int(sysView.MaxLeaseTTL().Seconds()), + "force_no_cache": mountEntry.Config.ForceNoCache, + "user_lockout_counter_reset_duration": int64(mountEntry.Config.UserLockoutConfig.LockoutCounterReset.Seconds()), + "user_lockout_threshold": mountEntry.Config.UserLockoutConfig.LockoutThreshold, + "user_lockout_duration": int64(mountEntry.Config.UserLockoutConfig.LockoutDuration.Seconds()), + "user_lockout_disable": mountEntry.Config.UserLockoutConfig.DisableLockout, }, } @@ -1576,6 +1582,7 @@ func (b *SystemBackend) handleAuthTuneWrite(ctx context.Context, req *logical.Re // handleMountTuneWrite is used to set config settings on a backend func (b *SystemBackend) handleMountTuneWrite(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { + q.Q(data) path := data.Get("path").(string) if path == "" { return logical.ErrorResponse("missing path"), nil @@ -1672,6 +1679,151 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, } } + // user-lockout config + { + var apiuserLockoutConfig APIUserLockoutConfig + + userLockoutConfigMap := data.Get("user_lockout_config").(map[string]interface{}) + var err error + // Augmenting userLockoutConfigMap for some config options to treat them as comma separated entries + err = expandStringValsWithCommas(userLockoutConfigMap) + if err != nil { + return logical.ErrorResponse( + "unable to parse given auth config information"), + logical.ErrInvalidRequest + } + if userLockoutConfigMap != nil && len(userLockoutConfigMap) != 0 { + err := mapstructure.Decode(userLockoutConfigMap, &apiuserLockoutConfig) + if err != nil { + return logical.ErrorResponse( + "unable to convert given mount config information"), + logical.ErrInvalidRequest + } + } + + if _, ok := userLockoutConfigMap["lockout_threshold"]; ok { + var newUserLockoutThreshold int64 + userLockoutThreshold, err := parseutil.ParseInt(apiuserLockoutConfig.LockoutThreshold) + if err != nil { + return nil, fmt.Errorf("unable to parse user lockout threshold: %w", err) + } + oldUserLockoutThreshold := mountEntry.Config.UserLockoutConfig.LockoutThreshold + + switch { + case userLockoutThreshold < 0: + newUserLockoutThreshold = oldUserLockoutThreshold + default: + newUserLockoutThreshold = userLockoutThreshold + } + mountEntry.Config.UserLockoutConfig.LockoutThreshold = newUserLockoutThreshold + // Update the mount table + switch { + case strings.HasPrefix(path, "auth/"): + err = b.Core.persistAuth(ctx, b.Core.auth, &mountEntry.Local) + default: + err = b.Core.persistMounts(ctx, b.Core.mounts, &mountEntry.Local) + } + if err != nil { + mountEntry.Config.UserLockoutConfig.LockoutThreshold = oldUserLockoutThreshold + return handleError(err) + } + if b.Core.logger.IsInfo() { + b.Core.logger.Info("mount tuning of user-lockout-threshold successful", "path", path, "user-lockout-threshold", userLockoutThreshold) + } + + } + + { + var newUserLockoutDuration time.Duration + oldUserLockoutDuration := mountEntry.Config.UserLockoutConfig.LockoutDuration + switch apiuserLockoutConfig.LockoutDuration { + + case "": + newUserLockoutDuration = oldUserLockoutDuration + case "system": + newUserLockoutDuration = time.Duration(0) + default: + tmpUserLockoutDuration, err := parseutil.ParseDurationSecond(apiuserLockoutConfig.LockoutDuration) + if err != nil { + return handleError(err) + } + newUserLockoutDuration = tmpUserLockoutDuration + + } + mountEntry.Config.UserLockoutConfig.LockoutDuration = newUserLockoutDuration + // Update the mount table + + switch { + case strings.HasPrefix(path, "auth/"): + err = b.Core.persistAuth(ctx, b.Core.auth, &mountEntry.Local) + default: + err = b.Core.persistMounts(ctx, b.Core.mounts, &mountEntry.Local) + } + if err != nil { + mountEntry.Config.UserLockoutConfig.LockoutDuration = oldUserLockoutDuration + return handleError(err) + } + if b.Core.logger.IsInfo() { + b.Core.logger.Info("mount tuning of user-lockout-duration successful", "path", path, "user-lockout-duration", apiuserLockoutConfig.LockoutDuration) + } + + } + { + var newUserLockoutCounterReset time.Duration + oldUserLockoutCounterReset := mountEntry.Config.UserLockoutConfig.LockoutCounterReset + switch apiuserLockoutConfig.LockoutCounterResetDuration { + case "": + newUserLockoutCounterReset = oldUserLockoutCounterReset + case "system": + newUserLockoutCounterReset = time.Duration(0) + default: + tmpUserLockoutCounterReset, err := parseutil.ParseDurationSecond(apiuserLockoutConfig.LockoutCounterResetDuration) + if err != nil { + return handleError(err) + } + newUserLockoutCounterReset = tmpUserLockoutCounterReset + } + + mountEntry.Config.UserLockoutConfig.LockoutCounterReset = newUserLockoutCounterReset + // Update the mount table + switch { + case strings.HasPrefix(path, "auth/"): + err = b.Core.persistAuth(ctx, b.Core.auth, &mountEntry.Local) + default: + err = b.Core.persistMounts(ctx, b.Core.mounts, &mountEntry.Local) + } + if err != nil { + mountEntry.Config.UserLockoutConfig.LockoutCounterReset = oldUserLockoutCounterReset + return handleError(err) + } + if b.Core.logger.IsInfo() { + b.Core.logger.Info("mount tuning of user-lockout-counter-reset successful", "path", path, "user-lockout-counter-reset", apiuserLockoutConfig.LockoutCounterResetDuration) + } + } + { + if rawVal, ok := userLockoutConfigMap["lockout_disable"]; ok { + userLockoutDisable := rawVal.(bool) + oldUserLockoutDisable := mountEntry.Config.UserLockoutConfig.DisableLockout + mountEntry.Config.UserLockoutConfig.DisableLockout = userLockoutDisable + var err error + switch { + case strings.HasPrefix(path, "auth/"): + err = b.Core.persistAuth(ctx, b.Core.auth, &mountEntry.Local) + default: + err = b.Core.persistMounts(ctx, b.Core.mounts, &mountEntry.Local) + } + if err != nil { + mountEntry.Config.UserLockoutConfig.DisableLockout = oldUserLockoutDisable + return handleError(err) + } + if b.Core.logger.IsInfo() { + b.Core.logger.Info("mount tuning of user-lockout-disable successful", "path", path, "user-lockout-disable", userLockoutDisable) + } + + } + } + + } if rawVal, ok := data.GetOk("description"); ok { description := rawVal.(string) @@ -2245,6 +2397,9 @@ func (b *SystemBackend) handleEnableAuth(ctx context.Context, req *logical.Reque return nil, logical.ErrReadOnly } + q.Q("handle enable auth") + q.Q(data) + // Get all the options path := data.Get("path").(string) path = sanitizePath(path) @@ -2381,6 +2536,8 @@ func (b *SystemBackend) handleEnableAuth(ctx context.Context, req *logical.Reque if len(apiConfig.AllowedManagedKeys) > 0 { config.AllowedManagedKeys = apiConfig.AllowedManagedKeys } + // update userLockoutConfig + config.UserLockoutConfig = b.getUserLockoutConfig(logicalType) // Create the mount entry me := &MountEntry{ @@ -2409,6 +2566,66 @@ func (b *SystemBackend) handleEnableAuth(ctx context.Context, req *logical.Reque return resp, nil } +func (b *SystemBackend) getUserLockoutConfig(logicalType string) UserLockoutConfig { + conf := b.Core.rawConfig.Load() + if conf == nil { + return UserLockoutConfig{} + } + userlockouts := conf.(*server.Config).UserLockoutConfigs + + if userlockouts == nil { + return UserLockoutConfig{} + } + + var commonUserLockoutConfig UserLockoutConfig + var authUserLockoutConfig UserLockoutConfig + for _, userLockoutConfig := range userlockouts { + if userLockoutConfig.Type == "all" { + commonUserLockoutConfig.LockoutThreshold = userLockoutConfig.LockoutThreshold + commonUserLockoutConfig.LockoutDuration = userLockoutConfig.LockoutDuration + commonUserLockoutConfig.LockoutCounterReset = userLockoutConfig.LockoutCounterReset + commonUserLockoutConfig.DisableLockout = userLockoutConfig.DisableLockout + } + if userLockoutConfig.Type == logicalType { + authUserLockoutConfig.LockoutThreshold = userLockoutConfig.LockoutThreshold + authUserLockoutConfig.LockoutDuration = userLockoutConfig.LockoutDuration + authUserLockoutConfig.LockoutCounterReset = userLockoutConfig.LockoutCounterReset + authUserLockoutConfig.DisableLockout = userLockoutConfig.DisableLockout + } + } + + // switch { + // case authUserLockoutConfig != UserLockoutConfig{}: + // return authUserLockoutConfig + // case commonUserLockoutConfig != UserLockoutConfig{}: + // return commonUserLockoutConfig + // } + // return UserLockoutConfig{} + + if (authUserLockoutConfig != UserLockoutConfig{}) { + return authUserLockoutConfig + } + return commonUserLockoutConfig + // var currentUserLockoutConfig UserLockoutConfig + // userLockoutConfigMap := make(map[string]*configutil.UserLockoutConfig) + // userLockoutConfigMap = configutil.SetMissingUserLockoutValuesInMap(userLockoutConfigMap) + // userLockoutAuthConfig, ok := userLockoutConfigMap[strings.ToLower(logicalType)] + // switch ok { + // case true: + // currentUserLockoutConfig.LockoutThreshold = userLockoutAuthConfig.LockoutThreshold + // currentUserLockoutConfig.LockoutDuration = userLockoutAuthConfig.LockoutDuration + // currentUserLockoutConfig.LockoutCounterReset = userLockoutAuthConfig.LockoutCounterReset + // currentUserLockoutConfig.DisableLockout = userLockoutAuthConfig.DisableLockout + + // default: + // currentUserLockoutConfig.LockoutThreshold = userLockoutConfigMap["all"].LockoutThreshold + // currentUserLockoutConfig.LockoutDuration = userLockoutConfigMap["all"].LockoutDuration + // currentUserLockoutConfig.LockoutCounterReset = userLockoutConfigMap["all"].LockoutCounterReset + // currentUserLockoutConfig.DisableLockout = userLockoutConfigMap["all"].DisableLockout + // } + // return currentUserLockoutConfig +} + // handleDisableAuth is used to disable a credential backend func (b *SystemBackend) handleDisableAuth(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { path := data.Get("path").(string) diff --git a/vault/logical_system_paths.go b/vault/logical_system_paths.go index ee666adc6341..20e8c29da7f7 100644 --- a/vault/logical_system_paths.go +++ b/vault/logical_system_paths.go @@ -1542,6 +1542,10 @@ func (b *SystemBackend) authPaths() []*framework.Path { Type: framework.TypeString, Description: strings.TrimSpace(sysHelp["token_type"][0]), }, + "user_lockout_config": { + Type: framework.TypeMap, + Description: strings.TrimSpace(sysHelp["user_lockout_config"][0]), + }, }, Operations: map[logical.Operation]framework.OperationHandler{ logical.ReadOperation: &framework.PathOperation{ diff --git a/vault/mount.go b/vault/mount.go index de60353fd2c9..18403cdcee82 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -16,6 +16,7 @@ import ( "github.com/hashicorp/vault/builtin/plugin" "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/vault/helper/namespace" + "github.com/hashicorp/vault/internalshared/configutil" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/logical" @@ -348,6 +349,7 @@ type MountConfig struct { AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" structs:"allowed_response_headers" mapstructure:"allowed_response_headers"` TokenType logical.TokenType `json:"token_type,omitempty" structs:"token_type" mapstructure:"token_type"` AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` + UserLockoutConfig UserLockoutConfig `json:"user_lockout_config,omitempty" mapstructure:"user_lockout_config"` // PluginName is the name of the plugin registered in the catalog. // @@ -355,6 +357,22 @@ type MountConfig struct { PluginName string `json:"plugin_name,omitempty" structs:"plugin_name,omitempty" mapstructure:"plugin_name"` } +type UserLockoutConfig struct { + LockoutThreshold int64 `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` // Override for global default + LockoutDuration time.Duration `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` // Override for global default + LockoutCounterReset time.Duration `json:"lockout_counter_reset,omitempty" structs:"lockout_counter_reset" mapstructure:"lockout_counter_reset"` // Override for global default + DisableLockout bool `json:"disable_lockout,omitempty" structs:"disable_lockout" mapstructure:"disable_lockout"` // Override for global default + +} + +type APIUserLockoutConfig struct { + LockoutThreshold string `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` // Override for global default + LockoutDuration string `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` // Override for global default + LockoutCounterResetDuration string `json:"lockout_counter_reset_duration,omitempty" structs:"lockout_counter_reset_duration" mapstructure:"lockout_counter_reset_duration"` // Override for global default + DisableLockout bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` // Override for global default + +} + // APIMountConfig is an embedded struct of api.MountConfigInput type APIMountConfig struct { DefaultLeaseTTL string `json:"default_lease_ttl" structs:"default_lease_ttl" mapstructure:"default_lease_ttl"` @@ -367,7 +385,7 @@ type APIMountConfig struct { AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" structs:"allowed_response_headers" mapstructure:"allowed_response_headers"` TokenType string `json:"token_type" structs:"token_type" mapstructure:"token_type"` AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` - + UserLockoutConfig UserLockoutConfig `json:"user_lockout_config,omitempty" mapstructure:"user_lockout_config"` // PluginName is the name of the plugin registered in the catalog. // // Deprecated: MountEntry.Type should be used instead for Vault 1.0.0 and beyond. @@ -1262,6 +1280,25 @@ func (c *Core) runMountUpdates(ctx context.Context, needPersist bool) error { entry.NamespaceID = namespace.RootNamespaceID needPersist = true } + + if (entry.Config.UserLockoutConfig == UserLockoutConfig{}) { + userLockoutConfigMap := make(map[string]*configutil.UserLockoutConfig) + userLockoutConfigMap = configutil.SetMissingUserLockoutValuesInMap(userLockoutConfigMap) + userLockoutAuthConfig, ok := userLockoutConfigMap[strings.ToLower(entry.Type)] + switch ok { + case true: + entry.Config.UserLockoutConfig.LockoutThreshold = userLockoutAuthConfig.LockoutThreshold + entry.Config.UserLockoutConfig.LockoutDuration = userLockoutAuthConfig.LockoutDuration + entry.Config.UserLockoutConfig.LockoutCounterReset = userLockoutAuthConfig.LockoutCounterReset + entry.Config.UserLockoutConfig.DisableLockout = userLockoutAuthConfig.DisableLockout + + default: + entry.Config.UserLockoutConfig.LockoutThreshold = userLockoutConfigMap["all"].LockoutThreshold + entry.Config.UserLockoutConfig.LockoutDuration = userLockoutConfigMap["all"].LockoutDuration + entry.Config.UserLockoutConfig.LockoutCounterReset = userLockoutConfigMap["all"].LockoutCounterReset + entry.Config.UserLockoutConfig.DisableLockout = userLockoutConfigMap["all"].DisableLockout + } + } } // Done if we have restored the mount table and we don't need diff --git a/vault/request_handling.go b/vault/request_handling.go index b37085c83c10..2b75f4c7aa24 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -17,6 +17,7 @@ import ( "github.com/hashicorp/go-secure-stdlib/strutil" "github.com/hashicorp/go-sockaddr" "github.com/hashicorp/go-uuid" + "github.com/hashicorp/vault/command/server" "github.com/hashicorp/vault/helper/identity" "github.com/hashicorp/vault/helper/identity/mfa" "github.com/hashicorp/vault/helper/metricsutil" @@ -31,6 +32,7 @@ import ( "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/vault/quotas" "github.com/hashicorp/vault/vault/tokens" + "github.com/ryboe/q" uberAtomic "go.uber.org/atomic" ) @@ -250,6 +252,10 @@ func (c *Core) fetchACLTokenEntryAndEntity(ctx context.Context, req *logical.Req return acl, te, entity, identityPolicies, nil } +func (c *Core) isUserLockoutDisabled(authMethodName string) bool { + return false +} + func (c *Core) checkToken(ctx context.Context, req *logical.Request, unauth bool) (*logical.Auth, *logical.TokenEntry, error) { defer metrics.MeasureSince([]string{"core", "check_token"}, time.Now()) @@ -1325,6 +1331,25 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re return nil, nil, ErrInternalError } + // testing reading userlockout config + + q.Q("read user lockout config in handleLoginRequest") + // q.Q(c.rawConfig) + conf := c.rawConfig.Load() + if conf == nil { + q.Q("failed to load core raw config") + } + userlockouts := conf.(*server.Config).UserLockoutConfigs + if userlockouts == nil { + q.Q("user lockouts configured") + } else { + q.Q("user lockouts configs") + q.Q(userlockouts) + } + q.Q("hi") + q.Q("details of mount entry") + q.Q(entry) + // Route the request resp, routeErr := c.doRouting(ctx, req) if resp != nil { From 9fc0fa2f0caf7792b5f668f6e9952e79bc5c128b Mon Sep 17 00:00:00 2001 From: akshya96 Date: Wed, 21 Sep 2022 19:15:28 -0700 Subject: [PATCH 03/24] auth tune r/w and auth tune --- api/sys_mounts.go | 33 ++++++++++++++++++++++++------ command/auth_tune.go | 43 ++++++++++++++++++++-------------------- command/server/config.go | 7 ++----- vault/logical_system.go | 33 ++++++++++++++++++------------ 4 files changed, 70 insertions(+), 46 deletions(-) diff --git a/api/sys_mounts.go b/api/sys_mounts.go index fa414162f3bb..9a9d8114e64a 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -187,6 +187,28 @@ func (c *Sys) RemountStatusWithContext(ctx context.Context, migrationID string) return &result, err } +func (c *Sys) TuneAuthMount(path string, config MountConfigInput) error { + return c.TuneAuthMountWithContext(context.Background(), path, config) +} + +func (c *Sys) TuneAuthMountWithContext(ctx context.Context, path string, config MountConfigInput) error { + ctx, cancelFunc := c.c.withConfiguredTimeout(ctx) + defer cancelFunc() + + r := c.c.NewRequest(http.MethodPost, fmt.Sprintf("/v1/sys/%s/tune", path)) + if err := r.SetJSONBody(config); err != nil { + return err + } + + q.Q("request") + q.Q(r) + resp, err := c.c.rawRequestWithContext(ctx, r) + if err == nil { + defer resp.Body.Close() + } + return err +} + func (c *Sys) TuneMount(path string, config MountConfigInput) error { return c.TuneMountWithContext(context.Background(), path, config) } @@ -200,7 +222,6 @@ func (c *Sys) TuneMountWithContext(ctx context.Context, path string, config Moun return err } - q.Q("request") q.Q(r) resp, err := c.c.rawRequestWithContext(ctx, r) @@ -271,7 +292,7 @@ type MountConfigInput struct { AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` - UserLockoutConfig UserLockoutInputConfig `json:"user_lockout_config"` + UserLockoutConfig UserLockoutConfigInput `json:"user_lockout_config"` // Deprecated: This field will always be blank for newer server responses. PluginName string `json:"plugin_name,omitempty" mapstructure:"plugin_name"` @@ -305,20 +326,20 @@ type MountConfigOutput struct { AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` - UserLockoutConfig UserLockoutOutputConfig `json:"user_lockout_config"` + UserLockoutConfig UserLockoutConfigOutput `json:"user_lockout_config"` // Deprecated: This field will always be blank for newer server responses. PluginName string `json:"plugin_name,omitempty" mapstructure:"plugin_name"` } -type UserLockoutInputConfig struct { +type UserLockoutConfigInput struct { LockoutThreshold string `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` // Override for global default LockoutDuration string `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` // Override for global default LockoutCounterResetDuration string `json:"lockout_counter_reset_duration,omitempty" structs:"lockout_counter_reset_duration" mapstructure:"lockout_counter_reset_duration"` // Override for global default - DisableLockout bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` // Override for global default + DisableLockout *bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` // Override for global default } -type UserLockoutOutputConfig struct { +type UserLockoutConfigOutput struct { LockoutThreshold int `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` // Override for global default LockoutDuration int `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` // Override for global default LockoutCounterReset int `json:"lockout_counter_reset,omitempty" structs:"lockout_counter_reset" mapstructure:"lockout_counter_reset"` // Override for global default diff --git a/command/auth_tune.go b/command/auth_tune.go index c61af282f4f0..f651a0a99193 100644 --- a/command/auth_tune.go +++ b/command/auth_tune.go @@ -10,7 +10,6 @@ import ( "github.com/hashicorp/vault/api" "github.com/mitchellh/cli" "github.com/posener/complete" - "github.com/ryboe/q" ) var ( @@ -21,21 +20,21 @@ var ( type AuthTuneCommand struct { *BaseCommand - flagAuditNonHMACRequestKeys []string - flagAuditNonHMACResponseKeys []string - flagDefaultLeaseTTL time.Duration - flagDescription string - flagListingVisibility string - flagMaxLeaseTTL time.Duration - flagPassthroughRequestHeaders []string - flagAllowedResponseHeaders []string - flagOptions map[string]string - flagTokenType string - flagVersion int - flagUserLockoutThreshold int - flagUserLockoutDuration time.Duration - flagUserLockoutCounterResetDuration time.Duration - flagUserLockoutDisable bool + flagAuditNonHMACRequestKeys []string + flagAuditNonHMACResponseKeys []string + flagDefaultLeaseTTL time.Duration + flagDescription string + flagListingVisibility string + flagMaxLeaseTTL time.Duration + flagPassthroughRequestHeaders []string + flagAllowedResponseHeaders []string + flagOptions map[string]string + flagTokenType string + flagVersion int + flagUserLockoutThreshold int + flagUserLockoutDuration time.Duration + flagUserLockoutCounterResetDuration time.Duration + flagUserLockoutDisable bool } func (c *AuthTuneCommand) Synopsis() string { @@ -176,8 +175,9 @@ func (c *AuthTuneCommand) Flags() *FlagSets { }) f.BoolVar(&BoolVar{ - Name: "user-lockout-disable", - Target: &c.flagUserLockoutDisable, + Name: "user-lockout-disable", + Target: &c.flagUserLockoutDisable, + Default: false, Usage: "Disable user lockout for this auth method. If unspecified, this " + "defaults to the Vault server's globally configured user lockout disable, " + "or a previously configured value for the auth method.", @@ -268,20 +268,19 @@ func (c *AuthTuneCommand) Run(args []string) int { if fl.Name == "user-lockout-duration" { mountConfigInput.UserLockoutConfig.LockoutDuration = ttlToAPI(c.flagUserLockoutDuration) } - if fl.Name == "user-lockout-counter-reset" { + if fl.Name == "user-lockout-counter-reset-duration" { mountConfigInput.UserLockoutConfig.LockoutCounterResetDuration = ttlToAPI(c.flagUserLockoutCounterResetDuration) } if fl.Name == "user-lockout-disable" { - mountConfigInput.UserLockoutConfig.DisableLockout = c.flagUserLockoutDisable + mountConfigInput.UserLockoutConfig.DisableLockout = &c.flagUserLockoutDisable } }) - q.Q(mountConfigInput) // Append /auth (since that's where auths live) and a trailing slash to // indicate it's a path in output mountPath := ensureTrailingSlash(sanitizePath(args[0])) - if err := client.Sys().TuneMount("/auth/"+mountPath, mountConfigInput); err != nil { + if err := client.Sys().TuneAuthMount("/auth/"+mountPath, mountConfigInput); err != nil { c.UI.Error(fmt.Sprintf("Error tuning auth method %s: %s", mountPath, err)) return 2 } diff --git a/command/server/config.go b/command/server/config.go index 115398d2ed41..61f95e473896 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -511,12 +511,9 @@ func LoadConfigFile(path string) (*Config, error) { q.Q("user lockout configs") q.Q(conf.UserLockoutConfigs) - //update the mount entries with the configuration - - // get the mount entry - - + // update the mount entries with the configuration + // get the mount entry return conf, nil } diff --git a/vault/logical_system.go b/vault/logical_system.go index a7bbd17b1263..2da8554072fe 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -45,7 +45,6 @@ import ( "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/sdk/version" "github.com/mitchellh/mapstructure" - "github.com/ryboe/q" ) const ( @@ -929,6 +928,15 @@ func (b *SystemBackend) mountInfo(ctx context.Context, entry *MountEntry) map[st if entry.Table == credentialTableType { entryConfig["token_type"] = entry.Config.TokenType.String() } + if (entry.Config.UserLockoutConfig != UserLockoutConfig{}) { + userLockoutConfig := map[string]interface{}{ + "user_lockout_counter_reset_duration": int64(entry.Config.UserLockoutConfig.LockoutCounterReset.Seconds()), + "user_lockout_threshold": entry.Config.UserLockoutConfig.LockoutThreshold, + "user_lockout_duration": int64(entry.Config.UserLockoutConfig.LockoutDuration.Seconds()), + "user_lockout_disable": entry.Config.UserLockoutConfig.DisableLockout, + } + entryConfig["user_lockout_config"] = userLockoutConfig + } // Add deprecation status only if it exists builtinType := b.Core.builtinTypeFromMountEntry(ctx, entry) @@ -1519,14 +1527,10 @@ func (b *SystemBackend) handleTuneReadCommon(ctx context.Context, path string) ( resp := &logical.Response{ Data: map[string]interface{}{ - "description": mountEntry.Description, - "default_lease_ttl": int(sysView.DefaultLeaseTTL().Seconds()), - "max_lease_ttl": int(sysView.MaxLeaseTTL().Seconds()), - "force_no_cache": mountEntry.Config.ForceNoCache, - "user_lockout_counter_reset_duration": int64(mountEntry.Config.UserLockoutConfig.LockoutCounterReset.Seconds()), - "user_lockout_threshold": mountEntry.Config.UserLockoutConfig.LockoutThreshold, - "user_lockout_duration": int64(mountEntry.Config.UserLockoutConfig.LockoutDuration.Seconds()), - "user_lockout_disable": mountEntry.Config.UserLockoutConfig.DisableLockout, + "description": mountEntry.Description, + "default_lease_ttl": int(sysView.DefaultLeaseTTL().Seconds()), + "max_lease_ttl": int(sysView.MaxLeaseTTL().Seconds()), + "force_no_cache": mountEntry.Config.ForceNoCache, }, } @@ -1563,6 +1567,13 @@ func (b *SystemBackend) handleTuneReadCommon(ctx context.Context, path string) ( resp.Data["allowed_managed_keys"] = rawVal.([]string) } + if (mountEntry.Config.UserLockoutConfig != UserLockoutConfig{}) { + resp.Data["user_lockout_counter_reset_duration"] = int64(mountEntry.Config.UserLockoutConfig.LockoutCounterReset.Seconds()) + resp.Data["user_lockout_threshold"] = mountEntry.Config.UserLockoutConfig.LockoutThreshold + resp.Data["user_lockout_duration"] = int64(mountEntry.Config.UserLockoutConfig.LockoutDuration.Seconds()) + resp.Data["user_lockout_disable"] = mountEntry.Config.UserLockoutConfig.DisableLockout + } + if len(mountEntry.Options) > 0 { resp.Data["options"] = mountEntry.Options } @@ -1582,7 +1593,6 @@ func (b *SystemBackend) handleAuthTuneWrite(ctx context.Context, req *logical.Re // handleMountTuneWrite is used to set config settings on a backend func (b *SystemBackend) handleMountTuneWrite(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { - q.Q(data) path := data.Get("path").(string) if path == "" { return logical.ErrorResponse("missing path"), nil @@ -2397,9 +2407,6 @@ func (b *SystemBackend) handleEnableAuth(ctx context.Context, req *logical.Reque return nil, logical.ErrReadOnly } - q.Q("handle enable auth") - q.Q(data) - // Get all the options path := data.Get("path").(string) path = sanitizePath(path) From 77aeee38b52a77ddaf91f3d6f5728911c08fefb4 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Fri, 23 Sep 2022 14:49:35 -0700 Subject: [PATCH 04/24] removing changes at enable --- api/sys_mounts.go | 2 -- vault/logical_system.go | 71 +++++---------------------------------- vault/mount.go | 20 ----------- vault/request_handling.go | 48 +++++++++++++++++++++++++- 4 files changed, 55 insertions(+), 86 deletions(-) diff --git a/api/sys_mounts.go b/api/sys_mounts.go index 9a9d8114e64a..d085ca002f4c 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -200,8 +200,6 @@ func (c *Sys) TuneAuthMountWithContext(ctx context.Context, path string, config return err } - q.Q("request") - q.Q(r) resp, err := c.c.rawRequestWithContext(ctx, r) if err == nil { defer resp.Body.Close() diff --git a/vault/logical_system.go b/vault/logical_system.go index 2da8554072fe..0dd0a890a1fc 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -30,7 +30,6 @@ import ( "github.com/hashicorp/go-secure-stdlib/parseutil" "github.com/hashicorp/go-secure-stdlib/strutil" semver "github.com/hashicorp/go-version" - "github.com/hashicorp/vault/command/server" "github.com/hashicorp/vault/helper/hostutil" "github.com/hashicorp/vault/helper/identity" "github.com/hashicorp/vault/helper/metricsutil" @@ -1709,6 +1708,14 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, "unable to convert given mount config information"), logical.ErrInvalidRequest } + + switch strings.ToLower(mountEntry.Type) { + case "all", "ldap", "approle", "userpass": + default: + return logical.ErrorResponse("mount tuning of user lockout configuration for auth type %q not allowed", mountEntry.Type), + logical.ErrInvalidRequest + + } } if _, ok := userLockoutConfigMap["lockout_threshold"]; ok { @@ -2543,8 +2550,6 @@ func (b *SystemBackend) handleEnableAuth(ctx context.Context, req *logical.Reque if len(apiConfig.AllowedManagedKeys) > 0 { config.AllowedManagedKeys = apiConfig.AllowedManagedKeys } - // update userLockoutConfig - config.UserLockoutConfig = b.getUserLockoutConfig(logicalType) // Create the mount entry me := &MountEntry{ @@ -2573,66 +2578,6 @@ func (b *SystemBackend) handleEnableAuth(ctx context.Context, req *logical.Reque return resp, nil } -func (b *SystemBackend) getUserLockoutConfig(logicalType string) UserLockoutConfig { - conf := b.Core.rawConfig.Load() - if conf == nil { - return UserLockoutConfig{} - } - userlockouts := conf.(*server.Config).UserLockoutConfigs - - if userlockouts == nil { - return UserLockoutConfig{} - } - - var commonUserLockoutConfig UserLockoutConfig - var authUserLockoutConfig UserLockoutConfig - for _, userLockoutConfig := range userlockouts { - if userLockoutConfig.Type == "all" { - commonUserLockoutConfig.LockoutThreshold = userLockoutConfig.LockoutThreshold - commonUserLockoutConfig.LockoutDuration = userLockoutConfig.LockoutDuration - commonUserLockoutConfig.LockoutCounterReset = userLockoutConfig.LockoutCounterReset - commonUserLockoutConfig.DisableLockout = userLockoutConfig.DisableLockout - } - if userLockoutConfig.Type == logicalType { - authUserLockoutConfig.LockoutThreshold = userLockoutConfig.LockoutThreshold - authUserLockoutConfig.LockoutDuration = userLockoutConfig.LockoutDuration - authUserLockoutConfig.LockoutCounterReset = userLockoutConfig.LockoutCounterReset - authUserLockoutConfig.DisableLockout = userLockoutConfig.DisableLockout - } - } - - // switch { - // case authUserLockoutConfig != UserLockoutConfig{}: - // return authUserLockoutConfig - // case commonUserLockoutConfig != UserLockoutConfig{}: - // return commonUserLockoutConfig - // } - // return UserLockoutConfig{} - - if (authUserLockoutConfig != UserLockoutConfig{}) { - return authUserLockoutConfig - } - return commonUserLockoutConfig - // var currentUserLockoutConfig UserLockoutConfig - // userLockoutConfigMap := make(map[string]*configutil.UserLockoutConfig) - // userLockoutConfigMap = configutil.SetMissingUserLockoutValuesInMap(userLockoutConfigMap) - // userLockoutAuthConfig, ok := userLockoutConfigMap[strings.ToLower(logicalType)] - // switch ok { - // case true: - // currentUserLockoutConfig.LockoutThreshold = userLockoutAuthConfig.LockoutThreshold - // currentUserLockoutConfig.LockoutDuration = userLockoutAuthConfig.LockoutDuration - // currentUserLockoutConfig.LockoutCounterReset = userLockoutAuthConfig.LockoutCounterReset - // currentUserLockoutConfig.DisableLockout = userLockoutAuthConfig.DisableLockout - - // default: - // currentUserLockoutConfig.LockoutThreshold = userLockoutConfigMap["all"].LockoutThreshold - // currentUserLockoutConfig.LockoutDuration = userLockoutConfigMap["all"].LockoutDuration - // currentUserLockoutConfig.LockoutCounterReset = userLockoutConfigMap["all"].LockoutCounterReset - // currentUserLockoutConfig.DisableLockout = userLockoutConfigMap["all"].DisableLockout - // } - // return currentUserLockoutConfig -} - // handleDisableAuth is used to disable a credential backend func (b *SystemBackend) handleDisableAuth(ctx context.Context, req *logical.Request, data *framework.FieldData) (*logical.Response, error) { path := data.Get("path").(string) diff --git a/vault/mount.go b/vault/mount.go index 18403cdcee82..bba12533b241 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -16,7 +16,6 @@ import ( "github.com/hashicorp/vault/builtin/plugin" "github.com/hashicorp/vault/helper/metricsutil" "github.com/hashicorp/vault/helper/namespace" - "github.com/hashicorp/vault/internalshared/configutil" "github.com/hashicorp/vault/sdk/helper/consts" "github.com/hashicorp/vault/sdk/helper/jsonutil" "github.com/hashicorp/vault/sdk/logical" @@ -1281,26 +1280,7 @@ func (c *Core) runMountUpdates(ctx context.Context, needPersist bool) error { needPersist = true } - if (entry.Config.UserLockoutConfig == UserLockoutConfig{}) { - userLockoutConfigMap := make(map[string]*configutil.UserLockoutConfig) - userLockoutConfigMap = configutil.SetMissingUserLockoutValuesInMap(userLockoutConfigMap) - userLockoutAuthConfig, ok := userLockoutConfigMap[strings.ToLower(entry.Type)] - switch ok { - case true: - entry.Config.UserLockoutConfig.LockoutThreshold = userLockoutAuthConfig.LockoutThreshold - entry.Config.UserLockoutConfig.LockoutDuration = userLockoutAuthConfig.LockoutDuration - entry.Config.UserLockoutConfig.LockoutCounterReset = userLockoutAuthConfig.LockoutCounterReset - entry.Config.UserLockoutConfig.DisableLockout = userLockoutAuthConfig.DisableLockout - - default: - entry.Config.UserLockoutConfig.LockoutThreshold = userLockoutConfigMap["all"].LockoutThreshold - entry.Config.UserLockoutConfig.LockoutDuration = userLockoutConfigMap["all"].LockoutDuration - entry.Config.UserLockoutConfig.LockoutCounterReset = userLockoutConfigMap["all"].LockoutCounterReset - entry.Config.UserLockoutConfig.DisableLockout = userLockoutConfigMap["all"].DisableLockout - } - } } - // Done if we have restored the mount table and we don't need // to persist if !needPersist { diff --git a/vault/request_handling.go b/vault/request_handling.go index 2b75f4c7aa24..3923720d768e 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -1348,7 +1348,20 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re } q.Q("hi") q.Q("details of mount entry") - q.Q(entry) + userLockoutConfigurations := UserLockoutConfig{} + switch { + case entry.Config.UserLockoutConfig == UserLockoutConfig{}: + + userLockoutConfigurations = c.GetUserLockoutFromConfig(entry.Type) + + default: + userLockoutConfigurations = entry.Config.UserLockoutConfig + + } + + q.Q(userLockoutConfigurations) + + // get userlockout configuration // Route the request resp, routeErr := c.doRouting(ctx, req) @@ -2017,3 +2030,36 @@ func (c *Core) checkSSCTokenInternal(ctx context.Context, token string, isPerfSt // status code. return "", logical.ErrMissingRequiredState } + +func (c *Core) GetUserLockoutFromConfig(logicalType string) UserLockoutConfig { + conf := c.rawConfig.Load() + if conf == nil { + return UserLockoutConfig{} + } + userlockouts := conf.(*server.Config).UserLockoutConfigs + + if userlockouts == nil { + return UserLockoutConfig{} + } + + var commonUserLockoutConfig UserLockoutConfig + var authUserLockoutConfig UserLockoutConfig + for _, userLockoutConfig := range userlockouts { + if userLockoutConfig.Type == "all" { + commonUserLockoutConfig.LockoutThreshold = userLockoutConfig.LockoutThreshold + commonUserLockoutConfig.LockoutDuration = userLockoutConfig.LockoutDuration + commonUserLockoutConfig.LockoutCounterReset = userLockoutConfig.LockoutCounterReset + commonUserLockoutConfig.DisableLockout = userLockoutConfig.DisableLockout + } + if userLockoutConfig.Type == logicalType { + authUserLockoutConfig.LockoutThreshold = userLockoutConfig.LockoutThreshold + authUserLockoutConfig.LockoutDuration = userLockoutConfig.LockoutDuration + authUserLockoutConfig.LockoutCounterReset = userLockoutConfig.LockoutCounterReset + authUserLockoutConfig.DisableLockout = userLockoutConfig.DisableLockout + } + } + if (authUserLockoutConfig != UserLockoutConfig{}) { + return authUserLockoutConfig + } + return commonUserLockoutConfig +} From 555e3502929bf19f562a9eaba64853fa2d222b93 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Tue, 27 Sep 2022 14:46:54 -0700 Subject: [PATCH 05/24] removing q.Q --- api/sys_mounts.go | 3 --- command/server/config.go | 9 --------- vault/request_handling.go | 33 --------------------------------- 3 files changed, 45 deletions(-) diff --git a/api/sys_mounts.go b/api/sys_mounts.go index d085ca002f4c..066179774d3a 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -8,7 +8,6 @@ import ( "time" "github.com/mitchellh/mapstructure" - "github.com/ryboe/q" ) func (c *Sys) ListMounts() (map[string]*MountOutput, error) { @@ -220,8 +219,6 @@ func (c *Sys) TuneMountWithContext(ctx context.Context, path string, config Moun return err } - q.Q("request") - q.Q(r) resp, err := c.c.rawRequestWithContext(ctx, r) if err == nil { defer resp.Body.Close() diff --git a/command/server/config.go b/command/server/config.go index 61f95e473896..583d2481156b 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -20,7 +20,6 @@ import ( "github.com/hashicorp/vault/helper/osutil" "github.com/hashicorp/vault/internalshared/configutil" "github.com/hashicorp/vault/sdk/helper/consts" - "github.com/ryboe/q" ) const ( @@ -506,14 +505,6 @@ func LoadConfigFile(path string) (*Config, error) { } } } - q.Q("Reading from config file") - q.Q(conf.Listeners) - q.Q("user lockout configs") - q.Q(conf.UserLockoutConfigs) - - // update the mount entries with the configuration - - // get the mount entry return conf, nil } diff --git a/vault/request_handling.go b/vault/request_handling.go index 3923720d768e..fb00f8764331 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -32,7 +32,6 @@ import ( "github.com/hashicorp/vault/sdk/logical" "github.com/hashicorp/vault/vault/quotas" "github.com/hashicorp/vault/vault/tokens" - "github.com/ryboe/q" uberAtomic "go.uber.org/atomic" ) @@ -1331,38 +1330,6 @@ func (c *Core) handleLoginRequest(ctx context.Context, req *logical.Request) (re return nil, nil, ErrInternalError } - // testing reading userlockout config - - q.Q("read user lockout config in handleLoginRequest") - // q.Q(c.rawConfig) - conf := c.rawConfig.Load() - if conf == nil { - q.Q("failed to load core raw config") - } - userlockouts := conf.(*server.Config).UserLockoutConfigs - if userlockouts == nil { - q.Q("user lockouts configured") - } else { - q.Q("user lockouts configs") - q.Q(userlockouts) - } - q.Q("hi") - q.Q("details of mount entry") - userLockoutConfigurations := UserLockoutConfig{} - switch { - case entry.Config.UserLockoutConfig == UserLockoutConfig{}: - - userLockoutConfigurations = c.GetUserLockoutFromConfig(entry.Type) - - default: - userLockoutConfigurations = entry.Config.UserLockoutConfig - - } - - q.Q(userLockoutConfigurations) - - // get userlockout configuration - // Route the request resp, routeErr := c.doRouting(ctx, req) if resp != nil { From 4cbf934bc94cc6527cb95f898f1e332f798cf52a Mon Sep 17 00:00:00 2001 From: akshya96 Date: Tue, 27 Sep 2022 14:51:56 -0700 Subject: [PATCH 06/24] go mod tidy --- command/server/config.go | 1 - go.mod | 1 - go.sum | 2 -- 3 files changed, 4 deletions(-) diff --git a/command/server/config.go b/command/server/config.go index 583d2481156b..5d985db69d08 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -505,7 +505,6 @@ func LoadConfigFile(path string) (*Config, error) { } } } - return conf, nil } diff --git a/go.mod b/go.mod index a7dc43754514..56d2bfe5981a 100644 --- a/go.mod +++ b/go.mod @@ -381,7 +381,6 @@ require ( github.com/prometheus/procfs v0.6.0 // indirect github.com/renier/xmlrpc v0.0.0-20170708154548-ce4a1a486c03 // indirect github.com/rogpeppe/go-internal v1.8.1 // indirect - github.com/ryboe/q v1.0.17 // indirect github.com/sirupsen/logrus v1.8.1 // indirect github.com/snowflakedb/gosnowflake v1.6.3 // indirect github.com/softlayer/softlayer-go v0.0.0-20180806151055-260589d94c7d // indirect diff --git a/go.sum b/go.sum index 865ab431a02c..8469a94fb43a 100644 --- a/go.sum +++ b/go.sum @@ -1660,8 +1660,6 @@ github.com/ryanuber/columnize v2.1.0+incompatible h1:j1Wcmh8OrK4Q7GXY+V7SVSY8nUW github.com/ryanuber/columnize v2.1.0+incompatible/go.mod h1:sm1tb6uqfes/u+d4ooFouqFdy9/2g9QGwK3SQygK0Ts= github.com/ryanuber/go-glob v1.0.0 h1:iQh3xXAumdQ+4Ufa5b25cRpC5TYKlno6hsv6Cb3pkBk= github.com/ryanuber/go-glob v1.0.0/go.mod h1:807d1WSdnB0XRJzKNil9Om6lcp/3a0v4qIHxIXzX/Yc= -github.com/ryboe/q v1.0.17 h1:Ap34VxlzBbjFHdApe1RzvBwrYmoLa4hC5J7P643ENtU= -github.com/ryboe/q v1.0.17/go.mod h1:7wNegax8bjSyGxm9Pnsy6i8z+Uy9X8hkm7pAId9PDdg= github.com/safchain/ethtool v0.0.0-20190326074333-42ed695e3de8/go.mod h1:Z0q5wiBQGYcxhMZ6gUqHn6pYNLypFAvaL3UvgZLR0U4= github.com/samuel/go-zookeeper v0.0.0-20190923202752-2cc03de413da h1:p3Vo3i64TCLY7gIfzeQaUJ+kppEO5WQG3cL8iE8tGHU= github.com/samuel/go-zookeeper v0.0.0-20190923202752-2cc03de413da/go.mod h1:gi+0XIa01GRL2eRQVjQkKGqKF3SF9vZR/HnPullcV2E= From fd216b1d58d9649249165a41cc4a0bb8d26d267a Mon Sep 17 00:00:00 2001 From: akshya96 Date: Tue, 27 Sep 2022 16:35:24 -0700 Subject: [PATCH 07/24] removing comments --- internalshared/configutil/userlockout.go | 6 +++--- vault/request_handling.go | 16 +++++++--------- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/internalshared/configutil/userlockout.go b/internalshared/configutil/userlockout.go index 1f4a96672fd8..57cd66892c55 100644 --- a/internalshared/configutil/userlockout.go +++ b/internalshared/configutil/userlockout.go @@ -95,8 +95,8 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { } userLockoutConfigsMap[userLockoutConfig.Type] = &userLockoutConfig } - // userLockoutConfigsMap = SetDefaultUserLockoutValuesInMap(userLockoutConfigsMap) - userLockoutConfigsMap = SetMissingUserLockoutValuesInMap(userLockoutConfigsMap) + + userLockoutConfigsMap = setMissingUserLockoutValuesInMap(userLockoutConfigsMap) for _, userLockoutValues := range userLockoutConfigsMap { result.UserLockoutConfigs = append(result.UserLockoutConfigs, userLockoutValues) } @@ -132,7 +132,7 @@ func setDefaultUserLockoutValuesInMap(userLockoutConfigsMap map[string]*UserLock } // setDefaultUserLockoutValuesInMap sets missing user lockout fields for other auth types with default values (from key "all") -func SetMissingUserLockoutValuesInMap(userLockoutConfigsMap map[string]*UserLockoutConfig) map[string]*UserLockoutConfig { +func setMissingUserLockoutValuesInMap(userLockoutConfigsMap map[string]*UserLockoutConfig) map[string]*UserLockoutConfig { userLockoutConfigsMap = setDefaultUserLockoutValuesInMap(userLockoutConfigsMap) for _, userLockoutAuth := range userLockoutConfigsMap { // set missing values diff --git a/vault/request_handling.go b/vault/request_handling.go index fb00f8764331..73782183d5af 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -2012,21 +2012,19 @@ func (c *Core) GetUserLockoutFromConfig(logicalType string) UserLockoutConfig { var commonUserLockoutConfig UserLockoutConfig var authUserLockoutConfig UserLockoutConfig for _, userLockoutConfig := range userlockouts { + if userLockoutConfig.Type == logicalType{ + authUserLockoutConfig.LockoutThreshold = userLockoutConfig.LockoutThreshold + authUserLockoutConfig.LockoutDuration = userLockoutConfig.LockoutDuration + authUserLockoutConfig.LockoutCounterReset = userLockoutConfig.LockoutCounterReset + authUserLockoutConfig.DisableLockout = userLockoutConfig.DisableLockout + return authUserLockoutConfig + } if userLockoutConfig.Type == "all" { commonUserLockoutConfig.LockoutThreshold = userLockoutConfig.LockoutThreshold commonUserLockoutConfig.LockoutDuration = userLockoutConfig.LockoutDuration commonUserLockoutConfig.LockoutCounterReset = userLockoutConfig.LockoutCounterReset commonUserLockoutConfig.DisableLockout = userLockoutConfig.DisableLockout } - if userLockoutConfig.Type == logicalType { - authUserLockoutConfig.LockoutThreshold = userLockoutConfig.LockoutThreshold - authUserLockoutConfig.LockoutDuration = userLockoutConfig.LockoutDuration - authUserLockoutConfig.LockoutCounterReset = userLockoutConfig.LockoutCounterReset - authUserLockoutConfig.DisableLockout = userLockoutConfig.DisableLockout - } - } - if (authUserLockoutConfig != UserLockoutConfig{}) { - return authUserLockoutConfig } return commonUserLockoutConfig } From 9219b5458d7d874cd2ded06ab36e228fb79555a6 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Tue, 27 Sep 2022 17:19:25 -0700 Subject: [PATCH 08/24] changing struct name for config file --- api/sys_mounts.go | 26 +++++++------- command/auth_tune.go | 24 ++++++------- internalshared/configutil/config.go | 8 ++--- internalshared/configutil/merge.go | 8 ++--- internalshared/configutil/userlockout.go | 46 ++++++++++++------------ vault/request_handling.go | 4 +-- 6 files changed, 58 insertions(+), 58 deletions(-) diff --git a/api/sys_mounts.go b/api/sys_mounts.go index 7111461548bb..ad1ff54ed8dc 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -274,19 +274,19 @@ type MountInput struct { } type MountConfigInput struct { - Options map[string]string `json:"options" mapstructure:"options"` - DefaultLeaseTTL string `json:"default_lease_ttl" mapstructure:"default_lease_ttl"` - Description *string `json:"description,omitempty" mapstructure:"description"` - MaxLeaseTTL string `json:"max_lease_ttl" mapstructure:"max_lease_ttl"` - ForceNoCache bool `json:"force_no_cache" mapstructure:"force_no_cache"` - AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" mapstructure:"audit_non_hmac_request_keys"` - AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" mapstructure:"audit_non_hmac_response_keys"` - ListingVisibility string `json:"listing_visibility,omitempty" mapstructure:"listing_visibility"` - PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" mapstructure:"passthrough_request_headers"` - AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` - TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` - AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` - PluginVersion string `json:"plugin_version,omitempty"` + Options map[string]string `json:"options" mapstructure:"options"` + DefaultLeaseTTL string `json:"default_lease_ttl" mapstructure:"default_lease_ttl"` + Description *string `json:"description,omitempty" mapstructure:"description"` + MaxLeaseTTL string `json:"max_lease_ttl" mapstructure:"max_lease_ttl"` + ForceNoCache bool `json:"force_no_cache" mapstructure:"force_no_cache"` + AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" mapstructure:"audit_non_hmac_request_keys"` + AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" mapstructure:"audit_non_hmac_response_keys"` + ListingVisibility string `json:"listing_visibility,omitempty" mapstructure:"listing_visibility"` + PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" mapstructure:"passthrough_request_headers"` + AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` + TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` + AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` + PluginVersion string `json:"plugin_version,omitempty"` UserLockoutConfig UserLockoutConfigInput `json:"user_lockout_config"` // Deprecated: This field will always be blank for newer server responses. PluginName string `json:"plugin_name,omitempty" mapstructure:"plugin_name"` diff --git a/command/auth_tune.go b/command/auth_tune.go index 5a7572c1e0f4..3a4bd6a394c3 100644 --- a/command/auth_tune.go +++ b/command/auth_tune.go @@ -20,18 +20,18 @@ var ( type AuthTuneCommand struct { *BaseCommand - flagAuditNonHMACRequestKeys []string - flagAuditNonHMACResponseKeys []string - flagDefaultLeaseTTL time.Duration - flagDescription string - flagListingVisibility string - flagMaxLeaseTTL time.Duration - flagPassthroughRequestHeaders []string - flagAllowedResponseHeaders []string - flagOptions map[string]string - flagTokenType string - flagVersion int - flagPluginVersion string + flagAuditNonHMACRequestKeys []string + flagAuditNonHMACResponseKeys []string + flagDefaultLeaseTTL time.Duration + flagDescription string + flagListingVisibility string + flagMaxLeaseTTL time.Duration + flagPassthroughRequestHeaders []string + flagAllowedResponseHeaders []string + flagOptions map[string]string + flagTokenType string + flagVersion int + flagPluginVersion string flagUserLockoutThreshold int flagUserLockoutDuration time.Duration flagUserLockoutCounterResetDuration time.Duration diff --git a/internalshared/configutil/config.go b/internalshared/configutil/config.go index 0877338dc6f4..bddaba964748 100644 --- a/internalshared/configutil/config.go +++ b/internalshared/configutil/config.go @@ -19,8 +19,8 @@ type SharedConfig struct { EntSharedConfig - Listeners []*Listener `hcl:"-"` - UserLockoutConfigs []*UserLockoutConfig `hcl:"-"` + Listeners []*Listener `hcl:"-"` + UserLockouts []*UserLockout `hcl:"-"` Seals []*KMS `hcl:"-"` Entropy *Entropy `hcl:"-"` @@ -203,9 +203,9 @@ func (c *SharedConfig) Sanitized() map[string]interface{} { } // Sanitize user lockout stanza - if len(c.UserLockoutConfigs) != 0 { + if len(c.UserLockouts) != 0 { var sanitizedUserLockouts []interface{} - for _, userlockout := range c.UserLockoutConfigs { + for _, userlockout := range c.UserLockouts { cleanUserLockout := map[string]interface{}{ "type": userlockout.Type, "lockout_threshold": userlockout.LockoutThreshold, diff --git a/internalshared/configutil/merge.go b/internalshared/configutil/merge.go index 17e20d26e70b..fda6238e2493 100644 --- a/internalshared/configutil/merge.go +++ b/internalshared/configutil/merge.go @@ -14,11 +14,11 @@ func (c *SharedConfig) Merge(c2 *SharedConfig) *SharedConfig { result.Listeners = append(result.Listeners, l) } - for _, userlockout := range c.UserLockoutConfigs { - result.UserLockoutConfigs = append(result.UserLockoutConfigs, userlockout) + for _, userlockout := range c.UserLockouts { + result.UserLockouts = append(result.UserLockouts, userlockout) } - for _, userlockout := range c2.UserLockoutConfigs { - result.UserLockoutConfigs = append(result.UserLockoutConfigs, userlockout) + for _, userlockout := range c2.UserLockouts { + result.UserLockouts = append(result.UserLockouts, userlockout) } result.HCPLinkConf = c.HCPLinkConf diff --git a/internalshared/configutil/userlockout.go b/internalshared/configutil/userlockout.go index 57cd66892c55..bb5b339a7c95 100644 --- a/internalshared/configutil/userlockout.go +++ b/internalshared/configutil/userlockout.go @@ -19,7 +19,7 @@ const ( DisableUserLockoutDefault = false ) -type UserLockoutConfig struct { +type UserLockout struct { Type string LockoutThreshold int64 `hcl:"-"` LockoutThresholdRaw interface{} `hcl:"lockout_threshold"` @@ -33,10 +33,10 @@ type UserLockoutConfig struct { func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { var err error - result.UserLockoutConfigs = make([]*UserLockoutConfig, 0, len(list.Items)) - userLockoutConfigsMap := make(map[string]*UserLockoutConfig) + result.UserLockouts = make([]*UserLockout, 0, len(list.Items)) + userLockoutsMap := make(map[string]*UserLockout) for i, item := range list.Items { - var userLockoutConfig UserLockoutConfig + var userLockoutConfig UserLockout if err := hcl.DecodeObject(&userLockoutConfig, item.Val); err != nil { return multierror.Prefix(err, fmt.Sprintf("userLockouts.%d:", i)) } @@ -93,25 +93,25 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { } } } - userLockoutConfigsMap[userLockoutConfig.Type] = &userLockoutConfig + userLockoutsMap[userLockoutConfig.Type] = &userLockoutConfig } - userLockoutConfigsMap = setMissingUserLockoutValuesInMap(userLockoutConfigsMap) - for _, userLockoutValues := range userLockoutConfigsMap { - result.UserLockoutConfigs = append(result.UserLockoutConfigs, userLockoutValues) + userLockoutsMap = setMissingUserLockoutValuesInMap(userLockoutsMap) + for _, userLockoutValues := range userLockoutsMap { + result.UserLockouts = append(result.UserLockouts, userLockoutValues) } return nil } // setDefaultUserLockoutValuesInMap sets user lockout default values for key "all" for user lockout fields thats are not configured -func setDefaultUserLockoutValuesInMap(userLockoutConfigsMap map[string]*UserLockoutConfig) map[string]*UserLockoutConfig { - if userLockoutAll, ok := userLockoutConfigsMap["all"]; !ok { - var tmpUserLockoutConfig UserLockoutConfig +func setDefaultUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) map[string]*UserLockout { + if userLockoutAll, ok := userLockoutsMap["all"]; !ok { + var tmpUserLockoutConfig UserLockout tmpUserLockoutConfig.LockoutThreshold = UserLockoutThresholdDefault tmpUserLockoutConfig.LockoutDuration = UserLockoutDurationDefault tmpUserLockoutConfig.LockoutCounterReset = UserLockoutCounterResetDefault tmpUserLockoutConfig.DisableLockout = DisableUserLockoutDefault - userLockoutConfigsMap["all"] = &tmpUserLockoutConfig + userLockoutsMap["all"] = &tmpUserLockoutConfig } else { if userLockoutAll.LockoutThresholdRaw == nil { @@ -126,27 +126,27 @@ func setDefaultUserLockoutValuesInMap(userLockoutConfigsMap map[string]*UserLock if userLockoutAll.DisableLockoutRaw == nil { userLockoutAll.DisableLockout = DisableUserLockoutDefault } - userLockoutConfigsMap[userLockoutAll.Type] = userLockoutAll + userLockoutsMap[userLockoutAll.Type] = userLockoutAll } - return userLockoutConfigsMap + return userLockoutsMap } // setDefaultUserLockoutValuesInMap sets missing user lockout fields for other auth types with default values (from key "all") -func setMissingUserLockoutValuesInMap(userLockoutConfigsMap map[string]*UserLockoutConfig) map[string]*UserLockoutConfig { - userLockoutConfigsMap = setDefaultUserLockoutValuesInMap(userLockoutConfigsMap) - for _, userLockoutAuth := range userLockoutConfigsMap { +func setMissingUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) map[string]*UserLockout { + userLockoutsMap = setDefaultUserLockoutValuesInMap(userLockoutsMap) + for _, userLockoutAuth := range userLockoutsMap { // set missing values if userLockoutAuth.LockoutThresholdRaw == nil { - userLockoutAuth.LockoutThreshold = userLockoutConfigsMap["all"].LockoutThreshold + userLockoutAuth.LockoutThreshold = userLockoutsMap["all"].LockoutThreshold } if userLockoutAuth.LockoutDurationRaw == nil { - userLockoutAuth.LockoutDuration = userLockoutConfigsMap["all"].LockoutDuration + userLockoutAuth.LockoutDuration = userLockoutsMap["all"].LockoutDuration } if userLockoutAuth.LockoutCounterResetRaw == nil { - userLockoutAuth.LockoutCounterReset = userLockoutConfigsMap["all"].LockoutCounterReset + userLockoutAuth.LockoutCounterReset = userLockoutsMap["all"].LockoutCounterReset } if userLockoutAuth.DisableLockoutRaw == nil { - userLockoutAuth.DisableLockout = userLockoutConfigsMap["all"].DisableLockout + userLockoutAuth.DisableLockout = userLockoutsMap["all"].DisableLockout } // set nil values to Raw fields @@ -163,8 +163,8 @@ func setMissingUserLockoutValuesInMap(userLockoutConfigsMap map[string]*UserLock userLockoutAuth.DisableLockoutRaw = nil } - userLockoutConfigsMap[userLockoutAuth.Type] = userLockoutAuth + userLockoutsMap[userLockoutAuth.Type] = userLockoutAuth } - return userLockoutConfigsMap + return userLockoutsMap } diff --git a/vault/request_handling.go b/vault/request_handling.go index 73782183d5af..9ecb401faed9 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -2003,7 +2003,7 @@ func (c *Core) GetUserLockoutFromConfig(logicalType string) UserLockoutConfig { if conf == nil { return UserLockoutConfig{} } - userlockouts := conf.(*server.Config).UserLockoutConfigs + userlockouts := conf.(*server.Config).UserLockouts if userlockouts == nil { return UserLockoutConfig{} @@ -2012,7 +2012,7 @@ func (c *Core) GetUserLockoutFromConfig(logicalType string) UserLockoutConfig { var commonUserLockoutConfig UserLockoutConfig var authUserLockoutConfig UserLockoutConfig for _, userLockoutConfig := range userlockouts { - if userLockoutConfig.Type == logicalType{ + if userLockoutConfig.Type == logicalType { authUserLockoutConfig.LockoutThreshold = userLockoutConfig.LockoutThreshold authUserLockoutConfig.LockoutDuration = userLockoutConfig.LockoutDuration authUserLockoutConfig.LockoutCounterReset = userLockoutConfig.LockoutCounterReset From 95bf91da7bd9195e6fea720f44e99041af5cd757 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Wed, 28 Sep 2022 10:38:51 -0700 Subject: [PATCH 09/24] fixing mount tune --- api/sys_mounts.go | 20 -------------------- command/auth_tune.go | 2 +- vault/logical_system_paths.go | 4 ++++ 3 files changed, 5 insertions(+), 21 deletions(-) diff --git a/api/sys_mounts.go b/api/sys_mounts.go index ad1ff54ed8dc..b80c6873e46b 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -186,26 +186,6 @@ func (c *Sys) RemountStatusWithContext(ctx context.Context, migrationID string) return &result, err } -func (c *Sys) TuneAuthMount(path string, config MountConfigInput) error { - return c.TuneAuthMountWithContext(context.Background(), path, config) -} - -func (c *Sys) TuneAuthMountWithContext(ctx context.Context, path string, config MountConfigInput) error { - ctx, cancelFunc := c.c.withConfiguredTimeout(ctx) - defer cancelFunc() - - r := c.c.NewRequest(http.MethodPost, fmt.Sprintf("/v1/sys/%s/tune", path)) - if err := r.SetJSONBody(config); err != nil { - return err - } - - resp, err := c.c.rawRequestWithContext(ctx, r) - if err == nil { - defer resp.Body.Close() - } - return err -} - func (c *Sys) TuneMount(path string, config MountConfigInput) error { return c.TuneMountWithContext(context.Background(), path, config) } diff --git a/command/auth_tune.go b/command/auth_tune.go index 3a4bd6a394c3..2b0e72b9c72a 100644 --- a/command/auth_tune.go +++ b/command/auth_tune.go @@ -293,7 +293,7 @@ func (c *AuthTuneCommand) Run(args []string) int { // indicate it's a path in output mountPath := ensureTrailingSlash(sanitizePath(args[0])) - if err := client.Sys().TuneAuthMount("/auth/"+mountPath, mountConfigInput); err != nil { + if err := client.Sys().TuneMount("/auth/"+mountPath, mountConfigInput); err != nil { c.UI.Error(fmt.Sprintf("Error tuning auth method %s: %s", mountPath, err)) return 2 } diff --git a/vault/logical_system_paths.go b/vault/logical_system_paths.go index e93dfd8c6576..f1ca2e7bfd82 100644 --- a/vault/logical_system_paths.go +++ b/vault/logical_system_paths.go @@ -1933,6 +1933,10 @@ func (b *SystemBackend) mountPaths() []*framework.Path { Type: framework.TypeString, Description: strings.TrimSpace(sysHelp["plugin-catalog_version"][0]), }, + "user_lockout_config": { + Type: framework.TypeMap, + Description: strings.TrimSpace(sysHelp["user_lockout_config"][0]), + }, }, Callbacks: map[logical.Operation]framework.OperationFunc{ From 7ee7a1e756887108b685369afe529bb9fdb0e9f2 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Wed, 28 Sep 2022 12:56:02 -0700 Subject: [PATCH 10/24] adding test file for user lockout --- internalshared/configutil/userlockout.go | 1 + internalshared/configutil/userlockout_test.go | 69 +++++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 internalshared/configutil/userlockout_test.go diff --git a/internalshared/configutil/userlockout.go b/internalshared/configutil/userlockout.go index bb5b339a7c95..eaf90fd281a8 100644 --- a/internalshared/configutil/userlockout.go +++ b/internalshared/configutil/userlockout.go @@ -107,6 +107,7 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { func setDefaultUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) map[string]*UserLockout { if userLockoutAll, ok := userLockoutsMap["all"]; !ok { var tmpUserLockoutConfig UserLockout + tmpUserLockoutConfig.Type = "all" tmpUserLockoutConfig.LockoutThreshold = UserLockoutThresholdDefault tmpUserLockoutConfig.LockoutDuration = UserLockoutDurationDefault tmpUserLockoutConfig.LockoutCounterReset = UserLockoutCounterResetDefault diff --git a/internalshared/configutil/userlockout_test.go b/internalshared/configutil/userlockout_test.go new file mode 100644 index 000000000000..d5ab42cbe86a --- /dev/null +++ b/internalshared/configutil/userlockout_test.go @@ -0,0 +1,69 @@ +package configutil + +import ( + "reflect" + "testing" + "time" +) + +func TestParseUserLockout(t *testing.T) { + t.Parallel() + t.Run("Missing user lockout block in config file", func(t *testing.T) { + t.Parallel() + inputConfig := make(map[string]*UserLockout) + expectedConfig := make(map[string]*UserLockout) + expectedConfigall := &UserLockout{} + expectedConfigall.Type = "all" + expectedConfigall.LockoutThreshold = UserLockoutThresholdDefault + expectedConfigall.LockoutDuration = UserLockoutDurationDefault + expectedConfigall.LockoutCounterReset = UserLockoutCounterResetDefault + expectedConfigall.DisableLockout = DisableUserLockoutDefault + expectedConfig["all"] = expectedConfigall + + outputConfig := setMissingUserLockoutValuesInMap(inputConfig) + if !reflect.DeepEqual(expectedConfig["all"], outputConfig["all"]) { + t.Errorf("user lockout config: expected %#v\nactual %#v", expectedConfig["all"], outputConfig["all"]) + } + }) + t.Run("setting default lockout counter reset and lockout duration for userpass in config ", func(t *testing.T) { + t.Parallel() + // input user lockout in config file + inputConfig := make(map[string]*UserLockout) + configAll := &UserLockout{} + configAll.Type = "all" + configAll.LockoutCounterReset = 20 * time.Minute + configAll.LockoutCounterResetRaw = "1200000000000" + inputConfig["all"] = configAll + configUserpass := &UserLockout{} + configUserpass.Type = "userpass" + configUserpass.LockoutDuration = 10 * time.Minute + configUserpass.LockoutDurationRaw = "600000000000" + inputConfig["userpass"] = configUserpass + + expectedConfig := make(map[string]*UserLockout) + expectedConfigall := &UserLockout{} + expectedConfigUserpass := &UserLockout{} + // expected default values + expectedConfigall.Type = "all" + expectedConfigall.LockoutThreshold = UserLockoutThresholdDefault + expectedConfigall.LockoutDuration = UserLockoutDurationDefault + expectedConfigall.LockoutCounterReset = 20 * time.Minute + expectedConfigall.DisableLockout = DisableUserLockoutDefault + // expected values for userpass + expectedConfigUserpass.Type = "userpass" + expectedConfigUserpass.LockoutThreshold = UserLockoutThresholdDefault + expectedConfigUserpass.LockoutDuration = 10 * time.Minute + expectedConfigUserpass.LockoutCounterReset = 20 * time.Minute + expectedConfigUserpass.DisableLockout = DisableUserLockoutDefault + expectedConfig["all"] = expectedConfigall + expectedConfig["userpass"] = expectedConfigUserpass + + outputConfig := setMissingUserLockoutValuesInMap(inputConfig) + if !reflect.DeepEqual(expectedConfig["all"], outputConfig["all"]) { + t.Errorf("user lockout config: expected %#v\nactual %#v", expectedConfig["all"], outputConfig["all"]) + } + if !reflect.DeepEqual(expectedConfig["userpass"], outputConfig["userpass"]) { + t.Errorf("user lockout config: expected %#v\nactual %#v", expectedConfig["userpass"], outputConfig["userpass"]) + } + }) +} From d8be9b5bd817619cb89775c2763596a3f80321dd Mon Sep 17 00:00:00 2001 From: akshya96 Date: Wed, 28 Sep 2022 16:16:36 -0700 Subject: [PATCH 11/24] fixing comments and add changelog --- api/sys_mounts.go | 18 ++++++++---------- changelog/17338.txt | 3 +++ internalshared/configutil/userlockout.go | 4 ++-- vault/logical_system.go | 14 +++++++------- vault/mount.go | 18 ++++++++---------- vault/request_handling.go | 4 ---- 6 files changed, 28 insertions(+), 33 deletions(-) create mode 100644 changelog/17338.txt diff --git a/api/sys_mounts.go b/api/sys_mounts.go index b80c6873e46b..999381fac225 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -305,19 +305,17 @@ type MountConfigOutput struct { } type UserLockoutConfigInput struct { - LockoutThreshold string `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` // Override for global default - LockoutDuration string `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` // Override for global default - LockoutCounterResetDuration string `json:"lockout_counter_reset_duration,omitempty" structs:"lockout_counter_reset_duration" mapstructure:"lockout_counter_reset_duration"` // Override for global default - DisableLockout *bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` // Override for global default - + LockoutThreshold string `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` + LockoutDuration string `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` + LockoutCounterResetDuration string `json:"lockout_counter_reset_duration,omitempty" structs:"lockout_counter_reset_duration" mapstructure:"lockout_counter_reset_duration"` + DisableLockout *bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` } type UserLockoutConfigOutput struct { - LockoutThreshold int `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` // Override for global default - LockoutDuration int `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` // Override for global default - LockoutCounterReset int `json:"lockout_counter_reset,omitempty" structs:"lockout_counter_reset" mapstructure:"lockout_counter_reset"` // Override for global default - DisableLockout bool `json:"disable_lockout,omitempty" structs:"disable_lockout" mapstructure:"disable_lockout"` // Override for global default - + LockoutThreshold int `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` + LockoutDuration int `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` + LockoutCounterReset int `json:"lockout_counter_reset,omitempty" structs:"lockout_counter_reset" mapstructure:"lockout_counter_reset"` + DisableLockout bool `json:"disable_lockout,omitempty" structs:"disable_lockout" mapstructure:"disable_lockout"` } type MountMigrationOutput struct { diff --git a/changelog/17338.txt b/changelog/17338.txt new file mode 100644 index 000000000000..e16448a3d4b0 --- /dev/null +++ b/changelog/17338.txt @@ -0,0 +1,3 @@ +```release-note:feature +core: Add user lockout field to config and configuring this for auth mount using auth tune +``` \ No newline at end of file diff --git a/internalshared/configutil/userlockout.go b/internalshared/configutil/userlockout.go index eaf90fd281a8..a7ff0ab8967f 100644 --- a/internalshared/configutil/userlockout.go +++ b/internalshared/configutil/userlockout.go @@ -103,7 +103,7 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { return nil } -// setDefaultUserLockoutValuesInMap sets user lockout default values for key "all" for user lockout fields thats are not configured +// setDefaultUserLockoutValuesInMap sets default user lockout values for key "all" (all auth methods) for user lockout fields that are not configured using config file func setDefaultUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) map[string]*UserLockout { if userLockoutAll, ok := userLockoutsMap["all"]; !ok { var tmpUserLockoutConfig UserLockout @@ -132,7 +132,7 @@ func setDefaultUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) m return userLockoutsMap } -// setDefaultUserLockoutValuesInMap sets missing user lockout fields for other auth types with default values (from key "all") +// setDefaultUserLockoutValuesInMap sets missing user lockout fields for auth methods with default values (from key "all") that are not configured using config file func setMissingUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) map[string]*UserLockout { userLockoutsMap = setDefaultUserLockoutValuesInMap(userLockoutsMap) for _, userLockoutAuth := range userLockoutsMap { diff --git a/vault/logical_system.go b/vault/logical_system.go index bd682c56aece..8ac23847a66c 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -1743,21 +1743,21 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, err = expandStringValsWithCommas(userLockoutConfigMap) if err != nil { return logical.ErrorResponse( - "unable to parse given auth config information"), + "unable to parse given auth user lockout config information"), logical.ErrInvalidRequest } if userLockoutConfigMap != nil && len(userLockoutConfigMap) != 0 { err := mapstructure.Decode(userLockoutConfigMap, &apiuserLockoutConfig) if err != nil { return logical.ErrorResponse( - "unable to convert given mount config information"), + "unable to convert given user lockout config information"), logical.ErrInvalidRequest } switch strings.ToLower(mountEntry.Type) { case "all", "ldap", "approle", "userpass": default: - return logical.ErrorResponse("mount tuning of user lockout configuration for auth type %q not allowed", mountEntry.Type), + return logical.ErrorResponse("tuning of user lockout configuration for auth type %q not allowed", mountEntry.Type), logical.ErrInvalidRequest } @@ -1790,7 +1790,7 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, return handleError(err) } if b.Core.logger.IsInfo() { - b.Core.logger.Info("mount tuning of user-lockout-threshold successful", "path", path, "user-lockout-threshold", userLockoutThreshold) + b.Core.logger.Info("tuning of user-lockout-threshold successful", "path", path, "user-lockout-threshold", userLockoutThreshold) } } @@ -1826,7 +1826,7 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, return handleError(err) } if b.Core.logger.IsInfo() { - b.Core.logger.Info("mount tuning of user-lockout-duration successful", "path", path, "user-lockout-duration", apiuserLockoutConfig.LockoutDuration) + b.Core.logger.Info("tuning of user-lockout-duration successful", "path", path, "user-lockout-duration", apiuserLockoutConfig.LockoutDuration) } } @@ -1859,7 +1859,7 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, return handleError(err) } if b.Core.logger.IsInfo() { - b.Core.logger.Info("mount tuning of user-lockout-counter-reset successful", "path", path, "user-lockout-counter-reset", apiuserLockoutConfig.LockoutCounterResetDuration) + b.Core.logger.Info("tuning of user-lockout-counter-reset successful", "path", path, "user-lockout-counter-reset", apiuserLockoutConfig.LockoutCounterResetDuration) } } { @@ -1879,7 +1879,7 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, return handleError(err) } if b.Core.logger.IsInfo() { - b.Core.logger.Info("mount tuning of user-lockout-disable successful", "path", path, "user-lockout-disable", userLockoutDisable) + b.Core.logger.Info("tuning of user-lockout-disable successful", "path", path, "user-lockout-disable", userLockoutDisable) } } diff --git a/vault/mount.go b/vault/mount.go index 106bd8ed32e1..8342796ba8f4 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -358,19 +358,17 @@ type MountConfig struct { } type UserLockoutConfig struct { - LockoutThreshold int64 `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` // Override for global default - LockoutDuration time.Duration `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` // Override for global default - LockoutCounterReset time.Duration `json:"lockout_counter_reset,omitempty" structs:"lockout_counter_reset" mapstructure:"lockout_counter_reset"` // Override for global default - DisableLockout bool `json:"disable_lockout,omitempty" structs:"disable_lockout" mapstructure:"disable_lockout"` // Override for global default - + LockoutThreshold int64 `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` + LockoutDuration time.Duration `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` + LockoutCounterReset time.Duration `json:"lockout_counter_reset,omitempty" structs:"lockout_counter_reset" mapstructure:"lockout_counter_reset"` + DisableLockout bool `json:"disable_lockout,omitempty" structs:"disable_lockout" mapstructure:"disable_lockout"` } type APIUserLockoutConfig struct { - LockoutThreshold string `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` // Override for global default - LockoutDuration string `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` // Override for global default - LockoutCounterResetDuration string `json:"lockout_counter_reset_duration,omitempty" structs:"lockout_counter_reset_duration" mapstructure:"lockout_counter_reset_duration"` // Override for global default - DisableLockout bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` // Override for global default - + LockoutThreshold string `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` + LockoutDuration string `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` + LockoutCounterResetDuration string `json:"lockout_counter_reset_duration,omitempty" structs:"lockout_counter_reset_duration" mapstructure:"lockout_counter_reset_duration"` + DisableLockout bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` } // APIMountConfig is an embedded struct of api.MountConfigInput diff --git a/vault/request_handling.go b/vault/request_handling.go index 9ecb401faed9..72fd5d481788 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -251,10 +251,6 @@ func (c *Core) fetchACLTokenEntryAndEntity(ctx context.Context, req *logical.Req return acl, te, entity, identityPolicies, nil } -func (c *Core) isUserLockoutDisabled(authMethodName string) bool { - return false -} - func (c *Core) checkToken(ctx context.Context, req *logical.Request, unauth bool) (*logical.Auth, *logical.TokenEntry, error) { defer metrics.MeasureSince([]string{"core", "check_token"}, time.Now()) From 0616553764c6bec3b77e60516398cf6acd46e0d3 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Mon, 3 Oct 2022 21:42:43 -0700 Subject: [PATCH 12/24] addressing comments --- changelog/17338.txt | 2 +- command/auth_tune.go | 8 +-- command/commands.go | 8 +++ internalshared/configutil/userlockout.go | 8 ++- vault/logical_system.go | 84 +++++++++++++----------- vault/logical_system_paths.go | 4 +- vault/mount.go | 2 +- 7 files changed, 69 insertions(+), 47 deletions(-) diff --git a/changelog/17338.txt b/changelog/17338.txt index e16448a3d4b0..78ebec874619 100644 --- a/changelog/17338.txt +++ b/changelog/17338.txt @@ -1,3 +1,3 @@ ```release-note:feature -core: Add user lockout field to config and configuring this for auth mount using auth tune +core: Add user lockout field to config and configuring this for auth mount using auth tune to prevent brute forcing in auth methods ``` \ No newline at end of file diff --git a/command/auth_tune.go b/command/auth_tune.go index 2b0e72b9c72a..09550daa9c3c 100644 --- a/command/auth_tune.go +++ b/command/auth_tune.go @@ -150,7 +150,7 @@ func (c *AuthTuneCommand) Flags() *FlagSets { }) f.IntVar(&IntVar{ - Name: "user-lockout-threshold", + Name: flagNameUserLockoutThreshold, Target: &c.flagUserLockoutThreshold, Usage: "The threshold for user lockout for this auth method. If unspecified, this " + "defaults to the Vault server's globally configured user lockout threshold, " + @@ -158,7 +158,7 @@ func (c *AuthTuneCommand) Flags() *FlagSets { }) f.DurationVar(&DurationVar{ - Name: "user-lockout-duration", + Name: flagNameUserLockoutDuration, Target: &c.flagUserLockoutDuration, Completion: complete.PredictAnything, Usage: "The user lockout duration for this auth method. If unspecified, this " + @@ -167,7 +167,7 @@ func (c *AuthTuneCommand) Flags() *FlagSets { }) f.DurationVar(&DurationVar{ - Name: "user-lockout-counter-reset-duration", + Name: flagNameUserLockoutCounterResetDuration, Target: &c.flagUserLockoutCounterResetDuration, Completion: complete.PredictAnything, Usage: "The user lockout counter reset duration for this auth method. If unspecified, this " + @@ -176,7 +176,7 @@ func (c *AuthTuneCommand) Flags() *FlagSets { }) f.BoolVar(&BoolVar{ - Name: "user-lockout-disable", + Name: flagNameUserLockoutDisable, Target: &c.flagUserLockoutDisable, Default: false, Usage: "Disable user lockout for this auth method. If unspecified, this " + diff --git a/command/commands.go b/command/commands.go index d4ce6b6ca38b..5ca18877866f 100644 --- a/command/commands.go +++ b/command/commands.go @@ -126,6 +126,14 @@ const ( flagNameAllowedManagedKeys = "allowed-managed-keys" // flagNamePluginVersion selects what version of a plugin should be used. flagNamePluginVersion = "plugin-version" + // flagNameUserLockoutThreshold is the flag name used for tuning the auth mount lockout threshold parameter + flagNameUserLockoutThreshold = "user-lockout-threshold" + // flagNameUserLockoutDuration is the flag name used for tuning the auth mount lockout duration parameter + flagNameUserLockoutDuration = "user-lockout-duration" + // flagNameUserLockoutCounterResetDuration is the flag name used for tuning the auth mount lockout counter reset parameter + flagNameUserLockoutCounterResetDuration = "user-lockout-counter-reset-duration" + // flagNameUserLockoutDisable is the flag name used for tuning the auth mount disable lockout parameter + flagNameUserLockoutDisable = "user-lockout-disable" ) var ( diff --git a/internalshared/configutil/userlockout.go b/internalshared/configutil/userlockout.go index a7ff0ab8967f..2b4faca584a8 100644 --- a/internalshared/configutil/userlockout.go +++ b/internalshared/configutil/userlockout.go @@ -52,6 +52,8 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { } userLockoutConfig.Type = strings.ToLower(userLockoutConfig.Type) + // Supported auth methods for user lockout configuration: ldap, approle, userpass + // "all" is used to apply the configuration to all supported auth methods switch userLockoutConfig.Type { case "all", "ldap", "approle", "userpass": result.found(userLockoutConfig.Type, userLockoutConfig.Type) @@ -103,7 +105,8 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { return nil } -// setDefaultUserLockoutValuesInMap sets default user lockout values for key "all" (all auth methods) for user lockout fields that are not configured using config file +// setDefaultUserLockoutValuesInMap sets default user lockout values for key "all" (all auth methods) +// for user lockout fields that are not configured using config file func setDefaultUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) map[string]*UserLockout { if userLockoutAll, ok := userLockoutsMap["all"]; !ok { var tmpUserLockoutConfig UserLockout @@ -132,7 +135,8 @@ func setDefaultUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) m return userLockoutsMap } -// setDefaultUserLockoutValuesInMap sets missing user lockout fields for auth methods with default values (from key "all") that are not configured using config file +// setDefaultUserLockoutValuesInMap sets missing user lockout fields for auth methods +// with default values (from key "all") that are not configured using config file func setMissingUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) map[string]*UserLockout { userLockoutsMap = setDefaultUserLockoutValuesInMap(userLockoutsMap) for _, userLockoutAuth := range userLockoutsMap { diff --git a/vault/logical_system.go b/vault/logical_system.go index 8ac23847a66c..745b0146b3e5 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -938,7 +938,7 @@ func (b *SystemBackend) mountInfo(ctx context.Context, entry *MountEntry) map[st if entry.Table == credentialTableType { entryConfig["token_type"] = entry.Config.TokenType.String() } - if (entry.Config.UserLockoutConfig != UserLockoutConfig{}) { + if entry.Config.UserLockoutConfig != nil { userLockoutConfig := map[string]interface{}{ "user_lockout_counter_reset_duration": int64(entry.Config.UserLockoutConfig.LockoutCounterReset.Seconds()), "user_lockout_threshold": entry.Config.UserLockoutConfig.LockoutThreshold, @@ -1607,7 +1607,7 @@ func (b *SystemBackend) handleTuneReadCommon(ctx context.Context, path string) ( resp.Data["allowed_managed_keys"] = rawVal.([]string) } - if (mountEntry.Config.UserLockoutConfig != UserLockoutConfig{}) { + if mountEntry.Config.UserLockoutConfig != nil { resp.Data["user_lockout_counter_reset_duration"] = int64(mountEntry.Config.UserLockoutConfig.LockoutCounterReset.Seconds()) resp.Data["user_lockout_threshold"] = mountEntry.Config.UserLockoutConfig.LockoutThreshold resp.Data["user_lockout_duration"] = int64(mountEntry.Config.UserLockoutConfig.LockoutDuration.Seconds()) @@ -1740,12 +1740,6 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, userLockoutConfigMap := data.Get("user_lockout_config").(map[string]interface{}) var err error // Augmenting userLockoutConfigMap for some config options to treat them as comma separated entries - err = expandStringValsWithCommas(userLockoutConfigMap) - if err != nil { - return logical.ErrorResponse( - "unable to parse given auth user lockout config information"), - logical.ErrInvalidRequest - } if userLockoutConfigMap != nil && len(userLockoutConfigMap) != 0 { err := mapstructure.Decode(userLockoutConfigMap, &apiuserLockoutConfig) if err != nil { @@ -1754,6 +1748,8 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, logical.ErrInvalidRequest } + // Supported auth methods for user lockout configuration: ldap, approle, userpass + // "all" is used to apply the configuration to all supported auth methods switch strings.ToLower(mountEntry.Type) { case "all", "ldap", "approle", "userpass": default: @@ -1763,21 +1759,25 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, } } + if mountEntry.Config.UserLockoutConfig == nil { + mountEntry.Config.UserLockoutConfig = &UserLockoutConfig{} + } + if _, ok := userLockoutConfigMap["lockout_threshold"]; ok { - var newUserLockoutThreshold int64 userLockoutThreshold, err := parseutil.ParseInt(apiuserLockoutConfig.LockoutThreshold) if err != nil { return nil, fmt.Errorf("unable to parse user lockout threshold: %w", err) } + oldUserLockoutThreshold := mountEntry.Config.UserLockoutConfig.LockoutThreshold switch { case userLockoutThreshold < 0: - newUserLockoutThreshold = oldUserLockoutThreshold + mountEntry.Config.UserLockoutConfig.LockoutThreshold = oldUserLockoutThreshold default: - newUserLockoutThreshold = userLockoutThreshold + mountEntry.Config.UserLockoutConfig.LockoutThreshold = userLockoutThreshold } - mountEntry.Config.UserLockoutConfig.LockoutThreshold = newUserLockoutThreshold + // Update the mount table switch { case strings.HasPrefix(path, "auth/"): @@ -1795,9 +1795,11 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, } - { - var newUserLockoutDuration time.Duration - oldUserLockoutDuration := mountEntry.Config.UserLockoutConfig.LockoutDuration + if _, ok := userLockoutConfigMap["lockout_duration"]; ok { + var newUserLockoutDuration, oldUserLockoutDuration time.Duration + + oldUserLockoutDuration = mountEntry.Config.UserLockoutConfig.LockoutDuration + switch apiuserLockoutConfig.LockoutDuration { case "": @@ -1830,9 +1832,12 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, } } - { - var newUserLockoutCounterReset time.Duration - oldUserLockoutCounterReset := mountEntry.Config.UserLockoutConfig.LockoutCounterReset + + if _, ok := userLockoutConfigMap["lockout_counter_reset_duration"]; ok { + var newUserLockoutCounterReset, oldUserLockoutCounterReset time.Duration + + oldUserLockoutCounterReset = mountEntry.Config.UserLockoutConfig.LockoutCounterReset + switch apiuserLockoutConfig.LockoutCounterResetDuration { case "": newUserLockoutCounterReset = oldUserLockoutCounterReset @@ -1862,27 +1867,28 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, b.Core.logger.Info("tuning of user-lockout-counter-reset successful", "path", path, "user-lockout-counter-reset", apiuserLockoutConfig.LockoutCounterResetDuration) } } - { - if rawVal, ok := userLockoutConfigMap["lockout_disable"]; ok { - userLockoutDisable := rawVal.(bool) - oldUserLockoutDisable := mountEntry.Config.UserLockoutConfig.DisableLockout - mountEntry.Config.UserLockoutConfig.DisableLockout = userLockoutDisable - var err error - switch { - case strings.HasPrefix(path, "auth/"): - err = b.Core.persistAuth(ctx, b.Core.auth, &mountEntry.Local) - default: - err = b.Core.persistMounts(ctx, b.Core.mounts, &mountEntry.Local) - } - if err != nil { - mountEntry.Config.UserLockoutConfig.DisableLockout = oldUserLockoutDisable - return handleError(err) - } - if b.Core.logger.IsInfo() { - b.Core.logger.Info("tuning of user-lockout-disable successful", "path", path, "user-lockout-disable", userLockoutDisable) - } + if rawVal, ok := userLockoutConfigMap["lockout_disable"]; ok { + userLockoutDisable := rawVal.(bool) + var oldUserLockoutDisable bool + + oldUserLockoutDisable = mountEntry.Config.UserLockoutConfig.DisableLockout + mountEntry.Config.UserLockoutConfig.DisableLockout = userLockoutDisable + var err error + switch { + case strings.HasPrefix(path, "auth/"): + err = b.Core.persistAuth(ctx, b.Core.auth, &mountEntry.Local) + default: + err = b.Core.persistMounts(ctx, b.Core.mounts, &mountEntry.Local) } + if err != nil { + mountEntry.Config.UserLockoutConfig.DisableLockout = oldUserLockoutDisable + return handleError(err) + } + if b.Core.logger.IsInfo() { + b.Core.logger.Info("tuning of user-lockout-disable successful", "path", path, "user-lockout-disable", userLockoutDisable) + } + } } @@ -5216,6 +5222,10 @@ in the plugin catalog.`, `The options to pass into the backend. Should be a json object with string keys and values.`, }, + "tune_user_lockout_config": { + `The user lockout configuration to pass into the backend. Should be a json object with string keys and values.`, + }, + "remount": { "Move the mount point of an already-mounted backend, within or across namespaces", ` diff --git a/vault/logical_system_paths.go b/vault/logical_system_paths.go index f1ca2e7bfd82..be774673f989 100644 --- a/vault/logical_system_paths.go +++ b/vault/logical_system_paths.go @@ -1544,7 +1544,7 @@ func (b *SystemBackend) authPaths() []*framework.Path { }, "user_lockout_config": { Type: framework.TypeMap, - Description: strings.TrimSpace(sysHelp["user_lockout_config"][0]), + Description: strings.TrimSpace(sysHelp["tune_user_lockout_config"][0]), }, "plugin_version": { Type: framework.TypeString, @@ -1935,7 +1935,7 @@ func (b *SystemBackend) mountPaths() []*framework.Path { }, "user_lockout_config": { Type: framework.TypeMap, - Description: strings.TrimSpace(sysHelp["user_lockout_config"][0]), + Description: strings.TrimSpace(sysHelp["tune_user_lockout_config"][0]), }, }, diff --git a/vault/mount.go b/vault/mount.go index 8342796ba8f4..7939a4a81e9d 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -349,7 +349,7 @@ type MountConfig struct { AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" structs:"allowed_response_headers" mapstructure:"allowed_response_headers"` TokenType logical.TokenType `json:"token_type,omitempty" structs:"token_type" mapstructure:"token_type"` AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` - UserLockoutConfig UserLockoutConfig `json:"user_lockout_config,omitempty" mapstructure:"user_lockout_config"` + UserLockoutConfig *UserLockoutConfig `json:"user_lockout_config,omitempty" mapstructure:"user_lockout_config"` // PluginName is the name of the plugin registered in the catalog. // From 3b9f1dfb3b5e8713afc92fe385282f42f8367514 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Mon, 3 Oct 2022 22:17:50 -0700 Subject: [PATCH 13/24] fixing mount table updates --- vault/logical_system.go | 77 ++++++++--------------------------------- 1 file changed, 15 insertions(+), 62 deletions(-) diff --git a/vault/logical_system.go b/vault/logical_system.go index 745b0146b3e5..a09028d533c8 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -1759,49 +1759,32 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, } } - if mountEntry.Config.UserLockoutConfig == nil { + if len(userLockoutConfigMap) > 0 && mountEntry.Config.UserLockoutConfig == nil { mountEntry.Config.UserLockoutConfig = &UserLockoutConfig{} } + var oldUserLockoutThreshold int64 + var newUserLockoutDuration, oldUserLockoutDuration time.Duration + var newUserLockoutCounterReset, oldUserLockoutCounterReset time.Duration + var oldUserLockoutDisable bool + if _, ok := userLockoutConfigMap["lockout_threshold"]; ok { userLockoutThreshold, err := parseutil.ParseInt(apiuserLockoutConfig.LockoutThreshold) if err != nil { return nil, fmt.Errorf("unable to parse user lockout threshold: %w", err) } - - oldUserLockoutThreshold := mountEntry.Config.UserLockoutConfig.LockoutThreshold - + oldUserLockoutThreshold = mountEntry.Config.UserLockoutConfig.LockoutThreshold switch { case userLockoutThreshold < 0: mountEntry.Config.UserLockoutConfig.LockoutThreshold = oldUserLockoutThreshold default: mountEntry.Config.UserLockoutConfig.LockoutThreshold = userLockoutThreshold } - - // Update the mount table - switch { - case strings.HasPrefix(path, "auth/"): - err = b.Core.persistAuth(ctx, b.Core.auth, &mountEntry.Local) - default: - err = b.Core.persistMounts(ctx, b.Core.mounts, &mountEntry.Local) - } - if err != nil { - mountEntry.Config.UserLockoutConfig.LockoutThreshold = oldUserLockoutThreshold - return handleError(err) - } - if b.Core.logger.IsInfo() { - b.Core.logger.Info("tuning of user-lockout-threshold successful", "path", path, "user-lockout-threshold", userLockoutThreshold) - } - } if _, ok := userLockoutConfigMap["lockout_duration"]; ok { - var newUserLockoutDuration, oldUserLockoutDuration time.Duration - oldUserLockoutDuration = mountEntry.Config.UserLockoutConfig.LockoutDuration - switch apiuserLockoutConfig.LockoutDuration { - case "": newUserLockoutDuration = oldUserLockoutDuration case "system": @@ -1815,29 +1798,10 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, } mountEntry.Config.UserLockoutConfig.LockoutDuration = newUserLockoutDuration - // Update the mount table - - switch { - case strings.HasPrefix(path, "auth/"): - err = b.Core.persistAuth(ctx, b.Core.auth, &mountEntry.Local) - default: - err = b.Core.persistMounts(ctx, b.Core.mounts, &mountEntry.Local) - } - if err != nil { - mountEntry.Config.UserLockoutConfig.LockoutDuration = oldUserLockoutDuration - return handleError(err) - } - if b.Core.logger.IsInfo() { - b.Core.logger.Info("tuning of user-lockout-duration successful", "path", path, "user-lockout-duration", apiuserLockoutConfig.LockoutDuration) - } - } if _, ok := userLockoutConfigMap["lockout_counter_reset_duration"]; ok { - var newUserLockoutCounterReset, oldUserLockoutCounterReset time.Duration - oldUserLockoutCounterReset = mountEntry.Config.UserLockoutConfig.LockoutCounterReset - switch apiuserLockoutConfig.LockoutCounterResetDuration { case "": newUserLockoutCounterReset = oldUserLockoutCounterReset @@ -1852,29 +1816,16 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, } mountEntry.Config.UserLockoutConfig.LockoutCounterReset = newUserLockoutCounterReset - // Update the mount table - switch { - case strings.HasPrefix(path, "auth/"): - err = b.Core.persistAuth(ctx, b.Core.auth, &mountEntry.Local) - default: - err = b.Core.persistMounts(ctx, b.Core.mounts, &mountEntry.Local) - } - if err != nil { - mountEntry.Config.UserLockoutConfig.LockoutCounterReset = oldUserLockoutCounterReset - return handleError(err) - } - if b.Core.logger.IsInfo() { - b.Core.logger.Info("tuning of user-lockout-counter-reset successful", "path", path, "user-lockout-counter-reset", apiuserLockoutConfig.LockoutCounterResetDuration) - } } if rawVal, ok := userLockoutConfigMap["lockout_disable"]; ok { userLockoutDisable := rawVal.(bool) - var oldUserLockoutDisable bool - oldUserLockoutDisable = mountEntry.Config.UserLockoutConfig.DisableLockout mountEntry.Config.UserLockoutConfig.DisableLockout = userLockoutDisable - var err error + } + + // Update the mount table + if len(userLockoutConfigMap) > 0 { switch { case strings.HasPrefix(path, "auth/"): err = b.Core.persistAuth(ctx, b.Core.auth, &mountEntry.Local) @@ -1882,13 +1833,15 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, err = b.Core.persistMounts(ctx, b.Core.mounts, &mountEntry.Local) } if err != nil { + mountEntry.Config.UserLockoutConfig.LockoutCounterReset = oldUserLockoutCounterReset + mountEntry.Config.UserLockoutConfig.LockoutThreshold = oldUserLockoutThreshold + mountEntry.Config.UserLockoutConfig.LockoutDuration = oldUserLockoutDuration mountEntry.Config.UserLockoutConfig.DisableLockout = oldUserLockoutDisable return handleError(err) } if b.Core.logger.IsInfo() { - b.Core.logger.Info("tuning of user-lockout-disable successful", "path", path, "user-lockout-disable", userLockoutDisable) + b.Core.logger.Info("tuning of user_lockout_config successful", "path", path) } - } } From 817e4ff74ae8000ec31962e3f38be95590af274b Mon Sep 17 00:00:00 2001 From: akshya96 Date: Mon, 3 Oct 2022 22:25:33 -0700 Subject: [PATCH 14/24] updating consts in auth_tune --- command/auth_tune.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/command/auth_tune.go b/command/auth_tune.go index 09550daa9c3c..a151bf3a1621 100644 --- a/command/auth_tune.go +++ b/command/auth_tune.go @@ -269,18 +269,18 @@ func (c *AuthTuneCommand) Run(args []string) int { if fl.Name == flagNameTokenType { mountConfigInput.TokenType = c.flagTokenType } - if fl.Name == "user-lockout-threshold" { + if fl.Name == flagNameUserLockoutThreshold { if c.flagUserLockoutThreshold > 0 { mountConfigInput.UserLockoutConfig.LockoutThreshold = strconv.Itoa(c.flagUserLockoutThreshold) } } - if fl.Name == "user-lockout-duration" { + if fl.Name == flagNameUserLockoutDuration { mountConfigInput.UserLockoutConfig.LockoutDuration = ttlToAPI(c.flagUserLockoutDuration) } - if fl.Name == "user-lockout-counter-reset-duration" { + if fl.Name == flagNameUserLockoutCounterResetDuration { mountConfigInput.UserLockoutConfig.LockoutCounterResetDuration = ttlToAPI(c.flagUserLockoutCounterResetDuration) } - if fl.Name == "user-lockout-disable" { + if fl.Name == flagNameUserLockoutDisable { mountConfigInput.UserLockoutConfig.DisableLockout = &c.flagUserLockoutDisable } From d3eca3b3f5d8b7f97ccaa123b4efeffd6a3d5797 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Mon, 3 Oct 2022 22:48:56 -0700 Subject: [PATCH 15/24] small fixes --- api/sys_mounts.go | 30 +++++++++++++++--------------- command/auth_tune.go | 8 +++++++- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/api/sys_mounts.go b/api/sys_mounts.go index 999381fac225..a33aff2bce6e 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -254,20 +254,20 @@ type MountInput struct { } type MountConfigInput struct { - Options map[string]string `json:"options" mapstructure:"options"` - DefaultLeaseTTL string `json:"default_lease_ttl" mapstructure:"default_lease_ttl"` - Description *string `json:"description,omitempty" mapstructure:"description"` - MaxLeaseTTL string `json:"max_lease_ttl" mapstructure:"max_lease_ttl"` - ForceNoCache bool `json:"force_no_cache" mapstructure:"force_no_cache"` - AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" mapstructure:"audit_non_hmac_request_keys"` - AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" mapstructure:"audit_non_hmac_response_keys"` - ListingVisibility string `json:"listing_visibility,omitempty" mapstructure:"listing_visibility"` - PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" mapstructure:"passthrough_request_headers"` - AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` - TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` - AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` - PluginVersion string `json:"plugin_version,omitempty"` - UserLockoutConfig UserLockoutConfigInput `json:"user_lockout_config"` + Options map[string]string `json:"options" mapstructure:"options"` + DefaultLeaseTTL string `json:"default_lease_ttl" mapstructure:"default_lease_ttl"` + Description *string `json:"description,omitempty" mapstructure:"description"` + MaxLeaseTTL string `json:"max_lease_ttl" mapstructure:"max_lease_ttl"` + ForceNoCache bool `json:"force_no_cache" mapstructure:"force_no_cache"` + AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" mapstructure:"audit_non_hmac_request_keys"` + AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" mapstructure:"audit_non_hmac_response_keys"` + ListingVisibility string `json:"listing_visibility,omitempty" mapstructure:"listing_visibility"` + PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" mapstructure:"passthrough_request_headers"` + AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` + TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` + AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` + PluginVersion string `json:"plugin_version,omitempty"` + UserLockoutConfig *UserLockoutConfigInput `json:"user_lockout_config,omitempty"` // Deprecated: This field will always be blank for newer server responses. PluginName string `json:"plugin_name,omitempty" mapstructure:"plugin_name"` } @@ -308,7 +308,7 @@ type UserLockoutConfigInput struct { LockoutThreshold string `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` LockoutDuration string `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` LockoutCounterResetDuration string `json:"lockout_counter_reset_duration,omitempty" structs:"lockout_counter_reset_duration" mapstructure:"lockout_counter_reset_duration"` - DisableLockout *bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` + DisableLockout bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` } type UserLockoutConfigOutput struct { diff --git a/command/auth_tune.go b/command/auth_tune.go index a151bf3a1621..d8bac2d41342 100644 --- a/command/auth_tune.go +++ b/command/auth_tune.go @@ -269,6 +269,12 @@ func (c *AuthTuneCommand) Run(args []string) int { if fl.Name == flagNameTokenType { mountConfigInput.TokenType = c.flagTokenType } + switch fl.Name { + case flagNameUserLockoutThreshold, flagNameUserLockoutDuration, flagNameUserLockoutCounterResetDuration, flagNameUserLockoutDisable: + if mountConfigInput.UserLockoutConfig == nil { + mountConfigInput.UserLockoutConfig = &api.UserLockoutConfigInput{} + } + } if fl.Name == flagNameUserLockoutThreshold { if c.flagUserLockoutThreshold > 0 { mountConfigInput.UserLockoutConfig.LockoutThreshold = strconv.Itoa(c.flagUserLockoutThreshold) @@ -281,7 +287,7 @@ func (c *AuthTuneCommand) Run(args []string) int { mountConfigInput.UserLockoutConfig.LockoutCounterResetDuration = ttlToAPI(c.flagUserLockoutCounterResetDuration) } if fl.Name == flagNameUserLockoutDisable { - mountConfigInput.UserLockoutConfig.DisableLockout = &c.flagUserLockoutDisable + mountConfigInput.UserLockoutConfig.DisableLockout = c.flagUserLockoutDisable } if fl.Name == flagNamePluginVersion { From d66876c654fa2179ef6d1c50d6ebe489ae1c0b66 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Tue, 4 Oct 2022 11:08:53 -0700 Subject: [PATCH 16/24] adding hcl parse test --- command/server/config_test.go | 4 ++ command/server/config_test_helpers.go | 67 +++++++++++++++++++++++++++ 2 files changed, 71 insertions(+) diff --git a/command/server/config_test.go b/command/server/config_test.go index e8e80cc99b3d..8bac3efd5cbd 100644 --- a/command/server/config_test.go +++ b/command/server/config_test.go @@ -36,6 +36,10 @@ func TestParseListeners(t *testing.T) { testParseListeners(t) } +func TestParseUserLockouts(t *testing.T) { + testParseUserLockouts(t) +} + func TestParseSockaddrTemplate(t *testing.T) { testParseSockaddrTemplate(t) } diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go index c459198263fa..6cd62a9cf5d8 100644 --- a/command/server/config_test_helpers.go +++ b/command/server/config_test_helpers.go @@ -861,6 +861,73 @@ listener "tcp" { } } +func testParseUserLockouts(t *testing.T) { + obj, _ := hcl.Parse(strings.TrimSpace(` + user_lockout "all" { + lockout_duration = "40m" + lockout_counter_reset = "45m" + disable_lockout = "false" + } + user_lockout "userpass" { + lockout_threshold = "100" + lockout_duration = "20m" + } + user_lockout "ldap" { + disable_lockout = "true" + }`)) + + config := Config{ + SharedConfig: &configutil.SharedConfig{}, + } + list, _ := obj.Node.(*ast.ObjectList) + objList := list.Filter("user_lockout") + configutil.ParseUserLockouts(config.SharedConfig, objList) + + expected := &Config{ + SharedConfig: &configutil.SharedConfig{ + UserLockouts: []*configutil.UserLockout{ + { + Type: "ldap", + LockoutThreshold: 5, + LockoutThresholdRaw: nil, + LockoutDuration: 2400000000000, + LockoutDurationRaw: nil, + LockoutCounterReset: 2700000000000, + LockoutCounterResetRaw: nil, + DisableLockout: true, + DisableLockoutRaw: nil, + }, + { + Type: "all", + LockoutThreshold: 5, + LockoutThresholdRaw: nil, + LockoutDuration: 2400000000000, + LockoutDurationRaw: nil, + LockoutCounterReset: 2700000000000, + LockoutCounterResetRaw: nil, + DisableLockout: false, + DisableLockoutRaw: nil, + }, + { + Type: "userpass", + LockoutThreshold: 100, + LockoutThresholdRaw: nil, + LockoutDuration: 1200000000000, + LockoutDurationRaw: nil, + LockoutCounterReset: 2700000000000, + LockoutCounterResetRaw: nil, + DisableLockout: false, + DisableLockoutRaw: nil, + }, + }, + }, + } + config.Prune() + if diff := deep.Equal(config, *expected); diff != nil { + t.Fatal(diff) + } +} + func testParseSockaddrTemplate(t *testing.T) { config, err := ParseConfig(` api_addr = < Date: Tue, 4 Oct 2022 13:29:24 -0700 Subject: [PATCH 17/24] fixing config compare --- command/server/config_test_helpers.go | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go index 6cd62a9cf5d8..18a4df7ac8cc 100644 --- a/command/server/config_test_helpers.go +++ b/command/server/config_test_helpers.go @@ -887,21 +887,21 @@ func testParseUserLockouts(t *testing.T) { SharedConfig: &configutil.SharedConfig{ UserLockouts: []*configutil.UserLockout{ { - Type: "ldap", + Type: "all", LockoutThreshold: 5, LockoutThresholdRaw: nil, LockoutDuration: 2400000000000, LockoutDurationRaw: nil, LockoutCounterReset: 2700000000000, LockoutCounterResetRaw: nil, - DisableLockout: true, + DisableLockout: false, DisableLockoutRaw: nil, }, { - Type: "all", - LockoutThreshold: 5, + Type: "userpass", + LockoutThreshold: 100, LockoutThresholdRaw: nil, - LockoutDuration: 2400000000000, + LockoutDuration: 1200000000000, LockoutDurationRaw: nil, LockoutCounterReset: 2700000000000, LockoutCounterResetRaw: nil, @@ -909,23 +909,21 @@ func testParseUserLockouts(t *testing.T) { DisableLockoutRaw: nil, }, { - Type: "userpass", - LockoutThreshold: 100, + Type: "ldap", + LockoutThreshold: 5, LockoutThresholdRaw: nil, - LockoutDuration: 1200000000000, + LockoutDuration: 2400000000000, LockoutDurationRaw: nil, LockoutCounterReset: 2700000000000, LockoutCounterResetRaw: nil, - DisableLockout: false, + DisableLockout: true, DisableLockoutRaw: nil, }, }, }, } config.Prune() - if diff := deep.Equal(config, *expected); diff != nil { - t.Fatal(diff) - } + require.Equal(t, config, *expected) } func testParseSockaddrTemplate(t *testing.T) { From 30769563bd1056478d62169ad772a154ba6bbef4 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Thu, 6 Oct 2022 15:11:58 -0700 Subject: [PATCH 18/24] fixing github comments --- api/sys_mounts.go | 24 ++++++++++++------------ command/auth_tune.go | 6 +++--- vault/logical_system.go | 27 ++++++++++----------------- vault/mount.go | 4 ++-- vault/request_handling.go | 32 -------------------------------- 5 files changed, 27 insertions(+), 66 deletions(-) diff --git a/api/sys_mounts.go b/api/sys_mounts.go index a33aff2bce6e..081891727961 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -289,17 +289,17 @@ type MountOutput struct { } type MountConfigOutput struct { - DefaultLeaseTTL int `json:"default_lease_ttl" mapstructure:"default_lease_ttl"` - MaxLeaseTTL int `json:"max_lease_ttl" mapstructure:"max_lease_ttl"` - ForceNoCache bool `json:"force_no_cache" mapstructure:"force_no_cache"` - AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" mapstructure:"audit_non_hmac_request_keys"` - AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" mapstructure:"audit_non_hmac_response_keys"` - ListingVisibility string `json:"listing_visibility,omitempty" mapstructure:"listing_visibility"` - PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" mapstructure:"passthrough_request_headers"` - AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` - TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` - AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` - UserLockoutConfig UserLockoutConfigOutput `json:"user_lockout_config"` + DefaultLeaseTTL int `json:"default_lease_ttl" mapstructure:"default_lease_ttl"` + MaxLeaseTTL int `json:"max_lease_ttl" mapstructure:"max_lease_ttl"` + ForceNoCache bool `json:"force_no_cache" mapstructure:"force_no_cache"` + AuditNonHMACRequestKeys []string `json:"audit_non_hmac_request_keys,omitempty" mapstructure:"audit_non_hmac_request_keys"` + AuditNonHMACResponseKeys []string `json:"audit_non_hmac_response_keys,omitempty" mapstructure:"audit_non_hmac_response_keys"` + ListingVisibility string `json:"listing_visibility,omitempty" mapstructure:"listing_visibility"` + PassthroughRequestHeaders []string `json:"passthrough_request_headers,omitempty" mapstructure:"passthrough_request_headers"` + AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` + TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` + AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` + UserLockoutConfig *UserLockoutConfigOutput `json:"user_lockout_config,,omitempty"` // Deprecated: This field will always be blank for newer server responses. PluginName string `json:"plugin_name,omitempty" mapstructure:"plugin_name"` } @@ -312,7 +312,7 @@ type UserLockoutConfigInput struct { } type UserLockoutConfigOutput struct { - LockoutThreshold int `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` + LockoutThreshold uint `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` LockoutDuration int `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` LockoutCounterReset int `json:"lockout_counter_reset,omitempty" structs:"lockout_counter_reset" mapstructure:"lockout_counter_reset"` DisableLockout bool `json:"disable_lockout,omitempty" structs:"disable_lockout" mapstructure:"disable_lockout"` diff --git a/command/auth_tune.go b/command/auth_tune.go index d8bac2d41342..e9740c126359 100644 --- a/command/auth_tune.go +++ b/command/auth_tune.go @@ -32,7 +32,7 @@ type AuthTuneCommand struct { flagTokenType string flagVersion int flagPluginVersion string - flagUserLockoutThreshold int + flagUserLockoutThreshold uint flagUserLockoutDuration time.Duration flagUserLockoutCounterResetDuration time.Duration flagUserLockoutDisable bool @@ -149,7 +149,7 @@ func (c *AuthTuneCommand) Flags() *FlagSets { Usage: "Select the version of the auth method to run. Not supported by all auth methods.", }) - f.IntVar(&IntVar{ + f.UintVar(&UintVar{ Name: flagNameUserLockoutThreshold, Target: &c.flagUserLockoutThreshold, Usage: "The threshold for user lockout for this auth method. If unspecified, this " + @@ -277,7 +277,7 @@ func (c *AuthTuneCommand) Run(args []string) int { } if fl.Name == flagNameUserLockoutThreshold { if c.flagUserLockoutThreshold > 0 { - mountConfigInput.UserLockoutConfig.LockoutThreshold = strconv.Itoa(c.flagUserLockoutThreshold) + mountConfigInput.UserLockoutConfig.LockoutThreshold = strconv.FormatUint(uint64(c.flagUserLockoutThreshold), 10) } } if fl.Name == flagNameUserLockoutDuration { diff --git a/vault/logical_system.go b/vault/logical_system.go index e3966b9ed0a7..fb8890170c81 100644 --- a/vault/logical_system.go +++ b/vault/logical_system.go @@ -1739,7 +1739,6 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, userLockoutConfigMap := data.Get("user_lockout_config").(map[string]interface{}) var err error - // Augmenting userLockoutConfigMap for some config options to treat them as comma separated entries if userLockoutConfigMap != nil && len(userLockoutConfigMap) != 0 { err := mapstructure.Decode(userLockoutConfigMap, &apiuserLockoutConfig) if err != nil { @@ -1749,9 +1748,8 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, } // Supported auth methods for user lockout configuration: ldap, approle, userpass - // "all" is used to apply the configuration to all supported auth methods switch strings.ToLower(mountEntry.Type) { - case "all", "ldap", "approle", "userpass": + case "ldap", "approle", "userpass": default: return logical.ErrorResponse("tuning of user lockout configuration for auth type %q not allowed", mountEntry.Type), logical.ErrInvalidRequest @@ -1763,26 +1761,21 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, mountEntry.Config.UserLockoutConfig = &UserLockoutConfig{} } - var oldUserLockoutThreshold int64 + var oldUserLockoutThreshold uint64 var newUserLockoutDuration, oldUserLockoutDuration time.Duration var newUserLockoutCounterReset, oldUserLockoutCounterReset time.Duration var oldUserLockoutDisable bool - if _, ok := userLockoutConfigMap["lockout_threshold"]; ok { - userLockoutThreshold, err := parseutil.ParseInt(apiuserLockoutConfig.LockoutThreshold) + if apiuserLockoutConfig.LockoutThreshold != "" { + userLockoutThreshold, err := strconv.ParseUint(apiuserLockoutConfig.LockoutThreshold, 10, 64) if err != nil { return nil, fmt.Errorf("unable to parse user lockout threshold: %w", err) } oldUserLockoutThreshold = mountEntry.Config.UserLockoutConfig.LockoutThreshold - switch { - case userLockoutThreshold < 0: - mountEntry.Config.UserLockoutConfig.LockoutThreshold = oldUserLockoutThreshold - default: - mountEntry.Config.UserLockoutConfig.LockoutThreshold = userLockoutThreshold - } + mountEntry.Config.UserLockoutConfig.LockoutThreshold = userLockoutThreshold } - if _, ok := userLockoutConfigMap["lockout_duration"]; ok { + if apiuserLockoutConfig.LockoutDuration != "" { oldUserLockoutDuration = mountEntry.Config.UserLockoutConfig.LockoutDuration switch apiuserLockoutConfig.LockoutDuration { case "": @@ -1800,7 +1793,7 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, mountEntry.Config.UserLockoutConfig.LockoutDuration = newUserLockoutDuration } - if _, ok := userLockoutConfigMap["lockout_counter_reset_duration"]; ok { + if apiuserLockoutConfig.LockoutCounterResetDuration != "" { oldUserLockoutCounterReset = mountEntry.Config.UserLockoutConfig.LockoutCounterReset switch apiuserLockoutConfig.LockoutCounterResetDuration { case "": @@ -1818,10 +1811,10 @@ func (b *SystemBackend) handleTuneWriteCommon(ctx context.Context, path string, mountEntry.Config.UserLockoutConfig.LockoutCounterReset = newUserLockoutCounterReset } - if rawVal, ok := userLockoutConfigMap["lockout_disable"]; ok { - userLockoutDisable := rawVal.(bool) + if apiuserLockoutConfig.DisableLockout != nil { oldUserLockoutDisable = mountEntry.Config.UserLockoutConfig.DisableLockout - mountEntry.Config.UserLockoutConfig.DisableLockout = userLockoutDisable + userLockoutDisable := apiuserLockoutConfig.DisableLockout + mountEntry.Config.UserLockoutConfig.DisableLockout = *userLockoutDisable } // Update the mount table diff --git a/vault/mount.go b/vault/mount.go index db21d5f14206..88b6d2bfe5b9 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -360,7 +360,7 @@ type MountConfig struct { } type UserLockoutConfig struct { - LockoutThreshold int64 `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` + LockoutThreshold uint64 `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` LockoutDuration time.Duration `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` LockoutCounterReset time.Duration `json:"lockout_counter_reset,omitempty" structs:"lockout_counter_reset" mapstructure:"lockout_counter_reset"` DisableLockout bool `json:"disable_lockout,omitempty" structs:"disable_lockout" mapstructure:"disable_lockout"` @@ -370,7 +370,7 @@ type APIUserLockoutConfig struct { LockoutThreshold string `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` LockoutDuration string `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` LockoutCounterResetDuration string `json:"lockout_counter_reset_duration,omitempty" structs:"lockout_counter_reset_duration" mapstructure:"lockout_counter_reset_duration"` - DisableLockout bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` + DisableLockout *bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` } // APIMountConfig is an embedded struct of api.MountConfigInput diff --git a/vault/request_handling.go b/vault/request_handling.go index 72fd5d481788..b37085c83c10 100644 --- a/vault/request_handling.go +++ b/vault/request_handling.go @@ -17,7 +17,6 @@ import ( "github.com/hashicorp/go-secure-stdlib/strutil" "github.com/hashicorp/go-sockaddr" "github.com/hashicorp/go-uuid" - "github.com/hashicorp/vault/command/server" "github.com/hashicorp/vault/helper/identity" "github.com/hashicorp/vault/helper/identity/mfa" "github.com/hashicorp/vault/helper/metricsutil" @@ -1993,34 +1992,3 @@ func (c *Core) checkSSCTokenInternal(ctx context.Context, token string, isPerfSt // status code. return "", logical.ErrMissingRequiredState } - -func (c *Core) GetUserLockoutFromConfig(logicalType string) UserLockoutConfig { - conf := c.rawConfig.Load() - if conf == nil { - return UserLockoutConfig{} - } - userlockouts := conf.(*server.Config).UserLockouts - - if userlockouts == nil { - return UserLockoutConfig{} - } - - var commonUserLockoutConfig UserLockoutConfig - var authUserLockoutConfig UserLockoutConfig - for _, userLockoutConfig := range userlockouts { - if userLockoutConfig.Type == logicalType { - authUserLockoutConfig.LockoutThreshold = userLockoutConfig.LockoutThreshold - authUserLockoutConfig.LockoutDuration = userLockoutConfig.LockoutDuration - authUserLockoutConfig.LockoutCounterReset = userLockoutConfig.LockoutCounterReset - authUserLockoutConfig.DisableLockout = userLockoutConfig.DisableLockout - return authUserLockoutConfig - } - if userLockoutConfig.Type == "all" { - commonUserLockoutConfig.LockoutThreshold = userLockoutConfig.LockoutThreshold - commonUserLockoutConfig.LockoutDuration = userLockoutConfig.LockoutDuration - commonUserLockoutConfig.LockoutCounterReset = userLockoutConfig.LockoutCounterReset - commonUserLockoutConfig.DisableLockout = userLockoutConfig.DisableLockout - } - } - return commonUserLockoutConfig -} From 4aff75d5a15f87a8d8089ad52fffb2e0f06da459 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Thu, 6 Oct 2022 18:23:08 -0700 Subject: [PATCH 19/24] optimize userlockouts.go --- api/sys_mounts.go | 2 +- command/auth_tune.go | 4 +- internalshared/configutil/userlockout.go | 92 +++++++++++++----------- vault/mount.go | 2 +- 4 files changed, 55 insertions(+), 45 deletions(-) diff --git a/api/sys_mounts.go b/api/sys_mounts.go index 081891727961..7cf2213f8a76 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -299,7 +299,7 @@ type MountConfigOutput struct { AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" mapstructure:"allowed_response_headers"` TokenType string `json:"token_type,omitempty" mapstructure:"token_type"` AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` - UserLockoutConfig *UserLockoutConfigOutput `json:"user_lockout_config,,omitempty"` + UserLockoutConfig *UserLockoutConfigOutput `json:"user_lockout_config,omitempty"` // Deprecated: This field will always be blank for newer server responses. PluginName string `json:"plugin_name,omitempty" mapstructure:"plugin_name"` } diff --git a/command/auth_tune.go b/command/auth_tune.go index e9740c126359..0d5cedd3f172 100644 --- a/command/auth_tune.go +++ b/command/auth_tune.go @@ -276,9 +276,7 @@ func (c *AuthTuneCommand) Run(args []string) int { } } if fl.Name == flagNameUserLockoutThreshold { - if c.flagUserLockoutThreshold > 0 { - mountConfigInput.UserLockoutConfig.LockoutThreshold = strconv.FormatUint(uint64(c.flagUserLockoutThreshold), 10) - } + mountConfigInput.UserLockoutConfig.LockoutThreshold = strconv.FormatUint(uint64(c.flagUserLockoutThreshold), 10) } if fl.Name == flagNameUserLockoutDuration { mountConfigInput.UserLockoutConfig.LockoutDuration = ttlToAPI(c.flagUserLockoutDuration) diff --git a/internalshared/configutil/userlockout.go b/internalshared/configutil/userlockout.go index 2b4faca584a8..d2d9fb35c48d 100644 --- a/internalshared/configutil/userlockout.go +++ b/internalshared/configutil/userlockout.go @@ -3,6 +3,7 @@ package configutil import ( "errors" "fmt" + "strconv" "strings" "time" @@ -21,7 +22,7 @@ const ( type UserLockout struct { Type string - LockoutThreshold int64 `hcl:"-"` + LockoutThreshold uint64 `hcl:"-"` LockoutThresholdRaw interface{} `hcl:"lockout_threshold"` LockoutDuration time.Duration `hcl:"-"` LockoutDurationRaw interface{} `hcl:"lockout_duration"` @@ -65,7 +66,8 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { // Lockout Parameters { if userLockoutConfig.LockoutThresholdRaw != nil { - if userLockoutConfig.LockoutThreshold, err = parseutil.ParseInt(userLockoutConfig.LockoutThresholdRaw); err != nil { + userLockoutThresholdString := fmt.Sprintf("%v", userLockoutConfig.LockoutThresholdRaw) + if userLockoutConfig.LockoutThreshold, err = strconv.ParseUint(userLockoutThresholdString, 10, 64); err != nil { return multierror.Prefix(fmt.Errorf("error parsing lockout_threshold: %w", err), fmt.Sprintf("user_lockouts.%d", i)) } } @@ -105,41 +107,49 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { return nil } -// setDefaultUserLockoutValuesInMap sets default user lockout values for key "all" (all auth methods) +// setUserLockoutValueAllInMap sets default user lockout values for key "all" (all auth methods) // for user lockout fields that are not configured using config file -func setDefaultUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) map[string]*UserLockout { - if userLockoutAll, ok := userLockoutsMap["all"]; !ok { - var tmpUserLockoutConfig UserLockout +func setUserLockoutValueAllInMap(userLockoutAll *UserLockout) *UserLockout { + var tmpUserLockoutConfig UserLockout + if userLockoutAll == nil { tmpUserLockoutConfig.Type = "all" tmpUserLockoutConfig.LockoutThreshold = UserLockoutThresholdDefault tmpUserLockoutConfig.LockoutDuration = UserLockoutDurationDefault tmpUserLockoutConfig.LockoutCounterReset = UserLockoutCounterResetDefault tmpUserLockoutConfig.DisableLockout = DisableUserLockoutDefault - userLockoutsMap["all"] = &tmpUserLockoutConfig - - } else { - if userLockoutAll.LockoutThresholdRaw == nil { - userLockoutAll.LockoutThreshold = UserLockoutThresholdDefault - } - if userLockoutAll.LockoutDurationRaw == nil { - userLockoutAll.LockoutDuration = UserLockoutDurationDefault - } - if userLockoutAll.LockoutCounterResetRaw == nil { - userLockoutAll.LockoutCounterReset = UserLockoutCounterResetDefault - } - if userLockoutAll.DisableLockoutRaw == nil { - userLockoutAll.DisableLockout = DisableUserLockoutDefault - } - userLockoutsMap[userLockoutAll.Type] = userLockoutAll + return &tmpUserLockoutConfig } - return userLockoutsMap + tmpUserLockoutConfig = *userLockoutAll + if tmpUserLockoutConfig.LockoutThresholdRaw == nil { + tmpUserLockoutConfig.LockoutThreshold = UserLockoutThresholdDefault + } + if tmpUserLockoutConfig.LockoutDurationRaw == nil { + tmpUserLockoutConfig.LockoutDuration = UserLockoutDurationDefault + } + if tmpUserLockoutConfig.LockoutCounterResetRaw == nil { + tmpUserLockoutConfig.LockoutCounterReset = UserLockoutCounterResetDefault + } + if tmpUserLockoutConfig.DisableLockoutRaw == nil { + tmpUserLockoutConfig.DisableLockout = DisableUserLockoutDefault + } + return setNilValuesForRawUserLockoutFields(&tmpUserLockoutConfig) } // setDefaultUserLockoutValuesInMap sets missing user lockout fields for auth methods // with default values (from key "all") that are not configured using config file func setMissingUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) map[string]*UserLockout { - userLockoutsMap = setDefaultUserLockoutValuesInMap(userLockoutsMap) + userLockoutAll, ok := userLockoutsMap["all"] + switch ok { + case true: + userLockoutsMap["all"] = setUserLockoutValueAllInMap(userLockoutAll) + default: + userLockoutsMap["all"] = setUserLockoutValueAllInMap(&UserLockout{}) + } + for _, userLockoutAuth := range userLockoutsMap { + if userLockoutAuth.Type == "all" { + continue + } // set missing values if userLockoutAuth.LockoutThresholdRaw == nil { userLockoutAuth.LockoutThreshold = userLockoutsMap["all"].LockoutThreshold @@ -153,23 +163,25 @@ func setMissingUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) m if userLockoutAuth.DisableLockoutRaw == nil { userLockoutAuth.DisableLockout = userLockoutsMap["all"].DisableLockout } - - // set nil values to Raw fields - if userLockoutAuth.LockoutThresholdRaw != nil { - userLockoutAuth.LockoutThresholdRaw = nil - } - if userLockoutAuth.LockoutDurationRaw != nil { - userLockoutAuth.LockoutDurationRaw = nil - } - if userLockoutAuth.LockoutCounterResetRaw != nil { - userLockoutAuth.LockoutCounterResetRaw = nil - } - if userLockoutAuth.DisableLockoutRaw != nil { - userLockoutAuth.DisableLockoutRaw = nil - } - + userLockoutAuth = setNilValuesForRawUserLockoutFields(userLockoutAuth) userLockoutsMap[userLockoutAuth.Type] = userLockoutAuth } - return userLockoutsMap } + +// setNilValuesForRawUserLockoutFields sets nil values for user lockout Raw fields +func setNilValuesForRawUserLockoutFields(userLockout *UserLockout) *UserLockout { + if userLockout.LockoutThresholdRaw != nil { + userLockout.LockoutThresholdRaw = nil + } + if userLockout.LockoutDurationRaw != nil { + userLockout.LockoutDurationRaw = nil + } + if userLockout.LockoutCounterResetRaw != nil { + userLockout.LockoutCounterResetRaw = nil + } + if userLockout.DisableLockoutRaw != nil { + userLockout.DisableLockoutRaw = nil + } + return userLockout +} diff --git a/vault/mount.go b/vault/mount.go index 88b6d2bfe5b9..4be2350aadf6 100644 --- a/vault/mount.go +++ b/vault/mount.go @@ -385,7 +385,7 @@ type APIMountConfig struct { AllowedResponseHeaders []string `json:"allowed_response_headers,omitempty" structs:"allowed_response_headers" mapstructure:"allowed_response_headers"` TokenType string `json:"token_type" structs:"token_type" mapstructure:"token_type"` AllowedManagedKeys []string `json:"allowed_managed_keys,omitempty" mapstructure:"allowed_managed_keys"` - UserLockoutConfig UserLockoutConfig `json:"user_lockout_config,omitempty" mapstructure:"user_lockout_config"` + UserLockoutConfig *UserLockoutConfig `json:"user_lockout_config,omitempty" mapstructure:"user_lockout_config"` PluginVersion string `json:"plugin_version,omitempty" mapstructure:"plugin_version"` // PluginName is the name of the plugin registered in the catalog. From 96991699cc465bd02a18cfba7a9cce7d5bf779d1 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Fri, 7 Oct 2022 11:03:12 -0700 Subject: [PATCH 20/24] fixing test --- internalshared/configutil/userlockout.go | 31 ++++++++++-------------- 1 file changed, 13 insertions(+), 18 deletions(-) diff --git a/internalshared/configutil/userlockout.go b/internalshared/configutil/userlockout.go index d2d9fb35c48d..cb799b225821 100644 --- a/internalshared/configutil/userlockout.go +++ b/internalshared/configutil/userlockout.go @@ -110,34 +110,29 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { // setUserLockoutValueAllInMap sets default user lockout values for key "all" (all auth methods) // for user lockout fields that are not configured using config file func setUserLockoutValueAllInMap(userLockoutAll *UserLockout) *UserLockout { - var tmpUserLockoutConfig UserLockout - if userLockoutAll == nil { - tmpUserLockoutConfig.Type = "all" - tmpUserLockoutConfig.LockoutThreshold = UserLockoutThresholdDefault - tmpUserLockoutConfig.LockoutDuration = UserLockoutDurationDefault - tmpUserLockoutConfig.LockoutCounterReset = UserLockoutCounterResetDefault - tmpUserLockoutConfig.DisableLockout = DisableUserLockoutDefault - return &tmpUserLockoutConfig + if userLockoutAll.Type == "" { + userLockoutAll.Type = "all" } - tmpUserLockoutConfig = *userLockoutAll - if tmpUserLockoutConfig.LockoutThresholdRaw == nil { - tmpUserLockoutConfig.LockoutThreshold = UserLockoutThresholdDefault + if userLockoutAll.LockoutThresholdRaw == nil { + userLockoutAll.LockoutThreshold = UserLockoutThresholdDefault } - if tmpUserLockoutConfig.LockoutDurationRaw == nil { - tmpUserLockoutConfig.LockoutDuration = UserLockoutDurationDefault + if userLockoutAll.LockoutDurationRaw == nil { + userLockoutAll.LockoutDuration = UserLockoutDurationDefault } - if tmpUserLockoutConfig.LockoutCounterResetRaw == nil { - tmpUserLockoutConfig.LockoutCounterReset = UserLockoutCounterResetDefault + if userLockoutAll.LockoutCounterResetRaw == nil { + userLockoutAll.LockoutCounterReset = UserLockoutCounterResetDefault } - if tmpUserLockoutConfig.DisableLockoutRaw == nil { - tmpUserLockoutConfig.DisableLockout = DisableUserLockoutDefault + if userLockoutAll.DisableLockoutRaw == nil { + userLockoutAll.DisableLockout = DisableUserLockoutDefault } - return setNilValuesForRawUserLockoutFields(&tmpUserLockoutConfig) + return setNilValuesForRawUserLockoutFields(userLockoutAll) } // setDefaultUserLockoutValuesInMap sets missing user lockout fields for auth methods // with default values (from key "all") that are not configured using config file func setMissingUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) map[string]*UserLockout { + // set values for "all" key with default values for "all" user lockout fields that are not configured + // the "all" key values will be used as default values for other auth methods userLockoutAll, ok := userLockoutsMap["all"] switch ok { case true: From 916b3722e3648a0de5732e2f5f30ef063c4e98d6 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Tue, 25 Oct 2022 16:47:14 -0700 Subject: [PATCH 21/24] minor changes --- internalshared/configutil/config.go | 3 ++- internalshared/configutil/userlockout.go | 16 ++++------------ 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/internalshared/configutil/config.go b/internalshared/configutil/config.go index bddaba964748..c13c80451bc5 100644 --- a/internalshared/configutil/config.go +++ b/internalshared/configutil/config.go @@ -19,7 +19,8 @@ type SharedConfig struct { EntSharedConfig - Listeners []*Listener `hcl:"-"` + Listeners []*Listener `hcl:"-"` + UserLockouts []*UserLockout `hcl:"-"` Seals []*KMS `hcl:"-"` diff --git a/internalshared/configutil/userlockout.go b/internalshared/configutil/userlockout.go index cb799b225821..d2d5257ad384 100644 --- a/internalshared/configutil/userlockout.go +++ b/internalshared/configutil/userlockout.go @@ -166,17 +166,9 @@ func setMissingUserLockoutValuesInMap(userLockoutsMap map[string]*UserLockout) m // setNilValuesForRawUserLockoutFields sets nil values for user lockout Raw fields func setNilValuesForRawUserLockoutFields(userLockout *UserLockout) *UserLockout { - if userLockout.LockoutThresholdRaw != nil { - userLockout.LockoutThresholdRaw = nil - } - if userLockout.LockoutDurationRaw != nil { - userLockout.LockoutDurationRaw = nil - } - if userLockout.LockoutCounterResetRaw != nil { - userLockout.LockoutCounterResetRaw = nil - } - if userLockout.DisableLockoutRaw != nil { - userLockout.DisableLockoutRaw = nil - } + userLockout.LockoutThresholdRaw = nil + userLockout.LockoutDurationRaw = nil + userLockout.LockoutCounterResetRaw = nil + userLockout.DisableLockoutRaw = nil return userLockout } From 7f6305ac58e452a671bd84ab8f8f6472a01dc239 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Mon, 31 Oct 2022 15:04:49 -0700 Subject: [PATCH 22/24] adding comments --- api/sys_mounts.go | 10 +++++----- command/auth_tune.go | 2 +- internalshared/configutil/userlockout.go | 12 ++++++++++++ 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/api/sys_mounts.go b/api/sys_mounts.go index 7cf2213f8a76..f55133cec4c6 100644 --- a/api/sys_mounts.go +++ b/api/sys_mounts.go @@ -308,14 +308,14 @@ type UserLockoutConfigInput struct { LockoutThreshold string `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` LockoutDuration string `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` LockoutCounterResetDuration string `json:"lockout_counter_reset_duration,omitempty" structs:"lockout_counter_reset_duration" mapstructure:"lockout_counter_reset_duration"` - DisableLockout bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` + DisableLockout *bool `json:"lockout_disable,omitempty" structs:"lockout_disable" mapstructure:"lockout_disable"` } type UserLockoutConfigOutput struct { - LockoutThreshold uint `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` - LockoutDuration int `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` - LockoutCounterReset int `json:"lockout_counter_reset,omitempty" structs:"lockout_counter_reset" mapstructure:"lockout_counter_reset"` - DisableLockout bool `json:"disable_lockout,omitempty" structs:"disable_lockout" mapstructure:"disable_lockout"` + LockoutThreshold uint `json:"lockout_threshold,omitempty" structs:"lockout_threshold" mapstructure:"lockout_threshold"` + LockoutDuration int `json:"lockout_duration,omitempty" structs:"lockout_duration" mapstructure:"lockout_duration"` + LockoutCounterReset int `json:"lockout_counter_reset,omitempty" structs:"lockout_counter_reset" mapstructure:"lockout_counter_reset"` + DisableLockout *bool `json:"disable_lockout,omitempty" structs:"disable_lockout" mapstructure:"disable_lockout"` } type MountMigrationOutput struct { diff --git a/command/auth_tune.go b/command/auth_tune.go index 0d5cedd3f172..6e3d3e7bce8f 100644 --- a/command/auth_tune.go +++ b/command/auth_tune.go @@ -285,7 +285,7 @@ func (c *AuthTuneCommand) Run(args []string) int { mountConfigInput.UserLockoutConfig.LockoutCounterResetDuration = ttlToAPI(c.flagUserLockoutCounterResetDuration) } if fl.Name == flagNameUserLockoutDisable { - mountConfigInput.UserLockoutConfig.DisableLockout = c.flagUserLockoutDisable + mountConfigInput.UserLockoutConfig.DisableLockout = &c.flagUserLockoutDisable } if fl.Name == flagNamePluginVersion { diff --git a/internalshared/configutil/userlockout.go b/internalshared/configutil/userlockout.go index d2d5257ad384..f2be4461b32d 100644 --- a/internalshared/configutil/userlockout.go +++ b/internalshared/configutil/userlockout.go @@ -64,6 +64,9 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { } // Lockout Parameters + + // Not setting raw entries to nil here as soon as they are parsed + // as they are used to set the missing user lockout configuration values later. { if userLockoutConfig.LockoutThresholdRaw != nil { userLockoutThresholdString := fmt.Sprintf("%v", userLockoutConfig.LockoutThresholdRaw) @@ -100,6 +103,15 @@ func ParseUserLockouts(result *SharedConfig, list *ast.ObjectList) error { userLockoutsMap[userLockoutConfig.Type] = &userLockoutConfig } + // Use raw entries to set values for user lockout configurations fields + // that were not configured using config file. + // The raw entries would mean that the entry was configured by the user using the config file. + // If any of these fields are not configured using the config file (missing fields), + // we set values for these fields with defaults + // The issue with not being able to use non-raw entries is because of fields lockout threshold + // and disable lockout. We cannot differentiate using non-raw entries if the user configured these fields + // with values (0 and false) or if the the user did not configure these values in config file at all. + // The raw fields are set to nil after setting missing values in setNilValuesForRawUserLockoutFields function userLockoutsMap = setMissingUserLockoutValuesInMap(userLockoutsMap) for _, userLockoutValues := range userLockoutsMap { result.UserLockouts = append(result.UserLockouts, userLockoutValues) From 17813cd071f21d1542a86bc9b80f2ca9405e2431 Mon Sep 17 00:00:00 2001 From: akshya96 Date: Mon, 31 Oct 2022 17:45:49 -0700 Subject: [PATCH 23/24] adding sort to flaky test --- command/server/config_test_helpers.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go index a3cb740ab8b8..a2cf8df6b6a8 100644 --- a/command/server/config_test_helpers.go +++ b/command/server/config_test_helpers.go @@ -3,6 +3,7 @@ package server import ( "fmt" "reflect" + "sort" "strings" "testing" "time" @@ -914,6 +915,10 @@ func testParseUserLockouts(t *testing.T) { objList := list.Filter("user_lockout") configutil.ParseUserLockouts(config.SharedConfig, objList) + sort.Slice(config.SharedConfig.UserLockouts[:], func(i, j int) bool { + return config.SharedConfig.UserLockouts[i].Type < config.SharedConfig.UserLockouts[j].Type + }) + expected := &Config{ SharedConfig: &configutil.SharedConfig{ UserLockouts: []*configutil.UserLockout{ @@ -953,6 +958,10 @@ func testParseUserLockouts(t *testing.T) { }, }, } + + sort.Slice(expected.SharedConfig.UserLockouts[:], func(i, j int) bool { + return expected.SharedConfig.UserLockouts[i].Type < expected.SharedConfig.UserLockouts[j].Type + }) config.Prune() require.Equal(t, config, *expected) } From fdb524847039070d2b8243b600a072bfcc30533b Mon Sep 17 00:00:00 2001 From: akshya96 Date: Mon, 31 Oct 2022 18:29:55 -0700 Subject: [PATCH 24/24] fix flaky test --- command/server/config_test_helpers.go | 42 ++++++++++----------------- 1 file changed, 15 insertions(+), 27 deletions(-) diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go index a2cf8df6b6a8..248040f9cf8b 100644 --- a/command/server/config_test_helpers.go +++ b/command/server/config_test_helpers.go @@ -923,37 +923,25 @@ func testParseUserLockouts(t *testing.T) { SharedConfig: &configutil.SharedConfig{ UserLockouts: []*configutil.UserLockout{ { - Type: "all", - LockoutThreshold: 5, - LockoutThresholdRaw: nil, - LockoutDuration: 2400000000000, - LockoutDurationRaw: nil, - LockoutCounterReset: 2700000000000, - LockoutCounterResetRaw: nil, - DisableLockout: false, - DisableLockoutRaw: nil, + Type: "all", + LockoutThreshold: 5, + LockoutDuration: 2400000000000, + LockoutCounterReset: 2700000000000, + DisableLockout: false, }, { - Type: "userpass", - LockoutThreshold: 100, - LockoutThresholdRaw: nil, - LockoutDuration: 1200000000000, - LockoutDurationRaw: nil, - LockoutCounterReset: 2700000000000, - LockoutCounterResetRaw: nil, - DisableLockout: false, - DisableLockoutRaw: nil, + Type: "userpass", + LockoutThreshold: 100, + LockoutDuration: 1200000000000, + LockoutCounterReset: 2700000000000, + DisableLockout: false, }, { - Type: "ldap", - LockoutThreshold: 5, - LockoutThresholdRaw: nil, - LockoutDuration: 2400000000000, - LockoutDurationRaw: nil, - LockoutCounterReset: 2700000000000, - LockoutCounterResetRaw: nil, - DisableLockout: true, - DisableLockoutRaw: nil, + Type: "ldap", + LockoutThreshold: 5, + LockoutDuration: 2400000000000, + LockoutCounterReset: 2700000000000, + DisableLockout: true, }, }, },