ldap: add sanitizer for both account and password to prevent ldap injection attacks #3354
Description
Preflight Checklist
- I agree to follow the Code of Conduct that this project adheres to.
- I have searched the issue tracker for an issue that matches the one I want to file, without success.
- I am not looking for support or already pursued the available support channels without success.
Version
v2.38.0
Storage Type
Kubernetes
Installation Type
Official container image
Expected Behavior
account/password with wildcard embedded(eg. account: J*, password: *) are marked as invalid requests.
Actual Behavior
the request would go inside the LDAP server and introduce LDAP injection risks.
Steps To Reproduce
- running the query with wildcard embedded account/password (eg. account: J*, password: *)
- the LDAP would run the query against the database.
Additional Information
we tested the following patches:
diff --git a/connector/ldap/ldap.go b/connector/ldap/ldap.go
index c50d8309..38b8172e 100644
--- a/connector/ldap/ldap.go
+++ b/connector/ldap/ldap.go
@@ -9,7 +9,9 @@ import (
"fmt"
"net"
"os"
+ "regexp"
"strings"
+ "errors"
"github.com/go-ldap/ldap/v3"
@@ -460,6 +462,22 @@ func (c *ldapConnector) userEntry(conn *ldap.Conn, username string) (user ldap.E
func (c *ldapConnector) Login(ctx context.Context, s connector.Scopes, username, password string) (ident connector.Identity, validPass bool, err error) {
// make this check to avoid unauthenticated bind to the LDAP server.
+
+ matched, err := regexp.MatchString(`[\w\s\*]+`, username)
+ if err != nil {
+ return connector.Identity{}, false, err
+ }
+ if matched {
+ return connector.Identity{}, false, errors.New("invalid input")
+ }
+ matched, err = regexp.MatchString(`[\w\*]+`, password)
+ if err != nil {
+ return connector.Identity{}, false, err
+ }
+ if matched {
+ return connector.Identity{}, false, errors.New("invalid input")
+ }
+
if password == "" {
return connector.Identity{}, false, nil
}
it can stop the wildcard query at the front. But we are not sure if this setup could against any possible scenarios.
Configuration
connectors:
- type: ldap
name: OpenLDAP
id: ldap
config:
# The following configurations seem to work with OpenLDAP:
#
# 1) Plain LDAP, without TLS:
host: localhost:389
insecureNoSSL: true
#
# 2) LDAPS without certificate validation:
#host: localhost:636
#insecureNoSSL: false
#insecureSkipVerify: true
#
# 3) LDAPS with certificate validation:
#host: YOUR-HOSTNAME:636
#insecureNoSSL: false
#insecureSkipVerify: false
#rootCAData: 'CERT'
# ...where CERT="$( base64 -w 0 your-cert.crt )"
# This would normally be a read-only user.
bindDN: cn=admin,dc=example,dc=org
bindPW: admin
usernamePrompt: Email Address
userSearch:
baseDN: ou=People,dc=example,dc=org
filter: "(objectClass=person)"
username: mail
# "DN" (case sensitive) is a special attribute name. It indicates that
# this value should be taken from the entity's DN not an attribute on
# the entity.
idAttr: DN
emailAttr: mail
nameAttr: cn
groupSearch:
baseDN: ou=Groups,dc=example,dc=org
filter: "(objectClass=groupOfNames)"
userMatchers:
# A user is a member of a group when their DN matches
# the value of a "member" attribute on the group entity.
- userAttr: DN
groupAttr: member
# The group name should be the "cn" value.
nameAttr: cnLogs
No response