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

release-24.2: pgwire, ccl: add support for LDAP server authentication #127124

Merged
merged 1 commit into from
Jul 15, 2024

Conversation

blathers-crl[bot]
Copy link

@blathers-crl blathers-crl bot commented Jul 13, 2024

Backport 1/1 commits from #126227 on behalf of @souravcrl.

/cc @cockroachdb/release


pgwire, ccl: add support for LDAP server authentication

informs #125087
fixes #124307
fixes #125076
fixes #125080
Epic CRDB-33829

Release note(enterprise, security): We will be adding a new authentication
mechanism authLDAP to connect to AD servers over LDAPs. Example hba conf entry
for this auth method:

 # TYPE    DATABASE      USER           ADDRESS             METHOD             OPTIONS
 # Allow all users to connect to using LDAP authentication with search and bind
   host    all           all            all                 ldap               ldapserver=ldap.example.com ldapport=636 "ldapbasedn=ou=users,dc=example,dc=com" "ldapbinddn=cn=readonly,dc=example,dc=com" ldapbindpasswd=readonly_password ldapsearchattribute=uid "ldapsearchfilter=(memberof=cn=cockroachdb_users,ou=groups,dc=example,dc=com)"
 # Fallback to password authentication for the root user
   host    all           root           0.0.0.0/0          password

Example to use for azure AD server:

SET cluster setting server.host_based_authentication.configuration = 'host    all           all            all                 ldap ldapserver=azure.dev ldapport=636 "ldapbasedn=OU=AADDC Users,DC=azure,DC=dev" "ldapbinddn=CN=Some User,OU=AADDC Users,DC=azure,DC=dev" ldapbindpasswd=my_pwd ldapsearchattribute=sAMAccountName "ldapsearchfilter=(memberOf=CN=azure-dev-domain-sync-users,OU=AADDC Users,DC=crlcloud,DC=dev)"
host    all           root           0.0.0.0/0          password';

We also add the following cluster settings:

  1. server.ldap_authentication.domain_ca to allow operators to set a custom CA
    for their domain (i.e. example.com).
  2. server.ldap_authentication.client.tls_certificate and
    server.ldap_authentication.client.tls_key to allow operators to set the client
    certificate and key for establishing mTLS connection with LDAP server.

Post configuration users should be able to authenticate to LDAP server if:

  1. The distinguished name corresponding to their sql username exists (we search
    for the sql username using ldapbinddn and ldapbindpasswd in the ldapbasedn
    domain with filter set to ldapsearchfilter and ldapsearchattribute key
    having value sql username in db connection string) and we retrieve the DN.
  2. Their bind attempt is successful with LDAP server using the retrieved DN and
    provided ldap password in db connection string.

Example DB client sql login commands. Note LDAP_SEARCH_VAL and SQL_USERNAME
are same. In case of azure LDAP_SEARCH_VAL will be value for sAMAccountName:
1.

cockroach sql --url "postgresql://{LDAP_SEARCH_VAL}:{LDAP_PASSWORD}@{CLUSTER_HOST}:26257" --certs-dir={CLUSTER_CERT_DIR}
export PGPASSWORD='LDAP_PASSWORD'
cockroach sql --certs-dir=certs --url "postgresql://{LDAP_SEARCH_VAL}@{CLUSTER_HOST}:26257"

Release justification: this is needed for private preview of ldap feature.

@blathers-crl blathers-crl bot force-pushed the blathers/backport-release-24.2-126227 branch from b3086fd to fcdb837 Compare July 13, 2024 18:36
@blathers-crl blathers-crl bot requested review from a team as code owners July 13, 2024 18:36
@blathers-crl blathers-crl bot added blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. labels Jul 13, 2024
Copy link
Author

blathers-crl bot commented Jul 13, 2024

Thanks for opening a backport.

