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.

Sign up for GitHub

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

Jump to bottom

[YCQL] LDAP integration for YCQL #4421

Closed
ajcaldera1 opened this issue May 8, 2020 · 1 comment
Closed

[YCQL] LDAP integration for YCQL #4421

ajcaldera1 opened this issue May 8, 2020 · 1 comment
Assignees
@m-iancu @pkj415
Labels
area/ysql Yugabyte SQL (YSQL)
Milestone
2.7.x

Comments

@ajcaldera1
Copy link
Contributor

ajcaldera1 commented May 8, 2020 •
edited by daniel-yb
Loading

Tracked by parent issue #2393. Ask is to support LDAP authentication in YCQL. We should follow the same type of parameters that YSQL uses for enabling LDAP authentication.

Analytics
@ajcaldera1 ajcaldera1 added the area/ysql Yugabyte SQL (YSQL) label May 8, 2020
@AmirAbrams AmirAbrams mentioned this issue May 8, 2020
Closed
@rkarthik007 rkarthik007 changed the title [YCQL] LDAP integration [YCQL] LDAP integration for YCQL May 12, 2020
@ajcaldera1
Copy link
Contributor Author

I've received some additional feedback on this item that we need to support search + bind as well as simple bind with LDAP in YCQL.

@m-iancu m-iancu added this to the 2.7.x milestone May 26, 2021
@pkj415 pkj415 self-assigned this Jun 7, 2021
@mengchoontan mengchoontan pinned this issue Jul 5, 2021
@VijiYB VijiYB unpinned this issue Jul 6, 2021
pkj415 added a commit that referenced this issue Aug 12, 2021
@pkj415
[#4421] [YCQL] Enable LDAP based authentication
d8c7713 
Summary:
In terms of supported functionality - YCQL will support all options that are
allowed in YSQL's LDAP auth. Broadly, this includes the simple bind and
search + bind mode.

Instead of a full blown file based auth config like ysql_hba.conf in YSQL (where
the config supports many features apart from LDAP), we chose to allow LDAP
configuration using a set of gflags. This is simpler to do now. In case we later
plan to support more auth based rules based on remote ip, keyspace name, etc,
we can add a similar auth config file for YCQL.

Most code is almost just a copy paste from src/postgres/src/backend/libpq/auth.c -
  1. InitializeLDAPConnection()
  2. CheckLDAPAuth()
  3. errdetail_for_ldap() is logic present in LDAPError class

Given the minimal functionality needed, a copy paste from auth.c is a simpler
and less error-prone than making functions in auth.c generic enough to be used
from both postgres and YCQL proxy.

One difference is that - LDAP_DEPRECATED is removed and so some interface
calls to ldap library are different.

Test Plan:
./yb_build.sh --java-test org.yb.cql.TestLDAPAuth
Jenkins: urgent

Reviewers: mihnea, neil, alan, dmitry

Reviewed By: dmitry

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D12095
pkj415 added a commit that referenced this issue Aug 12, 2021
@pkj415
[Backport 2.6] [#4421] [YCQL] Enable LDAP based authentication
38f43a0 
Summary:
In terms of supported functionality - YCQL will support all options that are
allowed in YSQL's LDAP auth. Broadly, this includes the simple bind and
search + bind mode.

Instead of a full blown file based auth config like ysql_hba.conf in YSQL (where
the config supports many features apart from LDAP), we chose to allow LDAP
configuration using a set of gflags. This is simpler to do now. In case we later
plan to support more auth based rules based on remote ip, keyspace name, etc,
we can add a similar auth config file for YCQL.

Most code is almost just a copy paste from src/postgres/src/backend/libpq/auth.c -
  1. InitializeLDAPConnection()
  2. CheckLDAPAuth()
  3. errdetail_for_ldap() is logic present in LDAPError class

Given the minimal functionality needed, a copy paste from auth.c is a simpler
and less error-prone than making functions in auth.c generic enough to be used
from both postgres and YCQL proxy.

One difference is that - LDAP_DEPRECATED is removed and so some interface
calls to ldap library are different.

Original commit: https://phabricator.dev.yugabyte.com/D12095 / d8c7713

Test Plan:
Jenkins: rebase: 2.6, urgent
./yb_build.sh --java-test org.yb.cql.TestLDAPAuth

Reviewers: dmitry, mihnea

Reviewed By: mihnea

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D12544
@pkj415 pkj415 closed this as completed Sep 9, 2021
pkj415 added a commit to pkj415/yugabyte-db that referenced this issue Sep 25, 2021
@pkj415
[yugabyte#4421] [YCQL] Disallow Unauthenticated LDAP binding
403673e 
Summary:
LDAP protocol has an "Unauthenticated" bind mechanism which allows successful
bind with a non-empty username but empty password.

As per https://datatracker.ietf.org/doc/html/rfc4513#section-6.3.1, clients and
servers, both, should steer clear of this bind mechanism. For this, a check has
been added to error out on empty passwords.

Test Plan: Jenkins: urgent

Reviewers: mihnea, dmitry, alan

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D13197
pkj415 added a commit that referenced this issue Sep 27, 2021
@pkj415
[#4421] [YCQL] Disallow Unauthenticated LDAP binding + add handling f…
08ad5c8 
…or ycql_ldap_search_filter

Summary:
LDAP protocol has an "Unauthenticated" bind mechanism which allows successful
bind with a non-empty username but empty password.

As per https://datatracker.ietf.org/doc/html/rfc4513#section-6.3.1, clients and
servers, both, should steer clear of this bind mechanism. For this, a check has
been added to error out on empty passwords.

Another fix in this diff is that the handling and tests for ycql_ldap_search_filter were missing.
They have been added now.

Test Plan:
Jenkins: urgent
./yb_build.sh --java-test org.yb.cql.TestLDAPAuth

Reviewers: dmitry, alan, mihnea

Reviewed By: alan, mihnea

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D13197
pkj415 added a commit that referenced this issue Sep 27, 2021
@pkj415
[Backport 2.6] [#4421] [YCQL] Disallow Unauthenticated LDAP binding +…
36f284c 
… add handling for ycql_ldap_search_filter

Summary:
LDAP protocol has an "Unauthenticated" bind mechanism which allows successful
bind with a non-empty username but empty password.

As per https://datatracker.ietf.org/doc/html/rfc4513#section-6.3.1, clients and
servers, both, should steer clear of this bind mechanism. For this, a check has
been added to error out on empty passwords.

Another fix in this diff is that the handling and tests for ycql_ldap_search_filter were missing.
They have been added now.

Original commit: https://phabricator.dev.yugabyte.com/D13197

Test Plan: Jenkins: rebase: 2.6, urgent

Reviewers: dmitry, alan, mihnea

Reviewed By: mihnea

Subscribers: yql

Differential Revision: https://phabricator.dev.yugabyte.com/D13203
pkj415 added a commit that referenced this issue Oct 7, 2021
@pkj415
[Backport 2.6.1] [#4421] [YCQL] Disallow Unauthenticated LDAP binding…
9231cea 
… + add handling for ycql_ldap_search_filter

Summary:
LDAP protocol has an "Unauthenticated" bind mechanism which allows successful
bind with a non-empty username but empty password.

As per https://datatracker.ietf.org/doc/html/rfc4513#section-6.3.1, clients and
servers, both, should steer clear of this bind mechanism. For this, a check has
been added to error out on empty passwords.

Another fix in this diff is that the handling and tests for ycql_ldap_search_filter were missing.
They have been added now.

Original commit: https://phabricator.dev.yugabyte.com/D13197, 08ad5c8

Test Plan:
Jenkins: urgent, rebase: 2.6.1
./yb_build.sh --java-test org.yb.cql.TestLDAPAuth

Reviewers: dmitry, alan, mihnea, steve.varnau

Reviewed By: mihnea, steve.varnau

Subscribers: steve.varnau, yql

Differential Revision: https://phabricator.dev.yugabyte.com/D13329
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@m-iancu m-iancu

@pkj415 pkj415

Labels
area/ysql Yugabyte SQL (YSQL)
Projects
None yet
Milestone
2.7.x
Development

No branches or pull requests
3 participants
@m-iancu @pkj415 @ajcaldera1