From 7478b5d3fe1659462fd34a9e6a76b6484a358b8b Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 29 Jul 2019 14:45:46 -0400 Subject: [PATCH 1/3] Add LDAP parameter to allow old-style group CN matching Tested manually; the online test server we use doesn't have suitable groups for being able to exercise this. --- builtin/credential/ldap/path_config.go | 14 ++++++++ helper/ldaputil/client.go | 16 ++++++--- helper/ldaputil/config.go | 49 +++++++++++++++++--------- 3 files changed, 57 insertions(+), 22 deletions(-) diff --git a/builtin/credential/ldap/path_config.go b/builtin/credential/ldap/path_config.go index f2776b1a0366..7ccfedc6db04 100644 --- a/builtin/credential/ldap/path_config.go +++ b/builtin/credential/ldap/path_config.go @@ -50,6 +50,9 @@ func (b *backend) Config(ctx context.Context, req *logical.Request) (*ldaputil.C result.CaseSensitiveNames = new(bool) *result.CaseSensitiveNames = false + result.UseDeprecatedGroupCNBehavior = new(bool) + *result.UseDeprecatedGroupCNBehavior = false + return result, nil } @@ -67,6 +70,12 @@ func (b *backend) Config(ctx context.Context, req *logical.Request) (*ldaputil.C persistNeeded = true } + if result.UseDeprecatedGroupCNBehavior == nil { + result.UseDeprecatedGroupCNBehavior = new(bool) + *result.UseDeprecatedGroupCNBehavior = true + persistNeeded = true + } + if persistNeeded && (b.System().LocalMount() || !b.System().ReplicationState().HasState(consts.ReplicationPerformanceSecondary|consts.ReplicationPerformanceStandby)) { entry, err := logical.StorageEntryJSON("config", result) if err != nil { @@ -109,6 +118,11 @@ func (b *backend) pathConfigWrite(ctx context.Context, req *logical.Request, d * *cfg.CaseSensitiveNames = false } + if cfg.UseDeprecatedGroupCNBehavior == nil { + cfg.UseDeprecatedGroupCNBehavior = new(bool) + *cfg.UseDeprecatedGroupCNBehavior = false + } + entry, err := logical.StorageEntryJSON("config", cfg) if err != nil { return nil, err diff --git a/helper/ldaputil/client.go b/helper/ldaputil/client.go index f117c6060687..a1d3dc23d948 100644 --- a/helper/ldaputil/client.go +++ b/helper/ldaputil/client.go @@ -357,12 +357,12 @@ func (c *Client) GetLdapGroups(cfg *ConfigEntry, conn Connection, userDN string, values := e.GetAttributeValues(cfg.GroupAttr) if len(values) > 0 { for _, val := range values { - groupCN := getCN(val) + groupCN := getCN(cfg, val) ldapMap[groupCN] = true } } else { // If groupattr didn't resolve, use self (enumerating group objects) - groupCN := getCN(e.DN) + groupCN := getCN(cfg, e.DN) ldapMap[groupCN] = true } } @@ -419,7 +419,7 @@ func EscapeLDAPValue(input string) string { * Given a non-conforming string (such as an already-extracted CN), * it will be returned as-is. */ -func getCN(dn string) string { +func getCN(cfg *ConfigEntry, dn string) string { parsedDN, err := ldap.ParseDN(dn) if err != nil || len(parsedDN.RDNs) == 0 { // It was already a CN, return as-is @@ -428,8 +428,14 @@ func getCN(dn string) string { for _, rdn := range parsedDN.RDNs { for _, rdnAttr := range rdn.Attributes { - if strings.EqualFold(rdnAttr.Type, "CN") { - return rdnAttr.Value + if cfg.UseDeprecatedGroupCNBehavior == nil || *cfg.UseDeprecatedGroupCNBehavior { + if rdnAttr.Type == "CN" { + return rdnAttr.Value + } + } else { + if strings.EqualFold(rdnAttr.Type, "CN") { + return rdnAttr.Value + } } } } diff --git a/helper/ldaputil/config.go b/helper/ldaputil/config.go index 7169a8c82b4a..12b2a3486892 100644 --- a/helper/ldaputil/config.go +++ b/helper/ldaputil/config.go @@ -136,6 +136,11 @@ Default: cn`, Default: false, Description: "If true, use the Active Directory tokenGroups constructed attribute of the user to find the group memberships. This will find all security groups including nested ones.", }, + + "use_deprecated_group_cn_behavior": { + Type: framework.TypeBool, + Description: "If true, will revert to Vault <= 1.1.0 behavior for matching group CNs, which would only match if the attribute was uppercase 'CN', not lowrecase 'cn'. 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.", + }, } } @@ -252,6 +257,12 @@ func NewConfigEntry(d *framework.FieldData) (*ConfigEntry, error) { *cfg.CaseSensitiveNames = caseSensitiveNames.(bool) } + useDeprecatedGroupCNBehavior, ok := d.GetOk("use_deprecated_group_cn_behavior") + if ok { + cfg.UseDeprecatedGroupCNBehavior = new(bool) + *cfg.UseDeprecatedGroupCNBehavior = useDeprecatedGroupCNBehavior.(bool) + } + useTokenGroups := d.Get("use_token_groups").(bool) if useTokenGroups { cfg.UseTokenGroups = useTokenGroups @@ -261,23 +272,24 @@ func NewConfigEntry(d *framework.FieldData) (*ConfigEntry, error) { } type ConfigEntry struct { - Url string `json:"url"` - UserDN string `json:"userdn"` - GroupDN string `json:"groupdn"` - GroupFilter string `json:"groupfilter"` - GroupAttr string `json:"groupattr"` - UPNDomain string `json:"upndomain"` - UserAttr string `json:"userattr"` - Certificate string `json:"certificate"` - InsecureTLS bool `json:"insecure_tls"` - StartTLS bool `json:"starttls"` - BindDN string `json:"binddn"` - BindPassword string `json:"bindpass"` - DenyNullBind bool `json:"deny_null_bind"` - DiscoverDN bool `json:"discoverdn"` - TLSMinVersion string `json:"tls_min_version"` - TLSMaxVersion string `json:"tls_max_version"` - UseTokenGroups bool `json:"use_token_groups"` + Url string `json:"url"` + UserDN string `json:"userdn"` + GroupDN string `json:"groupdn"` + GroupFilter string `json:"groupfilter"` + GroupAttr string `json:"groupattr"` + UPNDomain string `json:"upndomain"` + UserAttr string `json:"userattr"` + Certificate string `json:"certificate"` + InsecureTLS bool `json:"insecure_tls"` + StartTLS bool `json:"starttls"` + BindDN string `json:"binddn"` + BindPassword string `json:"bindpass"` + DenyNullBind bool `json:"deny_null_bind"` + DiscoverDN bool `json:"discoverdn"` + TLSMinVersion string `json:"tls_min_version"` + TLSMaxVersion string `json:"tls_max_version"` + UseTokenGroups bool `json:"use_token_groups"` + UseDeprecatedGroupCNBehavior *bool `json:"use_deprecated_group_cn_behavior"` // 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". @@ -314,6 +326,9 @@ func (c *ConfigEntry) PasswordlessMap() map[string]interface{} { if c.CaseSensitiveNames != nil { m["case_sensitive_names"] = *c.CaseSensitiveNames } + if c.UseDeprecatedGroupCNBehavior != nil { + m["use_deprecated_group_cn_behavior"] = *c.UseDeprecatedGroupCNBehavior + } return m } From dcd157fd34a0ce0de80932aba5348053dce9f499 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 29 Jul 2019 14:54:42 -0400 Subject: [PATCH 2/3] Fix speeling --- helper/ldaputil/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/helper/ldaputil/config.go b/helper/ldaputil/config.go index 12b2a3486892..50f84c6e39e1 100644 --- a/helper/ldaputil/config.go +++ b/helper/ldaputil/config.go @@ -139,7 +139,7 @@ Default: cn`, "use_deprecated_group_cn_behavior": { Type: framework.TypeBool, - Description: "If true, will revert to Vault <= 1.1.0 behavior for matching group CNs, which would only match if the attribute was uppercase 'CN', not lowrecase 'cn'. 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.", + Description: "If true, will revert to Vault <= 1.1.0 behavior for matching group CNs, which would only match if the attribute was uppercase 'CN', not lowercase 'cn'. 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.", }, } } From 7871c8d2197139ed7b81dbf1a85c4ed12c49a070 Mon Sep 17 00:00:00 2001 From: Jeff Mitchell Date: Mon, 29 Jul 2019 15:29:15 -0400 Subject: [PATCH 3/3] Change name of param --- builtin/credential/ldap/path_config.go | 16 ++++----- helper/ldaputil/client.go | 2 +- helper/ldaputil/config.go | 50 +++++++++++++------------- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/builtin/credential/ldap/path_config.go b/builtin/credential/ldap/path_config.go index 7ccfedc6db04..e426ca3bbb84 100644 --- a/builtin/credential/ldap/path_config.go +++ b/builtin/credential/ldap/path_config.go @@ -50,8 +50,8 @@ func (b *backend) Config(ctx context.Context, req *logical.Request) (*ldaputil.C result.CaseSensitiveNames = new(bool) *result.CaseSensitiveNames = false - result.UseDeprecatedGroupCNBehavior = new(bool) - *result.UseDeprecatedGroupCNBehavior = false + result.UsePre111GroupCNBehavior = new(bool) + *result.UsePre111GroupCNBehavior = false return result, nil } @@ -70,9 +70,9 @@ func (b *backend) Config(ctx context.Context, req *logical.Request) (*ldaputil.C persistNeeded = true } - if result.UseDeprecatedGroupCNBehavior == nil { - result.UseDeprecatedGroupCNBehavior = new(bool) - *result.UseDeprecatedGroupCNBehavior = true + if result.UsePre111GroupCNBehavior == nil { + result.UsePre111GroupCNBehavior = new(bool) + *result.UsePre111GroupCNBehavior = true persistNeeded = true } @@ -118,9 +118,9 @@ func (b *backend) pathConfigWrite(ctx context.Context, req *logical.Request, d * *cfg.CaseSensitiveNames = false } - if cfg.UseDeprecatedGroupCNBehavior == nil { - cfg.UseDeprecatedGroupCNBehavior = new(bool) - *cfg.UseDeprecatedGroupCNBehavior = false + if cfg.UsePre111GroupCNBehavior == nil { + cfg.UsePre111GroupCNBehavior = new(bool) + *cfg.UsePre111GroupCNBehavior = false } entry, err := logical.StorageEntryJSON("config", cfg) diff --git a/helper/ldaputil/client.go b/helper/ldaputil/client.go index a1d3dc23d948..2ba724929b4c 100644 --- a/helper/ldaputil/client.go +++ b/helper/ldaputil/client.go @@ -428,7 +428,7 @@ func getCN(cfg *ConfigEntry, dn string) string { for _, rdn := range parsedDN.RDNs { for _, rdnAttr := range rdn.Attributes { - if cfg.UseDeprecatedGroupCNBehavior == nil || *cfg.UseDeprecatedGroupCNBehavior { + if cfg.UsePre111GroupCNBehavior == nil || *cfg.UsePre111GroupCNBehavior { if rdnAttr.Type == "CN" { return rdnAttr.Value } diff --git a/helper/ldaputil/config.go b/helper/ldaputil/config.go index 50f84c6e39e1..46341b912d37 100644 --- a/helper/ldaputil/config.go +++ b/helper/ldaputil/config.go @@ -137,9 +137,9 @@ Default: cn`, Description: "If true, use the Active Directory tokenGroups constructed attribute of the user to find the group memberships. This will find all security groups including nested ones.", }, - "use_deprecated_group_cn_behavior": { + "use_pre111_group_cn_behavior": { Type: framework.TypeBool, - Description: "If true, will revert to Vault <= 1.1.0 behavior for matching group CNs, which would only match if the attribute was uppercase 'CN', not lowercase 'cn'. 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.", + 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.", }, } } @@ -257,10 +257,10 @@ func NewConfigEntry(d *framework.FieldData) (*ConfigEntry, error) { *cfg.CaseSensitiveNames = caseSensitiveNames.(bool) } - useDeprecatedGroupCNBehavior, ok := d.GetOk("use_deprecated_group_cn_behavior") + usePre111GroupCNBehavior, ok := d.GetOk("use_pre111_group_cn_behavior") if ok { - cfg.UseDeprecatedGroupCNBehavior = new(bool) - *cfg.UseDeprecatedGroupCNBehavior = useDeprecatedGroupCNBehavior.(bool) + cfg.UsePre111GroupCNBehavior = new(bool) + *cfg.UsePre111GroupCNBehavior = usePre111GroupCNBehavior.(bool) } useTokenGroups := d.Get("use_token_groups").(bool) @@ -272,24 +272,24 @@ func NewConfigEntry(d *framework.FieldData) (*ConfigEntry, error) { } type ConfigEntry struct { - Url string `json:"url"` - UserDN string `json:"userdn"` - GroupDN string `json:"groupdn"` - GroupFilter string `json:"groupfilter"` - GroupAttr string `json:"groupattr"` - UPNDomain string `json:"upndomain"` - UserAttr string `json:"userattr"` - Certificate string `json:"certificate"` - InsecureTLS bool `json:"insecure_tls"` - StartTLS bool `json:"starttls"` - BindDN string `json:"binddn"` - BindPassword string `json:"bindpass"` - DenyNullBind bool `json:"deny_null_bind"` - DiscoverDN bool `json:"discoverdn"` - TLSMinVersion string `json:"tls_min_version"` - TLSMaxVersion string `json:"tls_max_version"` - UseTokenGroups bool `json:"use_token_groups"` - UseDeprecatedGroupCNBehavior *bool `json:"use_deprecated_group_cn_behavior"` + Url string `json:"url"` + UserDN string `json:"userdn"` + GroupDN string `json:"groupdn"` + GroupFilter string `json:"groupfilter"` + GroupAttr string `json:"groupattr"` + UPNDomain string `json:"upndomain"` + UserAttr string `json:"userattr"` + Certificate string `json:"certificate"` + InsecureTLS bool `json:"insecure_tls"` + StartTLS bool `json:"starttls"` + BindDN string `json:"binddn"` + BindPassword string `json:"bindpass"` + DenyNullBind bool `json:"deny_null_bind"` + DiscoverDN bool `json:"discoverdn"` + TLSMinVersion string `json:"tls_min_version"` + TLSMaxVersion string `json:"tls_max_version"` + UseTokenGroups bool `json:"use_token_groups"` + UsePre111GroupCNBehavior *bool `json:"use_pre111_group_cn_behavior"` // 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". @@ -326,8 +326,8 @@ func (c *ConfigEntry) PasswordlessMap() map[string]interface{} { if c.CaseSensitiveNames != nil { m["case_sensitive_names"] = *c.CaseSensitiveNames } - if c.UseDeprecatedGroupCNBehavior != nil { - m["use_deprecated_group_cn_behavior"] = *c.UseDeprecatedGroupCNBehavior + if c.UsePre111GroupCNBehavior != nil { + m["use_pre111_group_cn_behavior"] = *c.UsePre111GroupCNBehavior } return m }