From c13977f1b86d438a3490864e68225425c865948a Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Tue, 19 Nov 2019 15:22:49 -0800 Subject: [PATCH 1/2] sdk/ldaputil: add request_timeout configuration option --- 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 ++ 6 files changed, 56 insertions(+), 16 deletions(-) diff --git a/builtin/credential/ldap/backend_test.go b/builtin/credential/ldap/backend_test.go index 3bc0b1d96c87..3119792abee8 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 f2b248318c69..5571565caa1d 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 ec9241982e78..548ada79f5a2 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 5a3972e37619..1ecceffc2775 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 0a52355e4b0c..40288cd02277 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 3e739267d186..71cc2ba2ea42 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 } From 5b18bb4c2f314f79dfb60730aa834d1e9716cf21 Mon Sep 17 00:00:00 2001 From: Calvin Leung Huang Date: Tue, 19 Nov 2019 17:10:06 -0800 Subject: [PATCH 2/2] go mod vendor --- .../hashicorp/vault/sdk/helper/ldaputil/client.go | 9 ++++++++- .../hashicorp/vault/sdk/helper/ldaputil/config.go | 11 +++++++++++ .../hashicorp/vault/sdk/helper/ldaputil/connection.go | 2 ++ .../hashicorp/vault/sdk/helper/tlsutil/tlsutil.go | 3 +++ 4 files changed, 24 insertions(+), 1 deletion(-) 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 ec9241982e78..548ada79f5a2 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 5a3972e37619..1ecceffc2775 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 3e739267d186..71cc2ba2ea42 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 236d32ec6748..1ead6e590b48 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