From d308c31cbf151cdbf92a149d034631fa58544bb0 Mon Sep 17 00:00:00 2001 From: "Luis (LT) Carbonell" Date: Thu, 20 Apr 2023 15:39:27 -0500 Subject: [PATCH] Add Configurable LDAP Max Page Size (#19032) * Add config flag for LDAP max page size * Add changelog * move changelog to correct file * cleanup * Default to non-paged searching for with -1 * Update website/content/api-docs/auth/ldap.mdx Co-authored-by: Austin Gebauer <34121980+austingebauer@users.noreply.github.com> * Update website/content/docs/auth/ldap.mdx Co-authored-by: Austin Gebauer <34121980+austingebauer@users.noreply.github.com> * Update tests --------- Co-authored-by: Austin Gebauer <34121980+austingebauer@users.noreply.github.com> --- builtin/credential/ldap/backend_test.go | 2 ++ changelog/19032.txt | 3 +++ helper/testhelpers/ldap/ldaphelper.go | 1 + sdk/helper/ldaputil/client.go | 4 ++-- sdk/helper/ldaputil/config.go | 13 +++++++++++++ sdk/helper/ldaputil/config_test.go | 1 + website/content/api-docs/auth/ldap.mdx | 9 ++++++++- website/content/docs/auth/ldap.mdx | 2 +- 8 files changed, 31 insertions(+), 4 deletions(-) create mode 100644 changelog/19032.txt diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index 7f81e0bfa..beda248e4 100644 --- a/builtin/credential/ldap/backend_test.go +++ b/builtin/credential/ldap/backend_test.go @@ -1197,6 +1197,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) { "token_period": "5m", "token_explicit_max_ttl": "24h", "request_timeout": cfg.RequestTimeout, + "max_page_size": cfg.MaximumPageSize, "connection_timeout": cfg.ConnectionTimeout, }, Storage: storage, @@ -1242,6 +1243,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) { ConnectionTimeout: cfg.ConnectionTimeout, UsernameAsAlias: false, DerefAliases: "never", + MaximumPageSize: 1000, }, } diff --git a/changelog/19032.txt b/changelog/19032.txt new file mode 100644 index 000000000..a474c22ce --- /dev/null +++ b/changelog/19032.txt @@ -0,0 +1,3 @@ +```release-note:bug +auth/ldap: Add max_page_size configurable to LDAP configuration +``` diff --git a/helper/testhelpers/ldap/ldaphelper.go b/helper/testhelpers/ldap/ldaphelper.go index 15b405f79..64df090f4 100644 --- a/helper/testhelpers/ldap/ldaphelper.go +++ b/helper/testhelpers/ldap/ldaphelper.go @@ -36,6 +36,7 @@ func PrepareTestContainer(t *testing.T, version string) (cleanup func(), cfg *ld cfg.GroupDN = "ou=people,dc=planetexpress,dc=com" cfg.GroupAttr = "cn" cfg.RequestTimeout = 60 + cfg.MaximumPageSize = 1000 svc, err := runner.StartService(context.Background(), func(ctx context.Context, host string, port int) (docker.ServiceConfig, error) { connURL := fmt.Sprintf("ldap://%s:%d", host, port) diff --git a/sdk/helper/ldaputil/client.go b/sdk/helper/ldaputil/client.go index f86bfd055..470de59f1 100644 --- a/sdk/helper/ldaputil/client.go +++ b/sdk/helper/ldaputil/client.go @@ -432,7 +432,7 @@ func (c *Client) performLdapFilterGroupsSearchPaging(cfg *ConfigEntry, conn Pagi cfg.GroupAttr, }, SizeLimit: math.MaxInt32, - }, math.MaxInt32) + }, uint32(cfg.MaximumPageSize)) if err != nil { return nil, fmt.Errorf("LDAP search failed: %w", err) } @@ -553,7 +553,7 @@ func (c *Client) GetLdapGroups(cfg *ConfigEntry, conn Connection, userDN string, if cfg.UseTokenGroups { entries, err = c.performLdapTokenGroupsSearch(cfg, conn, userDN) } else { - if paging, ok := conn.(PagingConnection); ok { + if paging, ok := conn.(PagingConnection); ok && cfg.MaximumPageSize >= 0 { entries, err = c.performLdapFilterGroupsSearchPaging(cfg, paging, userDN, username) } else { entries, err = c.performLdapFilterGroupsSearch(cfg, conn, userDN, username) diff --git a/sdk/helper/ldaputil/config.go b/sdk/helper/ldaputil/config.go index 12919ad50..0313817ae 100644 --- a/sdk/helper/ldaputil/config.go +++ b/sdk/helper/ldaputil/config.go @@ -9,6 +9,7 @@ import ( "encoding/pem" "errors" "fmt" + "math" "strings" "text/template" @@ -251,6 +252,12 @@ Default: ({{.UserAttr}}={{.Username}})`, Default: "never", AllowedValues: []interface{}{"never", "finding", "searching", "always"}, }, + + "max_page_size": { + Type: framework.TypeInt, + Description: "The maximum number of results to return for a single paged query. If not set, the server default will be used for paged searches. A requested max_page_size of 0 is interpreted as no limit by LDAP servers. If set to a negative value, search requests will not be paged.", + Default: math.MaxInt32, + }, } } @@ -425,6 +432,10 @@ func NewConfigEntry(existing *ConfigEntry, d *framework.FieldData) (*ConfigEntry cfg.DerefAliases = d.Get("dereference_aliases").(string) } + if _, ok := d.Raw["max_page_size"]; ok || !hadExisting { + cfg.MaximumPageSize = d.Get("max_page_size").(int) + } + return cfg, nil } @@ -453,6 +464,7 @@ type ConfigEntry struct { RequestTimeout int `json:"request_timeout"` ConnectionTimeout int `json:"connection_timeout"` DerefAliases string `json:"dereference_aliases"` + MaximumPageSize int `json:"max_page_size"` // These json tags deviate from snake case because there was a past issue // where the tag was being ignored, causing it to be jsonified as "CaseSensitiveNames", etc. @@ -493,6 +505,7 @@ func (c *ConfigEntry) PasswordlessMap() map[string]interface{} { "connection_timeout": c.ConnectionTimeout, "username_as_alias": c.UsernameAsAlias, "dereference_aliases": c.DerefAliases, + "max_page_size": c.MaximumPageSize, } if c.CaseSensitiveNames != nil { m["case_sensitive_names"] = *c.CaseSensitiveNames diff --git a/sdk/helper/ldaputil/config_test.go b/sdk/helper/ldaputil/config_test.go index e6f9997f0..1fd96385c 100644 --- a/sdk/helper/ldaputil/config_test.go +++ b/sdk/helper/ldaputil/config_test.go @@ -175,6 +175,7 @@ var jsonConfigDefault = []byte(` "request_timeout": 90, "connection_timeout": 30, "dereference_aliases": "never", + "max_page_size": 2147483647, "CaseSensitiveNames": false, "ClientTLSCert": "", "ClientTLSKey": "" diff --git a/website/content/api-docs/auth/ldap.mdx b/website/content/api-docs/auth/ldap.mdx index 208493743..ccb42faf3 100644 --- a/website/content/api-docs/auth/ldap.mdx +++ b/website/content/api-docs/auth/ldap.mdx @@ -97,6 +97,12 @@ This endpoint configures the LDAP auth method. - `dereference_aliases` `(string: never)` - When aliases should be dereferenced on search operations. Accepted values are 'never', 'finding', 'searching', 'always'. Defaults to 'never'. +- `max_page_size` `(int: math.MaxInt32)` - If set to a value greater than 0, the LDAP + backend will use the LDAP server's paged search control to request pages of + up to the given size. This can be used to avoid hitting the LDAP server's + maximum result size limit. A value of 0 will be interpreted by the LDAP + server as unlimited. If set to -1, the LDAP backend will not use the + paged search control. @include 'tokenfields.mdx' @@ -129,7 +135,8 @@ $ curl \ "url": "ldaps://ldap.myorg.com:636", "username_as_alias": false, "userattr": "samaccountname", - "userdn": "ou=Users,dc=example,dc=com" + "userdn": "ou=Users,dc=example,dc=com", + "max_page_size": 1000 } ``` diff --git a/website/content/docs/auth/ldap.mdx b/website/content/docs/auth/ldap.mdx index bcb1084a2..cabb3769e 100644 --- a/website/content/docs/auth/ldap.mdx +++ b/website/content/docs/auth/ldap.mdx @@ -156,7 +156,7 @@ Use `vault path-help` for more details. ### Other - `username_as_alias` (bool, optional) - If set to true, forces the auth method to use the username passed by the user as the alias name. - +- `max_page_size` (int, optional) - The maximum number of results to return for a single LDAP query. This is useful for preventing large queries from being run against the LDAP server. The default is the maximum value for an int32. ## Examples: