In LDAP, do not log username until we know the user exists.

This prevents accidentally logging a password if the user enters it into the username field by mistake.

Signed-off-by: Matt Moyer <moyerm@vmware.com>
This commit is contained in:
Matt Moyer 2021-05-28 16:37:31 -05:00
parent 854903c4ed
commit 7ee1f8c441
No known key found for this signature in database
GPG Key ID: EAE88AD172C5AE2D
3 changed files with 22 additions and 8 deletions

View File

@ -395,13 +395,27 @@ func (p *Provider) validateConfig() error {
func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(conn Conn, foundUserDN string) error) (string, string, []string, error) { func (p *Provider) searchAndBindUser(conn Conn, username string, bindFunc func(conn Conn, foundUserDN string) error) (string, string, []string, error) {
searchResult, err := conn.Search(p.userSearchRequest(username)) searchResult, err := conn.Search(p.userSearchRequest(username))
if err != nil { if err != nil {
return "", "", nil, fmt.Errorf(`error searching for user "%s": %w`, username, err) plog.All(`error searching for user`,
"upstreamName", p.GetName(),
"username", username,
"err", err,
)
return "", "", nil, fmt.Errorf(`error searching for user: %w`, err)
} }
if len(searchResult.Entries) == 0 { if len(searchResult.Entries) == 0 {
plog.Debug("error finding user: user not found (if this username is valid, please check the user search configuration)", if plog.Enabled(plog.LevelAll) {
"upstreamName", p.GetName(), "username", username) plog.All("error finding user: user not found (if this username is valid, please check the user search configuration)",
"upstreamName", p.GetName(),
"username", username,
)
} else {
plog.Debug("error finding user: user not found (cowardly avoiding printing username because log level is not 'all')", "upstreamName", p.GetName())
}
return "", "", nil, nil return "", "", nil, nil
} }
// At this point, we have matched at least one entry, so we can be confident that the username is not actually
// someone's password mistakenly entered into the username field, so we can log it without concern.
if len(searchResult.Entries) > 1 { if len(searchResult.Entries) > 1 {
return "", "", nil, fmt.Errorf(`searching for user "%s" resulted in %d search results, but expected 1 result`, return "", "", nil, fmt.Errorf(`searching for user "%s" resulted in %d search results, but expected 1 result`,
username, len(searchResult.Entries), username, len(searchResult.Entries),

View File

@ -546,7 +546,7 @@ func TestEndUserAuthentication(t *testing.T) {
conn.EXPECT().Search(expectedUserSearch(nil)).Return(nil, errors.New("some user search error")).Times(1) conn.EXPECT().Search(expectedUserSearch(nil)).Return(nil, errors.New("some user search error")).Times(1)
conn.EXPECT().Close().Times(1) conn.EXPECT().Close().Times(1)
}, },
wantError: fmt.Sprintf(`error searching for user "%s": some user search error`, testUpstreamUsername), wantError: `error searching for user: some user search error`,
}, },
{ {
name: "when searching for the user's groups returns an error", name: "when searching for the user's groups returns an error",

View File

@ -348,7 +348,7 @@ func TestLDAPSearch(t *testing.T) {
username: "pinny", username: "pinny",
password: pinnyPassword, password: pinnyPassword,
provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Filter = "*" })), provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Filter = "*" })),
wantError: `error searching for user "pinny": LDAP Result Code 201 "Filter Compile Error": ldap: error parsing filter`, wantError: `error searching for user: LDAP Result Code 201 "Filter Compile Error": ldap: error parsing filter`,
}, },
{ {
name: "when the group search filter does not compile", name: "when the group search filter does not compile",
@ -364,7 +364,7 @@ func TestLDAPSearch(t *testing.T) {
provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) {
p.UserSearch.Filter = "objectClass=*" // overly broad search filter p.UserSearch.Filter = "objectClass=*" // overly broad search filter
})), })),
wantError: `error searching for user "pinny": LDAP Result Code 4 "Size Limit Exceeded": `, wantError: `error searching for user: LDAP Result Code 4 "Size Limit Exceeded": `,
}, },
{ {
name: "when the server is unreachable with TLS", name: "when the server is unreachable with TLS",
@ -536,7 +536,7 @@ func TestLDAPSearch(t *testing.T) {
username: "pinny", username: "pinny",
password: pinnyPassword, password: pinnyPassword,
provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Base = "invalid-base" })), provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Base = "invalid-base" })),
wantError: `error searching for user "pinny": LDAP Result Code 34 "Invalid DN Syntax": invalid DN`, wantError: `error searching for user: LDAP Result Code 34 "Invalid DN Syntax": invalid DN`,
}, },
{ {
name: "when the group search base is invalid", name: "when the group search base is invalid",
@ -550,7 +550,7 @@ func TestLDAPSearch(t *testing.T) {
username: "pinny", username: "pinny",
password: pinnyPassword, password: pinnyPassword,
provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Base = "ou=does-not-exist,dc=pinniped,dc=dev" })), provider: upstreamldap.New(*providerConfig(func(p *upstreamldap.ProviderConfig) { p.UserSearch.Base = "ou=does-not-exist,dc=pinniped,dc=dev" })),
wantError: `error searching for user "pinny": LDAP Result Code 32 "No Such Object": `, wantError: `error searching for user: LDAP Result Code 32 "No Such Object": `,
}, },
{ {
name: "when the group search base does not exist", name: "when the group search base does not exist",