From bd248d684ba50c9f788ee6d652632ad686b159d6 Mon Sep 17 00:00:00 2001 From: hc-github-team-secure-vault-core <82990506+hc-github-team-secure-vault-core@users.noreply.github.com> Date: Thu, 13 Apr 2023 17:07:29 -0400 Subject: [PATCH] sdk/ldaputil: add connection_timeout configurable (#20144) (#20147) * sdk/ldaputil: add connection_timeout configurable * changelog * Update doc * Fix test * Change default to 30s Co-authored-by: Jason O'Donnell <2160810+jasonodonnell@users.noreply.github.com> --- builtin/credential/ldap/backend_test.go | 7 +++++++ changelog/20144.txt | 4 ++++ sdk/helper/ldaputil/client.go | 6 ++++++ sdk/helper/ldaputil/config.go | 12 ++++++++++++ sdk/helper/ldaputil/config_test.go | 21 ++++++++++++--------- website/content/api-docs/auth/ldap.mdx | 3 +++ website/content/api-docs/secret/ad.mdx | 1 + website/content/api-docs/secret/ldap.mdx | 3 +++ 8 files changed, 48 insertions(+), 9 deletions(-) create mode 100644 changelog/20144.txt diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index 74b4e18a17e3..820f117a0f0b 100644 --- a/builtin/credential/ldap/backend_test.go +++ b/builtin/credential/ldap/backend_test.go @@ -829,6 +829,7 @@ func testAccStepConfigUrl(t *testing.T, cfg *ldaputil.ConfigEntry) logicaltest.T "case_sensitive_names": true, "token_policies": "abc,xyz", "request_timeout": cfg.RequestTimeout, + "connection_timeout": cfg.ConnectionTimeout, "username_as_alias": cfg.UsernameAsAlias, }, } @@ -851,6 +852,7 @@ func testAccStepConfigUrlWithAuthBind(t *testing.T, cfg *ldaputil.ConfigEntry) l "case_sensitive_names": true, "token_policies": "abc,xyz", "request_timeout": cfg.RequestTimeout, + "connection_timeout": cfg.ConnectionTimeout, }, } } @@ -871,6 +873,7 @@ func testAccStepConfigUrlWithDiscover(t *testing.T, cfg *ldaputil.ConfigEntry) l "case_sensitive_names": true, "token_policies": "abc,xyz", "request_timeout": cfg.RequestTimeout, + "connection_timeout": cfg.ConnectionTimeout, }, } } @@ -888,6 +891,7 @@ func testAccStepConfigUrlNoGroupDN(t *testing.T, cfg *ldaputil.ConfigEntry) logi "discoverdn": true, "case_sensitive_names": true, "request_timeout": cfg.RequestTimeout, + "connection_timeout": cfg.ConnectionTimeout, }, } } @@ -908,6 +912,7 @@ func testAccStepConfigUrlWarningCheck(t *testing.T, cfg *ldaputil.ConfigEntry, o "case_sensitive_names": true, "token_policies": "abc,xyz", "request_timeout": cfg.RequestTimeout, + "connection_timeout": cfg.ConnectionTimeout, }, Check: func(response *logical.Response) error { if len(response.Warnings) == 0 { @@ -1189,6 +1194,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) { "token_period": "5m", "token_explicit_max_ttl": "24h", "request_timeout": cfg.RequestTimeout, + "connection_timeout": cfg.ConnectionTimeout, }, Storage: storage, Connection: &logical.Connection{}, @@ -1230,6 +1236,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) { CaseSensitiveNames: falseBool, UsePre111GroupCNBehavior: new(bool), RequestTimeout: cfg.RequestTimeout, + ConnectionTimeout: cfg.ConnectionTimeout, UsernameAsAlias: false, }, } diff --git a/changelog/20144.txt b/changelog/20144.txt new file mode 100644 index 000000000000..ef8b9a01810c --- /dev/null +++ b/changelog/20144.txt @@ -0,0 +1,4 @@ +```release-note:improvement +sdk/ldaputil: added `connection_timeout` to tune connection timeout duration +for all LDAP plugins. +``` diff --git a/sdk/helper/ldaputil/client.go b/sdk/helper/ldaputil/client.go index 8a7ac4822c34..4a7622d01ee3 100644 --- a/sdk/helper/ldaputil/client.go +++ b/sdk/helper/ldaputil/client.go @@ -28,6 +28,12 @@ func (c *Client) DialLDAP(cfg *ConfigEntry) (Connection, error) { var retErr *multierror.Error var conn Connection urls := strings.Split(cfg.Url, ",") + + // Default timeout in the pacakge is 60 seconds, which we default to on our + // end. This is useful if you want to take advantage of the URL list to increase + // availability of LDAP. + ldap.DefaultTimeout = time.Duration(cfg.ConnectionTimeout) * time.Second + for _, uut := range urls { u, err := url.Parse(uut) if err != nil { diff --git a/sdk/helper/ldaputil/config.go b/sdk/helper/ldaputil/config.go index 43844da22b13..e860893f7b00 100644 --- a/sdk/helper/ldaputil/config.go +++ b/sdk/helper/ldaputil/config.go @@ -226,6 +226,12 @@ Default: ({{.UserAttr}}={{.Username}})`, Description: "Timeout, in seconds, for the connection when making requests against the server before returning back an error.", Default: "90s", }, + + "connection_timeout": { + Type: framework.TypeDurationSecond, + Description: "Timeout, in seconds, when attempting to connect to the LDAP server before trying the next URL in the configuration.", + Default: "30s", + }, } } @@ -392,6 +398,10 @@ func NewConfigEntry(existing *ConfigEntry, d *framework.FieldData) (*ConfigEntry cfg.RequestTimeout = d.Get("request_timeout").(int) } + if _, ok := d.Raw["connection_timeout"]; ok || !hadExisting { + cfg.ConnectionTimeout = d.Get("connection_timeout").(int) + } + return cfg, nil } @@ -418,6 +428,7 @@ type ConfigEntry struct { UseTokenGroups bool `json:"use_token_groups"` UsePre111GroupCNBehavior *bool `json:"use_pre111_group_cn_behavior"` RequestTimeout int `json:"request_timeout"` + ConnectionTimeout int `json:"connection_timeout"` // 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. @@ -455,6 +466,7 @@ func (c *ConfigEntry) PasswordlessMap() map[string]interface{} { "use_token_groups": c.UseTokenGroups, "anonymous_group_search": c.AnonymousGroupSearch, "request_timeout": c.RequestTimeout, + "connection_timeout": c.ConnectionTimeout, "username_as_alias": c.UsernameAsAlias, } if c.CaseSensitiveNames != nil { diff --git a/sdk/helper/ldaputil/config_test.go b/sdk/helper/ldaputil/config_test.go index 32edb5dffaad..4e02fb757c8f 100644 --- a/sdk/helper/ldaputil/config_test.go +++ b/sdk/helper/ldaputil/config_test.go @@ -71,15 +71,16 @@ func testConfig(t *testing.T) *ConfigEntry { t.Helper() return &ConfigEntry{ - Url: "ldap://138.91.247.105", - UserDN: "example,com", - BindDN: "kitty", - BindPassword: "cats", - TLSMaxVersion: "tls12", - TLSMinVersion: "tls12", - RequestTimeout: 30, - ClientTLSCert: "", - ClientTLSKey: "", + Url: "ldap://138.91.247.105", + UserDN: "example,com", + BindDN: "kitty", + BindPassword: "cats", + TLSMaxVersion: "tls12", + TLSMinVersion: "tls12", + RequestTimeout: 30, + ConnectionTimeout: 15, + ClientTLSCert: "", + ClientTLSKey: "", } } @@ -138,6 +139,7 @@ var jsonConfig = []byte(`{ "tls_max_version": "tls12", "tls_min_version": "tls12", "request_timeout": 30, + "connection_timeout": 15, "ClientTLSCert": "", "ClientTLSKey": "" }`) @@ -168,6 +170,7 @@ var jsonConfigDefault = []byte(` "use_pre111_group_cn_behavior": null, "username_as_alias": false, "request_timeout": 90, + "connection_timeout": 30, "CaseSensitiveNames": false, "ClientTLSCert": "", "ClientTLSKey": "" diff --git a/website/content/api-docs/auth/ldap.mdx b/website/content/api-docs/auth/ldap.mdx index e28d0292fb0a..a1dd1c897efa 100644 --- a/website/content/api-docs/auth/ldap.mdx +++ b/website/content/api-docs/auth/ldap.mdx @@ -35,6 +35,9 @@ This endpoint configures the LDAP auth method. names will be normalized to lower case. Case will still be preserved when sending the username to the LDAP server at login time; this is only for matching local user/group definitions. +- `connection_timeout` `(integer: 30 or string: "30s")` - Timeout, in seconds, + when attempting to connect to the LDAP server before trying the next URL in + the configuration. - `request_timeout` `(integer: 90 or string: "90s")` - Timeout, in seconds, for the connection when making requests against the server before returning back an error. diff --git a/website/content/api-docs/secret/ad.mdx b/website/content/api-docs/secret/ad.mdx index c8e63630bf98..cac8265a384e 100644 --- a/website/content/api-docs/secret/ad.mdx +++ b/website/content/api-docs/secret/ad.mdx @@ -45,6 +45,7 @@ text that fulfills those requirements. `{{PASSWORD}}` must appear exactly once a ### Connection parameters - `url` (string, optional) - The LDAP server to connect to. Examples: `ldaps://ldap.myorg.com`, `ldaps://ldap.myorg.com:636`. This can also be a comma-delineated list of URLs, e.g. `ldaps://ldap.myorg.com,ldaps://ldap.myorg.com:636`, in which case the servers will be tried in-order if there are errors during the connection process. Default is `ldap://127.0.0.1`. +- `connection_timeout` `(integer: 30 or string: "30s")` - Timeout, in seconds, when attempting to connect to the LDAP server before trying the next URL in the configuration. - `request_timeout` `(integer: 90 or string: "90s")` - Timeout, in seconds, for the connection when making requests against the server before returning back an error. - `starttls` (bool, optional) - If true, issues a `StartTLS` command after establishing an unencrypted connection. - `insecure_tls` - (bool, optional) - If true, skips LDAP server SSL certificate verification - insecure, use with caution! diff --git a/website/content/api-docs/secret/ldap.mdx b/website/content/api-docs/secret/ldap.mdx index 544484cd844a..9868fdf67d2d 100644 --- a/website/content/api-docs/secret/ldap.mdx +++ b/website/content/api-docs/secret/ldap.mdx @@ -53,6 +53,9 @@ to search and change entry passwords in LDAP. string for authentication. The constructed UPN will appear as `[binddn]@[upndomain]`. For example, if `upndomain=example.com` and `binddn=admin`, the UPN string `admin@example.com` will be used to log in to Active Directory. +- `connection_timeout` `(integer: 30 or string: "30s")` - Timeout, in seconds, + when attempting to connect to the LDAP server before trying the next URL in + the configuration. - `request_timeout` `(integer: 90, string: "90s" )` - Timeout, in seconds, for the connection when making requests against the server before returning back an error. - `starttls` `(bool: )` - If true, issues a `StartTLS` command after establishing an unencrypted connection.