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

privilege, server: support LDAP authentication #43582

Merged
merged 8 commits into from
May 10, 2023

Conversation

YangKeao
Copy link
Member

@YangKeao YangKeao commented May 6, 2023

What problem does this PR solve?

Issue Number: close #43580

Problem Summary:

Support LDAP authentication in TiDB.

What is changed and how it works?

  1. Allow creating users identified with ldap.
  2. Authenticate through binding with LDAP server, with simple method or SASL method.

This PR has some know issues, and will be fixed in the following PR:

  1. If the LDAP connection timeout (or killed by the server), it'll not be released or re-initialized.
  2. The LDAP connection pool is not automatically scaled, so the variable authentication_ldap_{simple,sasl}_max_pool_size has no meaning. Maybe we should use a single authentication_ldap_{simple,sasl}_pool_size variable to replace init and max pool size, if we don't support auto scaling? I'm not sure whether it's needed actually.

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Here is the guide for manually test:

Manual Test Instruction

Here is a guide to manually test the following functions. To help the reviewer test this function, I have prepared a docker image to setup the LDAP server environment. It will use port 389 and 3306 to setup a LDAP server and a percona server (as reference). You can execute the following commands and don't exit the shell. It's also suggested to use the mysql client in this docker image, in case that your local mysql client doesn't support LDAP authentication (or don't have related dependencies installed):

docker run --network host -it yangkeao/ldap-sasl-example:d2b324 /bin/bash

To test this PR, you'll need to compile the tidb server and execute it. I assume the TiDB server runs with default configuration and listens port 4000.

Then we need to set some basic variables to make LDAP work:

set global authentication_ldap_sasl_server_host='127.0.0.1';

Simple LDAP authentication

  1. Create a user with simple LDAP authentication by executing the following SQL in TiDB server.
create user yangkeao IDENTIFIED WITH authentication_ldap_simple as 'cn=yangkeao+uid=yangkeao,dc=example,dc=org';
  1. Try to login the tidb-server with user yangkeao. The password is 123456.
LIBMYSQL_ENABLE_CLEARTEXT_PLUGIN=1 mysql -h 127.0.0.1 -u yangkeao -P 4000 -p123456

Login successfully. It means the simple LDAP authentication method really works!

LDAP SASL authentication (SCRAM-SHA-1/256)

  1. Create a user with SASL LDAP authentication, or alter the previous user to use SASL LDAP authentication:
alter user yangkeao IDENTIFIED WITH authentication_ldap_sasl as 'cn=yangkeao+uid=yangkeao,dc=example,dc=org';
  1. Try to login the tidb-server with user yangkeao. As the LDAP SASL method uses authentication_ldap_sasl_client plugin, but not clear_text plugin, so you don't need to enable the clear_text plugin with environment variables:
mysql -h 127.0.0.1 -u yangkeao -P 4000 -p123456

Login successfully. It means the SASL LDAP authentication method really works!

  1. You can also change to another SASL method to have a try (the default one is SCRAM-SHA-1). For example:
set global authentication_ldap_sasl_auth_method_name='SCRAM-SHA-256';
  1. Then you can login through mysql -h 127.0.0.1 -u yangkeao -P 4000 -p123456, you'll get the same result.

StartTLS

TiDB also support using TLS connection between tidb-server and LDAP server (NOTE: LDAP over SSL is not supported, like MySQL). The CA certificate locates in /etc/ssl/certs/example.crt in the container. You can copy it to anywhere the TiDB server can read. For example, I copied it to /tmp/ca.crt:

cat <<EOF > /tmp/ca.crt
-----BEGIN CERTIFICATE-----
MIIFZTCCA02gAwIBAgIUZ2hQOFVvjuAHrC8Tv+5dnwPGvF0wDQYJKoZIhvcNAQEL
BQAwQjELMAkGA1UEBhMCWFgxFTATBgNVBAcMDERlZmF1bHQgQ2l0eTEcMBoGA1UE
CgwTRGVmYXVsdCBDb21wYW55IEx0ZDAeFw0yMzA0MjQwNjM1MTRaFw0yODA0MjMw
NjM1MTRaMEIxCzAJBgNVBAYTAlhYMRUwEwYDVQQHDAxEZWZhdWx0IENpdHkxHDAa
BgNVBAoME0RlZmF1bHQgQ29tcGFueSBMdGQwggIiMA0GCSqGSIb3DQEBAQUAA4IC
DwAwggIKAoICAQDFvQt3xupYFQxZsyQPr2byhR9ZHoAWBxxqNWxbvpqy7RzHeccJ
Jg36dO1BNIBY8NjIy/JHV7eLDVGCh9FTGozn8dODQMOwDXTYqxFOiBHb2/M9wxVX
ILafa1GlsOnUFxEws9T0XH7ZBqMLC/KlXdJ5xQD1C36K1eWHvphjD0AFhgUnqQ4N
O3NT3tJjzcY7oXBoKpgSgQs7qeTdJLTKJE7pY02C/hJI2WvJDdIiEhZTi0UWqE06
5aXp8Heag/H4VlZzRA+RzQuDXqgXC3Bt7mJwtnoym0HgyTvoKBKO/vzfAW1yQhGo
6ikfSZkvIy3kyPAxD1FSWeSA0Xo8soGNDUsZjV6dQRtcnlWLPFA+7VfwivCPNiFh
91csXhHNEkYPNq4yCe6ZsycydZ+GNdNygzIrMjQ+DjNnHXXmfdeiiLLJbyxYzwuu
GaAT9eD98vXeJFhuWSbKyj4oXcMKnj3bTAQnudMCHIV8cMDe7Zuq1d4/gjXvk95Z
s/OnxqRYYNTXECkdLrevAPfGI2Qg9d7IrhnAh6KqCDDiFkhDkS5TMbWeHA88ZPKZ
6RvLYZmA+j3tjtKPpta9yPiTglExjBUDHIe+37K84G4p0C4bEo5RxEop5hHuX4EW
QvwNb0254i+RCsdyt+tFHiAVzo3/mTg9EMkWlTzoy0381HytFNcLDGb37QIDAQAB
o1MwUTAdBgNVHQ4EFgQUHs+YTH+x0YRNja11v18CF4iu5XowHwYDVR0jBBgwFoAU
Hs+YTH+x0YRNja11v18CF4iu5XowDwYDVR0TAQH/BAUwAwEB/zANBgkqhkiG9w0B
AQsFAAOCAgEAqf2ukpuqXC7u0kTqWdrw+3x2R+mwoLSsy7QbpsjdPbJwVoEysC02
WHiZcMj+w75wZQ2SS5Md1+1K4HiIWC3n+eRDodz+Di0Hg9lxtoMFuOIBpnYUsDuA
Fooo/B7HadZkw9AbWFxPK5EGLyfCRuZF50981svSX5rnYqgCLLs0zGxr7Uswhzvh
3fQMDd0OGLST0M4FW/pQkRYIWnQ4zn/n+wJaHBeaKXHJ7QfgNCtVXOLTXdzpreIL
RvvydcOdVoPnjMgCs1jyhB6g226+/nOuQqic53pxnUTUTvHFJQ5B6/JlzMHeJS1m
ycvSxmF+3RqhjePiwRAT/ui9FBXkhXSG3wp0n0w83rpq7Ne0uwPH/KE9hqFkiI5x
PzobjKy6ahzoSbZrw/a4gDXfZe3fYGtm1EdyDYTh1HFCi7hkdoxY9iCIL1Gr+mpB
YruELQZ+RpvZQ7V8JN7bPtzWfPybPkOSozP1EoLXhUAnXl4/DinoBZvum1MpvPWY
sKF9qQfTP6cAqOuIL1LcVhJ7yHAjR+BK7tvhA2h4sIqxEjhlDmJjH0XMr9JpYQcb
yBzNgkS0YycMPJru0zb2p7vodql5rxSWArQW13Pyqza6N803Qk3vP0/SCfYXeR/i
hv/InNBQBwfHo79FBEv/T7UB8yS7CIu75f562jp23DFKUQD+doybmDg=
-----END CERTIFICATE-----
EOF

Then configure the TiDB to use StartTLS to connect to the LDAP:

set global authentication_ldap_sasl_tls='ON';
set global authentication_ldap_sasl_ca_path='/tmp/ca.crt';

Then you can login the user yangkeao with the StartTLS:

mysql -h 127.0.0.1 -u yangkeao -P 4000 -p123456

NOTE: this certificate is signed for localhost and 127.0.0.1. Using it on other host will refuse to login.

Release note

Support `authentication_ldap_simple` and `authentication_ldap_sasl` authentication plugin. Allow authenticate user with LDAP service.

@YangKeao YangKeao requested a review from a team as a code owner May 6, 2023 09:00
@ti-chi-bot
Copy link

ti-chi-bot bot commented May 6, 2023

[REVIEW NOTIFICATION]

This pull request has been approved by:

  • CbcWestwolf
  • bb7133

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@ti-chi-bot ti-chi-bot bot added release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels May 6, 2023
@YangKeao
Copy link
Member Author

YangKeao commented May 6, 2023

This PR is around 300-400 lines. Most of the 1500 lines of changes are refractor (adding arguments to existing functions), system variables, getter/setter, format padding... Though, 300-400 lines are quite large too...

@YangKeao YangKeao requested a review from CbcWestwolf May 6, 2023 09:04
@YangKeao YangKeao force-pushed the ldap-support branch 6 times, most recently from c557361 to 3160024 Compare May 6, 2023 12:46
@ti-chi-bot ti-chi-bot bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 7, 2023
YangKeao and others added 4 commits May 8, 2023 15:09
Signed-off-by: Yang Keao <yangkeao@chunibyo.icu>
Signed-off-by: Yang Keao <yangkeao@chunibyo.icu>
Signed-off-by: Yang Keao <yangkeao@chunibyo.icu>
Signed-off-by: Yang Keao <yangkeao@chunibyo.icu>
@ti-chi-bot ti-chi-bot bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 8, 2023
ddl/db_integration_test.go Outdated Show resolved Hide resolved
@YangKeao YangKeao force-pushed the ldap-support branch 3 times, most recently from d7f5798 to 721fbc0 Compare May 8, 2023 09:16
Signed-off-by: Yang Keao <yangkeao@chunibyo.icu>
Copy link
Member

@CbcWestwolf CbcWestwolf left a comment

Choose a reason for hiding this comment

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

LGTM

sessionctx/variable/sysvar.go Outdated Show resolved Hide resolved
@ti-chi-bot ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 8, 2023
Signed-off-by: Yang Keao <yangkeao@chunibyo.icu>
@CbcWestwolf
Copy link
Member

/retest-required

@livepo
Copy link

livepo commented May 9, 2023

LGTM

Signed-off-by: Yang Keao <yangkeao@chunibyo.icu>
@YangKeao
Copy link
Member Author

YangKeao commented May 9, 2023

/retest-required

ddl/db_integration_test.go Outdated Show resolved Hide resolved
ddl/db_integration_test.go Outdated Show resolved Hide resolved
go.mod Show resolved Hide resolved
Copy link
Member

@bb7133 bb7133 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot ti-chi-bot bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels May 10, 2023
@YangKeao YangKeao added the needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. label May 10, 2023
Signed-off-by: Yang Keao <yangkeao@chunibyo.icu>
@bb7133
Copy link
Member

bb7133 commented May 10, 2023

/merge

@ti-chi-bot
Copy link

ti-chi-bot bot commented May 10, 2023

This pull request has been accepted and is ready to merge.

Commit hash: 5764079

@ti-chi-bot ti-chi-bot bot added the status/can-merge Indicates a PR has been approved by a committer. label May 10, 2023
@ti-chi-bot ti-chi-bot bot merged commit 7caffd9 into pingcap:master May 10, 2023
@ti-chi-bot
Copy link
Member

In response to a cherrypick label: new pull request created to branch release-7.1: #43696.

ti-chi-bot pushed a commit to ti-chi-bot/tidb that referenced this pull request May 10, 2023
Signed-off-by: ti-chi-bot <ti-community-prow-bot@tidb.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-cherry-pick-release-7.1 Should cherry pick this PR to release-7.1 branch. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

support LDAP authentication
6 participants