Please check the backport criteria before merging:

  • Backports should only be created for serious
    issues
    or test-only changes.
  • Backports should not break backwards-compatibility.
  • Backports should change as little code as possible.
  • Backports should not change on-disk formats or node communication protocols.
  • Backports should not add new functionality (except as defined
    here).
  • Backports must not add, edit, or otherwise modify cluster versions; or add version gates.
  • All backports must be reviewed by the owning areas TL and one additional
    TL. For more information as to how that review should be conducted, please consult the backport
    policy
    .
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
  • There is a high priority need for the functionality that cannot wait until the next release and is difficult to address in another way.
  • The new functionality is additive-only and only runs for clusters which have specifically “opted in” to it (e.g. by a cluster setting).
  • New code is protected by a conditional check that is trivial to verify and ensures that it only runs for opt-in clusters. State changes must be further protected such that nodes running old binaries will not be negatively impacted by the new state (with a mixed version test added).
  • The PM and TL on the team that owns the changed code have signed off that the change obeys the above rules.
  • Your backport must be accompanied by a post to the appropriate Slack
    channel (#db-backports-point-releases or #db-backports-XX-X-release) for awareness and discussion.

Also, please add a brief release justification to the body of your PR to justify this
backport.

@blathers-crl blathers-crl bot added the backport Label PR's that are backports to older release branches label Jul 13, 2024
Copy link
Author

blathers-crl bot commented Jul 13, 2024

Your pull request contains more than 1000 changes. It is strongly encouraged to split big PRs into smaller chunks.

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Member

This change is Reviewable

informs #125087
fixes #124307
fixes #125076
fixes #125080
Epic CRDB-33829

Release note(enterprise, security): We will be adding a new authentication
mechanism `authLDAP` to connect to AD servers over LDAPs. Example hba conf entry
for this auth method:
```
 # TYPE    DATABASE      USER           ADDRESS             METHOD             OPTIONS
 # Allow all users to connect to using LDAP authentication with search and bind
   host    all           all            all                 ldap               ldapserver=ldap.example.com ldapport=636 "ldapbasedn=ou=users,dc=example,dc=com" "ldapbinddn=cn=readonly,dc=example,dc=com" ldapbindpasswd=readonly_password ldapsearchattribute=uid "ldapsearchfilter=(memberof=cn=cockroachdb_users,ou=groups,dc=example,dc=com)"
 # Fallback to password authentication for the root user
   host    all           root           0.0.0.0/0          password
```
Example to use for azure AD server:
```
SET cluster setting server.host_based_authentication.configuration = 'host    all           all            all                 ldap ldapserver=azure.dev ldapport=636 "ldapbasedn=OU=AADDC Users,DC=azure,DC=dev" "ldapbinddn=CN=Some User,OU=AADDC Users,DC=azure,DC=dev" ldapbindpasswd=my_pwd ldapsearchattribute=sAMAccountName "ldapsearchfilter=(memberOf=CN=azure-dev-domain-sync-users,OU=AADDC Users,DC=crlcloud,DC=dev)"
host    all           root           0.0.0.0/0          password';
```

We also add the following cluster settings:
1. `server.ldap_authentication.domain_ca` to allow operators to set a custom CA
for their domain (i.e. `example.com`).
2. `server.ldap_authentication.client.tls_certificate` and
`server.ldap_authentication.client.tls_key` to allow operators to set the client
certificate and key for establishing mTLS connection with LDAP server.

Post configuration users should be able to authenticate to LDAP server if:
1. The distinguished name corresponding to their sql username exists (we search
for the sql username using `ldapbinddn` and `ldapbindpasswd` in the `ldapbasedn`
domain with filter set to `ldapsearchfilter` and `ldapsearchattribute` key
having value sql username in db connection string) and we retrieve the DN.
2. Their bind attempt is successful with LDAP server using the retrieved DN and
provided ldap password in db connection string.

Example DB client sql login commands. Note `LDAP_SEARCH_VAL` and `SQL_USERNAME`
are same. In case of azure `LDAP_SEARCH_VAL` will be value for `sAMAccountName`:
1.
```
cockroach sql --url "postgresql://{LDAP_SEARCH_VAL}:{LDAP_PASSWORD}@{CLUSTER_HOST}:26257" --certs-dir={CLUSTER_CERT_DIR}
```
2.
```
export PGPASSWORD='LDAP_PASSWORD'
cockroach sql --certs-dir=certs --url "postgresql://{LDAP_SEARCH_VAL}@{CLUSTER_HOST}:26257"
```
@souravcrl souravcrl force-pushed the blathers/backport-release-24.2-126227 branch from fcdb837 to 0bbbb36 Compare July 15, 2024 07:42
Copy link
Contributor

@pritesh-lahoti pritesh-lahoti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job in getting this into 24.2!

Reviewed 13 of 13 files at r1, all commit messages.
Reviewable status: :shipit: complete! 0 of 0 LGTMs obtained (waiting on @rafiss)

@souravcrl souravcrl merged commit 6d484db into release-24.2 Jul 15, 2024
20 checks passed
@souravcrl souravcrl deleted the blathers/backport-release-24.2-126227 branch July 15, 2024 18:52
@souravcrl souravcrl restored the blathers/backport-release-24.2-126227 branch July 15, 2024 18:52
@souravcrl souravcrl deleted the blathers/backport-release-24.2-126227 branch July 15, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Label PR's that are backports to older release branches blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants