From 6191cfaf917fe943e3a5ebb2b5ce1d084f6ae85e Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Wed, 20 Nov 2019 11:26:13 -0800 Subject: [PATCH] sdk/ldaputil: add request_timeout configuration option (#7909) * sdk/ldaputil: add request_timeout configuration option * go mod vendor --- builtin/credential/ldap/backend_test.go | 6 +++ helper/testhelpers/ldap/ldaphelper.go | 1 + sdk/helper/ldaputil/client.go | 9 +++- sdk/helper/ldaputil/config.go | 11 +++++ sdk/helper/ldaputil/config_test.go | 43 ++++++++++++------- sdk/helper/ldaputil/connection.go | 2 + .../vault/sdk/helper/ldaputil/client.go | 9 +++- .../vault/sdk/helper/ldaputil/config.go | 11 +++++ .../vault/sdk/helper/ldaputil/connection.go | 2 + .../vault/sdk/helper/tlsutil/tlsutil.go | 3 ++ 10 files changed, 80 insertions(+), 17 deletions(-) diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index 3bc0b1d96c..3119792abe 100644 --- a/builtin/credential/ldap/backend_test.go +++ b/builtin/credential/ldap/backend_test.go @@ -608,6 +608,7 @@ func testAccStepConfigUrl(t *testing.T, cfg *ldaputil.ConfigEntry) logicaltest.T "bindpass": cfg.BindPassword, "case_sensitive_names": true, "token_policies": "abc,xyz", + "request_timeout": cfg.RequestTimeout, }, } } @@ -627,6 +628,7 @@ func testAccStepConfigUrlWithAuthBind(t *testing.T, cfg *ldaputil.ConfigEntry) l "bindpass": cfg.BindPassword, "case_sensitive_names": true, "token_policies": "abc,xyz", + "request_timeout": cfg.RequestTimeout, }, } } @@ -646,6 +648,7 @@ func testAccStepConfigUrlWithDiscover(t *testing.T, cfg *ldaputil.ConfigEntry) l "discoverdn": true, "case_sensitive_names": true, "token_policies": "abc,xyz", + "request_timeout": cfg.RequestTimeout, }, } } @@ -662,6 +665,7 @@ func testAccStepConfigUrlNoGroupDN(t *testing.T, cfg *ldaputil.ConfigEntry) logi "bindpass": cfg.BindPassword, "discoverdn": true, "case_sensitive_names": true, + "request_timeout": cfg.RequestTimeout, }, } } @@ -891,6 +895,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) { "bindpass": cfg.BindPassword, "token_period": "5m", "token_explicit_max_ttl": "24h", + "request_timeout": cfg.RequestTimeout, }, Storage: storage, Connection: &logical.Connection{}, @@ -930,6 +935,7 @@ func TestLdapAuthBackend_ConfigUpgrade(t *testing.T) { TLSMaxVersion: defParams.TLSMaxVersion, CaseSensitiveNames: falseBool, UsePre111GroupCNBehavior: new(bool), + RequestTimeout: cfg.RequestTimeout, }, } diff --git a/helper/testhelpers/ldap/ldaphelper.go b/helper/testhelpers/ldap/ldaphelper.go index f2b248318c..5571565caa 100644 --- a/helper/testhelpers/ldap/ldaphelper.go +++ b/helper/testhelpers/ldap/ldaphelper.go @@ -48,6 +48,7 @@ func PrepareTestContainer(t *testing.T, version string) (cleanup func(), cfg *ld cfg.BindPassword = "GoodNewsEveryone" cfg.GroupDN = "ou=people,dc=planetexpress,dc=com" cfg.GroupAttr = "memberOf" + cfg.RequestTimeout = 60 conn, err := client.DialLDAP(cfg) if err != nil { return err diff --git a/sdk/helper/ldaputil/client.go b/sdk/helper/ldaputil/client.go index ec9241982e..548ada79f5 100644 --- a/sdk/helper/ldaputil/client.go +++ b/sdk/helper/ldaputil/client.go @@ -11,6 +11,7 @@ import ( "net/url" "strings" "text/template" + "time" "github.com/go-ldap/ldap/v3" "github.com/hashicorp/errwrap" @@ -85,6 +86,10 @@ func (c *Client) DialLDAP(cfg *ConfigEntry) (Connection, error) { retErr = multierror.Append(retErr, errwrap.Wrapf(fmt.Sprintf("error connecting to host %q: {{err}}", uut), err)) } + if timeout := cfg.RequestTimeout; timeout > 0 { + conn.SetTimeout(time.Duration(timeout) * time.Second) + } + return conn, retErr.ErrorOrNil() } @@ -205,7 +210,9 @@ func (c *Client) performLdapFilterGroupsSearch(cfg *ConfigEntry, conn Connection } var renderedQuery bytes.Buffer - t.Execute(&renderedQuery, context) + if err := t.Execute(&renderedQuery, context); err != nil { + return nil, errwrap.Wrapf("LDAP search failed due to template parsing error: {{error}}", err) + } if c.Logger.IsDebug() { c.Logger.Debug("searching", "groupdn", cfg.GroupDN, "rendered_query", renderedQuery.String()) diff --git a/sdk/helper/ldaputil/config.go b/sdk/helper/ldaputil/config.go index 5a3972e376..1ecceffc27 100644 --- a/sdk/helper/ldaputil/config.go +++ b/sdk/helper/ldaputil/config.go @@ -172,6 +172,12 @@ Default: cn`, Type: framework.TypeBool, Description: "In Vault 1.1.1 a fix for handling group CN values of different cases unfortunately introduced a regression that could cause previously defined groups to not be found due to a change in the resulting name. If set true, the pre-1.1.1 behavior for matching group CNs will be used. This is only needed in some upgrade scenarios for backwards compatibility. It is enabled by default if the config is upgraded but disabled by default on new configurations.", }, + + "request_timeout": { + Type: framework.TypeDurationSecond, + Description: "Timeout, in seconds, for the connection when making requests against the server before returning back an error.", + Default: "90s", + }, } } @@ -302,6 +308,10 @@ func NewConfigEntry(existing *ConfigEntry, d *framework.FieldData) (*ConfigEntry cfg.UseTokenGroups = d.Get("use_token_groups").(bool) } + if _, ok := d.Raw["request_timeout"]; ok || !hadExisting { + cfg.RequestTimeout = d.Get("request_timeout").(int) + } + return cfg, nil } @@ -324,6 +334,7 @@ type ConfigEntry struct { TLSMaxVersion string `json:"tls_max_version"` UseTokenGroups bool `json:"use_token_groups"` UsePre111GroupCNBehavior *bool `json:"use_pre111_group_cn_behavior"` + RequestTimeout int `json:"request_timeout"` // This json tag deviates from snake case because there was a past issue // where the tag was being ignored, causing it to be jsonified as "CaseSensitiveNames". diff --git a/sdk/helper/ldaputil/config_test.go b/sdk/helper/ldaputil/config_test.go index 0a52355e4b..40288cd022 100644 --- a/sdk/helper/ldaputil/config_test.go +++ b/sdk/helper/ldaputil/config_test.go @@ -3,6 +3,8 @@ package ldaputil import ( "encoding/json" "testing" + + "github.com/go-test/deep" ) func TestCertificateValidation(t *testing.T) { @@ -28,26 +30,36 @@ func TestCertificateValidation(t *testing.T) { } } -func TestUseTokenGroupsDefault(t *testing.T) { +func TestConfig(t *testing.T) { config := testConfig() - if config.UseTokenGroups { - t.Errorf("expected false UseTokenGroups but got %t", config.UseTokenGroups) - } + configFromJSON := testJSONConfig(t) - config = testJSONConfig(t) - if config.UseTokenGroups { - t.Errorf("expected false UseTokenGroups from JSON but got %t", config.UseTokenGroups) - } + t.Run("equality_check", func(t *testing.T) { + if diff := deep.Equal(config, configFromJSON); len(diff) > 0 { + t.Fatalf("bad, diff: %#v", diff) + } + }) + + t.Run("default_use_token_groups", func(t *testing.T) { + if config.UseTokenGroups { + t.Errorf("expected false UseTokenGroups but got %t", config.UseTokenGroups) + } + + if configFromJSON.UseTokenGroups { + t.Errorf("expected false UseTokenGroups from JSON but got %t", configFromJSON.UseTokenGroups) + } + }) } func testConfig() *ConfigEntry { return &ConfigEntry{ - Url: "ldap://138.91.247.105", - UserDN: "example,com", - BindDN: "kitty", - BindPassword: "cats", - TLSMaxVersion: "tls12", - TLSMinVersion: "tls12", + Url: "ldap://138.91.247.105", + UserDN: "example,com", + BindDN: "kitty", + BindPassword: "cats", + TLSMaxVersion: "tls12", + TLSMinVersion: "tls12", + RequestTimeout: 30, } } @@ -103,6 +115,7 @@ var jsonConfig = []byte(` "binddn": "kitty", "bindpass": "cats", "tls_max_version": "tls12", - "tls_min_version": "tls12" + "tls_min_version": "tls12", + "request_timeout": 30 } `) diff --git a/sdk/helper/ldaputil/connection.go b/sdk/helper/ldaputil/connection.go index 3e739267d1..71cc2ba2ea 100644 --- a/sdk/helper/ldaputil/connection.go +++ b/sdk/helper/ldaputil/connection.go @@ -2,6 +2,7 @@ package ldaputil import ( "crypto/tls" + "time" "github.com/go-ldap/ldap/v3" ) @@ -14,5 +15,6 @@ type Connection interface { Modify(modifyRequest *ldap.ModifyRequest) error Search(searchRequest *ldap.SearchRequest) (*ldap.SearchResult, error) StartTLS(config *tls.Config) error + SetTimeout(timeout time.Duration) UnauthenticatedBind(username string) error } diff --git a/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/client.go b/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/client.go index ec9241982e..548ada79f5 100644 --- a/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/client.go +++ b/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/client.go @@ -11,6 +11,7 @@ import ( "net/url" "strings" "text/template" + "time" "github.com/go-ldap/ldap/v3" "github.com/hashicorp/errwrap" @@ -85,6 +86,10 @@ func (c *Client) DialLDAP(cfg *ConfigEntry) (Connection, error) { retErr = multierror.Append(retErr, errwrap.Wrapf(fmt.Sprintf("error connecting to host %q: {{err}}", uut), err)) } + if timeout := cfg.RequestTimeout; timeout > 0 { + conn.SetTimeout(time.Duration(timeout) * time.Second) + } + return conn, retErr.ErrorOrNil() } @@ -205,7 +210,9 @@ func (c *Client) performLdapFilterGroupsSearch(cfg *ConfigEntry, conn Connection } var renderedQuery bytes.Buffer - t.Execute(&renderedQuery, context) + if err := t.Execute(&renderedQuery, context); err != nil { + return nil, errwrap.Wrapf("LDAP search failed due to template parsing error: {{error}}", err) + } if c.Logger.IsDebug() { c.Logger.Debug("searching", "groupdn", cfg.GroupDN, "rendered_query", renderedQuery.String()) diff --git a/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/config.go b/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/config.go index 5a3972e376..1ecceffc27 100644 --- a/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/config.go +++ b/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/config.go @@ -172,6 +172,12 @@ Default: cn`, Type: framework.TypeBool, Description: "In Vault 1.1.1 a fix for handling group CN values of different cases unfortunately introduced a regression that could cause previously defined groups to not be found due to a change in the resulting name. If set true, the pre-1.1.1 behavior for matching group CNs will be used. This is only needed in some upgrade scenarios for backwards compatibility. It is enabled by default if the config is upgraded but disabled by default on new configurations.", }, + + "request_timeout": { + Type: framework.TypeDurationSecond, + Description: "Timeout, in seconds, for the connection when making requests against the server before returning back an error.", + Default: "90s", + }, } } @@ -302,6 +308,10 @@ func NewConfigEntry(existing *ConfigEntry, d *framework.FieldData) (*ConfigEntry cfg.UseTokenGroups = d.Get("use_token_groups").(bool) } + if _, ok := d.Raw["request_timeout"]; ok || !hadExisting { + cfg.RequestTimeout = d.Get("request_timeout").(int) + } + return cfg, nil } @@ -324,6 +334,7 @@ type ConfigEntry struct { TLSMaxVersion string `json:"tls_max_version"` UseTokenGroups bool `json:"use_token_groups"` UsePre111GroupCNBehavior *bool `json:"use_pre111_group_cn_behavior"` + RequestTimeout int `json:"request_timeout"` // This json tag deviates from snake case because there was a past issue // where the tag was being ignored, causing it to be jsonified as "CaseSensitiveNames". diff --git a/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/connection.go b/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/connection.go index 3e739267d1..71cc2ba2ea 100644 --- a/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/connection.go +++ b/vendor/github.com/hashicorp/vault/sdk/helper/ldaputil/connection.go @@ -2,6 +2,7 @@ package ldaputil import ( "crypto/tls" + "time" "github.com/go-ldap/ldap/v3" ) @@ -14,5 +15,6 @@ type Connection interface { Modify(modifyRequest *ldap.ModifyRequest) error Search(searchRequest *ldap.SearchRequest) (*ldap.SearchResult, error) StartTLS(config *tls.Config) error + SetTimeout(timeout time.Duration) UnauthenticatedBind(username string) error } diff --git a/vendor/github.com/hashicorp/vault/sdk/helper/tlsutil/tlsutil.go b/vendor/github.com/hashicorp/vault/sdk/helper/tlsutil/tlsutil.go index 236d32ec67..1ead6e590b 100644 --- a/vendor/github.com/hashicorp/vault/sdk/helper/tlsutil/tlsutil.go +++ b/vendor/github.com/hashicorp/vault/sdk/helper/tlsutil/tlsutil.go @@ -42,6 +42,9 @@ var cipherMap = map[string]uint16{ "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384": tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305": tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305, "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305": tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305, + "TLS_AES_128_GCM_SHA256": tls.TLS_AES_128_GCM_SHA256, + "TLS_AES_256_GCM_SHA384": tls.TLS_AES_256_GCM_SHA384, + "TLS_CHACHA20_POLY1305_SHA256": tls.TLS_CHACHA20_POLY1305_SHA256, } // ParseCiphers parse ciphersuites from the comma-separated string into recognized slice