Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ldap: add sanitizer for both account and password to prevent ldap injection attacks #3354

Closed
3 tasks done
hsinatfootprintai opened this issue Feb 14, 2024 · 0 comments · Fixed by #3372
Closed
3 tasks done

Comments

@hsinatfootprintai
Copy link

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

  1. running the query with wildcard embedded account/password (eg. account: J*, password: *)
  2. 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: cn

Logs

No response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants