From d2251d2ea7a3ed4404482300ac80bade0d88ea67 Mon Sep 17 00:00:00 2001 From: Ryan Richard Date: Thu, 27 May 2021 13:47:10 -0700 Subject: [PATCH] Use base64 binary-encoded value as UID for LDAP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is to allow the use of binary LDAP entry attributes as the UID. For example, a user might like to configure AD’s objectGUID or maybe objectSid attributes as the UID attribute. This negatively impacts the readability of the UID when it did not come from a binary value, but we're considering this an okay trade-off to keep things simple for now. In the future, we may offer more customizable encoding options for binary attributes. These UIDs are currently only used in the downstream OIDC `sub` claim. They do not effect the user's identity on the Kubernetes cluster, which is only based on their mapped username and group memberships from the upstream identity provider. We are not currently supporting any special encoding for those username and group name LDAP attributes, so their values in the LDAP entry must be ASCII or UTF-8 in order for them to be interpreted correctly. --- internal/upstreamldap/upstreamldap.go | 29 ++++++++++++- internal/upstreamldap/upstreamldap_test.go | 7 ++-- test/integration/ldap_client_test.go | 49 +++++++++++++--------- test/integration/supervisor_login_test.go | 8 +++- 4 files changed, 67 insertions(+), 26 deletions(-) diff --git a/internal/upstreamldap/upstreamldap.go b/internal/upstreamldap/upstreamldap.go index a7732261..11f800eb 100644 --- a/internal/upstreamldap/upstreamldap.go +++ b/internal/upstreamldap/upstreamldap.go @@ -8,6 +8,7 @@ import ( "context" "crypto/tls" "crypto/x509" + "encoding/base64" "errors" "fmt" "net" @@ -417,7 +418,9 @@ func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(c return "", "", nil, err } - mappedUID, err := p.getSearchResultAttributeValue(p.c.UserSearch.UIDAttribute, userEntry, username) + // We would like to support binary typed attributes for UIDs, so always read them as binary and encode them, + // even when the attribute may not be binary. + mappedUID, err := p.getSearchResultAttributeRawValueEncoded(p.c.UserSearch.UIDAttribute, userEntry, username) if err != nil { return "", "", nil, err } @@ -526,6 +529,30 @@ func (p *Provider) escapeUsernameForSearchFilter(username string) string { return ldap.EscapeFilter(username) } +// Returns the (potentially) binary data of the attribute's value, base64 URL encoded. +func (p *Provider) getSearchResultAttributeRawValueEncoded(attributeName string, entry *ldap.Entry, username string) (string, error) { + if attributeName == distinguishedNameAttributeName { + return base64.RawURLEncoding.EncodeToString([]byte(entry.DN)), nil + } + + attributeValues := entry.GetRawAttributeValues(attributeName) + + if len(attributeValues) != 1 { + return "", fmt.Errorf(`found %d values for attribute "%s" while searching for user "%s", but expected 1 result`, + len(attributeValues), attributeName, username, + ) + } + + attributeValue := attributeValues[0] + if len(attributeValue) == 0 { + return "", fmt.Errorf(`found empty value for attribute "%s" while searching for user "%s", but expected value to be non-empty`, + attributeName, username, + ) + } + + return base64.RawURLEncoding.EncodeToString(attributeValue), nil +} + func (p *Provider) getSearchResultAttributeValue(attributeName string, entry *ldap.Entry, username string) (string, error) { if attributeName == distinguishedNameAttributeName { return entry.DN, nil diff --git a/internal/upstreamldap/upstreamldap_test.go b/internal/upstreamldap/upstreamldap_test.go index c9e88be3..35609bcf 100644 --- a/internal/upstreamldap/upstreamldap_test.go +++ b/internal/upstreamldap/upstreamldap_test.go @@ -6,6 +6,7 @@ package upstreamldap import ( "context" "crypto/tls" + "encoding/base64" "errors" "fmt" "net" @@ -153,7 +154,7 @@ func TestEndUserAuthentication(t *testing.T) { expectedAuthResponse := func(editFunc func(r *user.DefaultInfo)) *authenticator.Response { u := &user.DefaultInfo{ Name: testUserSearchResultUsernameAttributeValue, - UID: testUserSearchResultUIDAttributeValue, + UID: base64.RawURLEncoding.EncodeToString([]byte(testUserSearchResultUIDAttributeValue)), Groups: []string{testGroupSearchResultGroupNameAttributeValue1, testGroupSearchResultGroupNameAttributeValue2}, } if editFunc != nil { @@ -311,7 +312,7 @@ func TestEndUserAuthentication(t *testing.T) { conn.EXPECT().Bind(testUserSearchResultDNValue, testUpstreamPassword).Times(1) }, wantAuthResponse: expectedAuthResponse(func(r *user.DefaultInfo) { - r.UID = testUserSearchResultDNValue + r.UID = base64.RawURLEncoding.EncodeToString([]byte(testUserSearchResultDNValue)) }), }, { @@ -477,7 +478,7 @@ func TestEndUserAuthentication(t *testing.T) { wantAuthResponse: &authenticator.Response{ User: &user.DefaultInfo{ Name: testUserSearchResultUsernameAttributeValue, - UID: testUserSearchResultUIDAttributeValue, + UID: base64.RawURLEncoding.EncodeToString([]byte(testUserSearchResultUIDAttributeValue)), Groups: []string{"a", "b", "c"}, }, }, diff --git a/test/integration/ldap_client_test.go b/test/integration/ldap_client_test.go index e95173f1..5e4735c3 100644 --- a/test/integration/ldap_client_test.go +++ b/test/integration/ldap_client_test.go @@ -5,6 +5,7 @@ package integration import ( "context" + "encoding/base64" "fmt" "io" "net" @@ -58,6 +59,10 @@ func TestLDAPSearch(t *testing.T) { pinnyPassword := env.SupervisorUpstreamLDAP.TestUserPassword + b64 := func(s string) string { + return base64.RawURLEncoding.EncodeToString([]byte(s)) + } + tests := []struct { name string username string @@ -73,7 +78,7 @@ func TestLDAPSearch(t *testing.T) { password: pinnyPassword, provider: upstreamldap.New(*providerConfig(nil)), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"ball-game-players", "seals"}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, }, }, { @@ -85,7 +90,7 @@ func TestLDAPSearch(t *testing.T) { p.ConnectionProtocol = upstreamldap.StartTLS })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"ball-game-players", "seals"}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, }, }, { @@ -94,7 +99,7 @@ func TestLDAPSearch(t *testing.T) { password: pinnyPassword, provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Base = "dc=pinniped,dc=dev" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"ball-game-players", "seals"}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, }, }, { @@ -103,7 +108,7 @@ func TestLDAPSearch(t *testing.T) { password: pinnyPassword, provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Filter = "(cn={})" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"ball-game-players", "seals"}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, }, }, { @@ -115,7 +120,7 @@ func TestLDAPSearch(t *testing.T) { p.UserSearch.Filter = "cn={}" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "cn=pinny,ou=users,dc=pinniped,dc=dev", UID: "1000", Groups: []string{"ball-game-players", "seals"}}, + User: &user.DefaultInfo{Name: "cn=pinny,ou=users,dc=pinniped,dc=dev", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, }, }, { @@ -126,7 +131,7 @@ func TestLDAPSearch(t *testing.T) { p.UserSearch.Filter = "(|(cn={})(mail={}))" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"ball-game-players", "seals"}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, }, }, { @@ -137,7 +142,7 @@ func TestLDAPSearch(t *testing.T) { p.UserSearch.Filter = "(|(cn={})(mail={}))" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"ball-game-players", "seals"}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, }, }, { @@ -146,7 +151,7 @@ func TestLDAPSearch(t *testing.T) { password: pinnyPassword, provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.UIDAttribute = "dn" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "cn=pinny,ou=users,dc=pinniped,dc=dev", Groups: []string{"ball-game-players", "seals"}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("cn=pinny,ou=users,dc=pinniped,dc=dev"), Groups: []string{"ball-game-players", "seals"}}, }, }, { @@ -155,7 +160,7 @@ func TestLDAPSearch(t *testing.T) { password: pinnyPassword, provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.UIDAttribute = "sn" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "Seal", Groups: []string{"ball-game-players", "seals"}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("Seal"), Groups: []string{"ball-game-players", "seals"}}, }, }, { @@ -164,7 +169,7 @@ func TestLDAPSearch(t *testing.T) { password: pinnyPassword, provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.UsernameAttribute = "sn" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "Seal", UID: "1000", Groups: []string{"ball-game-players", "seals"}}, // note that the final answer has case preserved from the entry + User: &user.DefaultInfo{Name: "Seal", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, // note that the final answer has case preserved from the entry }, }, { @@ -177,7 +182,7 @@ func TestLDAPSearch(t *testing.T) { p.UserSearch.UIDAttribute = "givenName" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "Pinny the 🦭", UID: "Pinny the 🦭", Groups: []string{"ball-game-players", "seals"}}, + User: &user.DefaultInfo{Name: "Pinny the 🦭", UID: b64("Pinny the 🦭"), Groups: []string{"ball-game-players", "seals"}}, }, }, { @@ -189,7 +194,7 @@ func TestLDAPSearch(t *testing.T) { p.UserSearch.UsernameAttribute = "cn" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"ball-game-players", "seals"}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, }, }, { @@ -210,7 +215,7 @@ func TestLDAPSearch(t *testing.T) { p.GroupSearch.Base = "" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{}}, }, }, { @@ -221,7 +226,7 @@ func TestLDAPSearch(t *testing.T) { p.GroupSearch.Base = "ou=users,dc=pinniped,dc=dev" // there are no groups under this part of the tree })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{}}, }, }, { @@ -232,7 +237,7 @@ func TestLDAPSearch(t *testing.T) { p.GroupSearch.GroupNameAttribute = "dn" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{ + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{ "cn=ball-game-players,ou=beach-groups,ou=groups,dc=pinniped,dc=dev", "cn=seals,ou=groups,dc=pinniped,dc=dev", }}, @@ -246,7 +251,7 @@ func TestLDAPSearch(t *testing.T) { p.GroupSearch.GroupNameAttribute = "objectClass" // silly example, but still a meaningful test })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"groupOfNames", "groupOfNames"}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"groupOfNames", "groupOfNames"}}, }, }, { @@ -257,7 +262,7 @@ func TestLDAPSearch(t *testing.T) { p.GroupSearch.Filter = "(&(&(objectClass=groupOfNames)(member={}))(cn=seals))" })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"seals"}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"seals"}}, }, }, { @@ -268,7 +273,7 @@ func TestLDAPSearch(t *testing.T) { p.GroupSearch.Filter = "foobar={}" // foobar is not a valid attribute name for this LDAP server's schema })), wantAuthResponse: &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{}}, }, }, { @@ -593,7 +598,7 @@ func TestLDAPSearch(t *testing.T) { } } -func TestSimultaneousRequestsOnSingleProvider(t *testing.T) { +func TestSimultaneousLDAPRequestsOnSingleProvider(t *testing.T) { env := library.IntegrationEnv(t) // Note that these tests depend on the values hard-coded in the LDIF file in test/deploy/tools/ldap.yaml. @@ -614,6 +619,10 @@ func TestSimultaneousRequestsOnSingleProvider(t *testing.T) { provider := upstreamldap.New(*defaultProviderConfig(env, ldapHostPort)) + b64 := func(s string) string { + return base64.RawURLEncoding.EncodeToString([]byte(s)) + } + // Making multiple simultaneous requests on the same upstreamldap.Provider instance should all succeed // without triggering the race detector. iterations := 150 @@ -639,7 +648,7 @@ func TestSimultaneousRequestsOnSingleProvider(t *testing.T) { assert.NoError(t, result.err) assert.True(t, result.authenticated, "expected the user to be authenticated, but they were not") assert.Equal(t, &authenticator.Response{ - User: &user.DefaultInfo{Name: "pinny", UID: "1000", Groups: []string{"ball-game-players", "seals"}}, + User: &user.DefaultInfo{Name: "pinny", UID: b64("1000"), Groups: []string{"ball-game-players", "seals"}}, }, result.response) } } diff --git a/test/integration/supervisor_login_test.go b/test/integration/supervisor_login_test.go index 12e4553e..5e550580 100644 --- a/test/integration/supervisor_login_test.go +++ b/test/integration/supervisor_login_test.go @@ -119,7 +119,9 @@ func TestSupervisorLogin(t *testing.T) { }, // the ID token Subject should be the Host URL plus the value pulled from the requested UserSearch.Attributes.UID attribute wantDownstreamIDTokenSubjectToMatch: regexp.QuoteMeta( - "ldaps://" + env.SupervisorUpstreamLDAP.Host + "?base=" + url.QueryEscape(env.SupervisorUpstreamLDAP.UserSearchBase) + "&sub=" + env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue, + "ldaps://" + env.SupervisorUpstreamLDAP.Host + + "?base=" + url.QueryEscape(env.SupervisorUpstreamLDAP.UserSearchBase) + + "&sub=" + base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue)), ), // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute wantDownstreamIDTokenUsernameToMatch: regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserMailAttributeValue), @@ -176,7 +178,9 @@ func TestSupervisorLogin(t *testing.T) { }, // the ID token Subject should be the Host URL plus the value pulled from the requested UserSearch.Attributes.UID attribute wantDownstreamIDTokenSubjectToMatch: regexp.QuoteMeta( - "ldaps://" + env.SupervisorUpstreamLDAP.StartTLSOnlyHost + "?base=" + url.QueryEscape(env.SupervisorUpstreamLDAP.UserSearchBase) + "&sub=" + env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue, + "ldaps://" + env.SupervisorUpstreamLDAP.StartTLSOnlyHost + + "?base=" + url.QueryEscape(env.SupervisorUpstreamLDAP.UserSearchBase) + + "&sub=" + base64.RawURLEncoding.EncodeToString([]byte(env.SupervisorUpstreamLDAP.TestUserUniqueIDAttributeValue)), ), // the ID token Username should have been pulled from the requested UserSearch.Attributes.Username attribute wantDownstreamIDTokenUsernameToMatch: regexp.QuoteMeta(env.SupervisorUpstreamLDAP.TestUserDN),