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

Fix basic auth #349

Merged
merged 2 commits into from
Jun 4, 2021
Merged

Fix basic auth #349

merged 2 commits into from
Jun 4, 2021

Conversation

carlanton
Copy link
Contributor

Hi,

I couldn't get basic auth working and found this. I'm using a nginx proxy between grafana and clickhouse that is configured with basic auth.

Cheers,
Anton

pkg/client.go Show resolved Hide resolved
@carlanton carlanton requested a review from Slach June 4, 2021 08:56
@Slach Slach merged commit 990fdd5 into Altinity:master Jun 4, 2021
@Slach
Copy link
Collaborator

Slach commented Jun 4, 2021

thanks a lot for your contribution

@carlanton carlanton deleted the fix-backend-basic-auth branch June 4, 2021 08:59
@@ -45,6 +45,7 @@ func (client *ClickHouseClient) Query(query string) (*Response, error) {

if client.settings.Instance.BasicAuthEnabled {
password, _ := client.settings.Instance.DecryptedSecureJSONData["basicAuthPassword"]
req.SetBasicAuth(client.settings.Instance.BasicAuthUser, password)
Copy link
Contributor

@bmanth60 bmanth60 Jun 4, 2021

Choose a reason for hiding this comment

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

@Slach in the scenario mentioned regarding a proxy, shouldn't X-ClickHouse-User and password be set from the header config parameter? That way the basic auth could be a different value than the click house user and password?

Copy link
Collaborator

Choose a reason for hiding this comment

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

i think it should resolve during implements #348

@carlanton
Copy link
Contributor Author

Hi again,

This actually didn't fix my issue :)
Nginx is happy with the basic auth and sends the request to clickhouse, but since we're now also setting X-ClickHouse-User and X-ClickHouse-Password, Clickhouse complains that the user doesn't exist.

Maybe my setup is a bit weird, and perhaps it would make more sense to to the authentication in clickhouse instead of in nginx. However, the documentation (https://clickhouse.tech/docs/en/interfaces/http/#default-database) says that the password should be passed in one of three ways :-) Doesn't it make more sense to just use standard basic auth and not the special headers?

What do you think?

Thanks,
Anton

@Slach
Copy link
Collaborator

Slach commented Jun 8, 2021

@carlanton do you have host_regexp or host or ip restriction in your profile settings? unfortunately "User doesn't exists" mean "doesn't exists user which fit current security restriction including host, host_regexp, IP and password protection" ;)

@Slach
Copy link
Collaborator

Slach commented Jun 8, 2021

@carlanton

wget -qO- --header="X-ClickHouse-User: default" "http://default@127.0.0.1:8123/?query=SELECT Version()"

works fine, so you can pass headers + basic auth to clickhouse server
but if you have different authorization credentials for nginx and clickhouse, you should clean Basic Auth headers on nginx side (as I remember it's a default behavior)

@carlanton
Copy link
Contributor Author

This works:

$ echo "SELECT 1" | curl -i -u user:password "http://nginx-clickhouse.local" -d@-
HTTP/2 200 
date: Tue, 08 Jun 2021 10:08:15 GMT
content-type: text/tab-separated-values; charset=UTF-8
x-clickhouse-summary: {"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"0"}
....

This does not:

$echo "SELECT 1" | curl -i -u user:password -H 'X-ClickHouse-User: user' -H 'X-ClickHouse-Key: password' "http://nginx-clickhouse.local" -d@-
HTTP/2 500 
date: Tue, 08 Jun 2021 10:08:16 GMT
content-type: text/plain; charset=UTF-8
x-clickhouse-exception-code: 516
x-clickhouse-server-display-name: .....
strict-transport-security: max-age=15724800; includeSubDomains

Code: 516, e.displayText() = DB::Exception: dist: Authentication failed: password is incorrect or there is no user with such name (version 21.1.2.15 (official build))

Nginx cleans the basic auth header (ie. Authorization), but passes X-Clickhouse-* to clickhouse. I could add add rules in nginx for removing those headers, but I'm thinking that not sending the authentication headers twice makes more sense.

@Slach
Copy link
Collaborator

Slach commented Jun 8, 2021

Could you try to check authorization headers directly to clickhouse?

curl -vvv -H 'X-ClickHouse-User: user' -H 'X-ClickHouse-Key: password' "http://clickhouse-proxy_pass_ip:8123?query=SELECT Version()"

@carlanton
Copy link
Contributor Author

User default works. Other users does not work. Note that the default user doesn't have a password set and it works with whatever password...

$ wget -qO- --server-response --header="X-ClickHouse-User: default" --header="X-Clickhouse-Password: xyz" \
    "http://127.0.0.1:8123/?query=SELECT 1"
  HTTP/1.1 200 OK
  Date: Tue, 08 Jun 2021 12:43:28 GMT
  Connection: Keep-Alive
  Content-Type: text/tab-separated-values; charset=UTF-8
  Transfer-Encoding: chunked
  X-ClickHouse-Query-Id: dcfffa6b-f769-484e-b8af-eeaba25b4cad
  X-ClickHouse-Format: TabSeparated
  X-ClickHouse-Timezone: UTC
  Keep-Alive: timeout=3
  X-ClickHouse-Summary: {"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"0"}
1
$ wget -qO- --server-response --header="X-ClickHouse-User: default" --header="X-Clickhouse-Password: random" \
    "http://127.0.0.1:8123/?query=SELECT 1"
  HTTP/1.1 200 OK
  Date: Tue, 08 Jun 2021 12:50:57 GMT
  Connection: Keep-Alive
  Content-Type: text/tab-separated-values; charset=UTF-8
  X-ClickHouse-Server-Display-Name: clickhouse-viewer-0.clickhouse-viewer.dist.svc.app.aurora
  Transfer-Encoding: chunked
  X-ClickHouse-Query-Id: 03cb4f26-d63e-4f9b-b3fc-9c2dc27b734b
  X-ClickHouse-Format: TabSeparated
  X-ClickHouse-Timezone: UTC
  Keep-Alive: timeout=3
  X-ClickHouse-Summary: {"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"0"}
1
$ wget -qO- --server-response --header="X-ClickHouse-User: anton" --header="X-Clickhouse-Password: xyz" \
    "http://127.0.0.1:8123/?query=SELECT 1"
  HTTP/1.1 500 Internal Server Error
  Date: Tue, 08 Jun 2021 12:45:29 GMT
  Connection: Keep-Alive
  Content-Type: text/plain; charset=UTF-8
  Transfer-Encoding: chunked
  X-ClickHouse-Exception-Code: 516

Interesting enough, basic auth behaves differently:

$ wget -qO- --server-response  --http-user=default --http-password=xyz "http://127.0.0.1:8123/?query=SELECT 1"
  HTTP/1.1 200 OK
  Date: Tue, 08 Jun 2021 12:48:24 GMT
  Connection: Keep-Alive
  Content-Type: text/tab-separated-values; charset=UTF-8
  Transfer-Encoding: chunked
  X-ClickHouse-Query-Id: 6a6d658d-ace7-424b-af52-560c3ddca67e
  X-ClickHouse-Format: TabSeparated
  X-ClickHouse-Timezone: UTC
  Keep-Alive: timeout=3
  X-ClickHouse-Summary: {"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"0"}
1
$ wget -qO- --server-response  --http-user=anton --http-password=xyz "http://127.0.0.1:8123/?query=SELECT 1"
  HTTP/1.1 200 OK
  Date: Tue, 08 Jun 2021 12:49:38 GMT
  Connection: Keep-Alive
  Content-Type: text/tab-separated-values; charset=UTF-8
  Transfer-Encoding: chunked
  X-ClickHouse-Query-Id: 298c650d-ad61-41f2-a4b4-db2e8640de68
  X-ClickHouse-Format: TabSeparated
  X-ClickHouse-Timezone: UTC
  Keep-Alive: timeout=3
  X-ClickHouse-Summary: {"read_rows":"0","read_bytes":"0","written_rows":"0","written_bytes":"0","total_rows_to_read":"0"}
1

@Slach
Copy link
Collaborator

Slach commented Jun 8, 2021

please use X-ClickHouse-Key instead of X-ClickHouse-Password
could you share?

``
wget-vvv --header='X-ClickHouse-User: anton' --header='X-ClickHouse-Key: antons_password' "http://clickhouse-proxy_pass_ip:8123?query=SELECT Version()"

@carlanton
Copy link
Contributor Author

Ah, right sorry. Now I get the same behavior for basic auth and header auth (without nginx). And again, the user anton does not exist, and there is no password for the default user.

$ wget -qO- --server-response --header="X-ClickHouse-User: anton" --header="X-ClickHouse-Key: zzz" \
    "http://127.0.0.1:8123/?query=SELECT 1"
  HTTP/1.1 500 Internal Server Error
  Date: Tue, 08 Jun 2021 13:19:02 GMT
  Connection: Keep-Alive
  Content-Type: text/plain; charset=UTF-8
  Transfer-Encoding: chunked
  X-ClickHouse-Exception-Code: 516
$ wget -qO- --server-response --header="X-ClickHouse-User: default" --header="X-ClickHouse-Key: xyz" \
    "http://127.0.0.1:8123/?query=SELECT 1"
  HTTP/1.1 500 Internal Server Error
  Date: Tue, 08 Jun 2021 13:19:02 GMT
  Connection: Keep-Alive
  Content-Type: text/plain; charset=UTF-8
  Transfer-Encoding: chunked
  X-ClickHouse-Exception-Code: 516

Thanks!

@Slach
Copy link
Collaborator

Slach commented Jun 8, 2021

ok. let's complete figure out
do you have the same credentials for nginx basic authorization and clickhouse header-based authorization or not?

@carlanton
Copy link
Contributor Author

My setup looks like this:

grafana --> nginx --> clickhouse

Nginx is configured to require a valid user and password (basic auth) for all requests. The config looks something like this:

location / {
    auth_basic           "restricted";
    auth_basic_user_file /etc/nginx/htpasswd; 
    proxy_pass http://clickhouse:8123
}

Clickhouse has only the default user configured with no password.

So basically, nginx is responsible for all authentication.

The problem with the current implementation (setting Authentication header and X-ClickHouse-User/Key) is that while nginx will accept the user and send the request to further, Clickhouse will deny access since that user doesn't exist (the only user is default)

This may not be the most common setup and I should perhaps do the authentication in Clickhouse instead and pass-trough all requests from nginx. Another workaround would be setting proxy_set_header X-ClickHouse-User "" in nginx to stop those headers from getting to Clickhouse.

However, I think that a better and less confusing option would be to not set X-ClickHouse-User/Key from this plugin when using basic auth.

Thanks,
Anton

@Slach
Copy link
Collaborator

Slach commented Nov 10, 2021

@carlanton thanks a lot for detailed explanation i remove X-ClickHouse-Key/User headers for Basic Auth method.
We can pass proxy_set_header basic_auth on nginx side

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

Successfully merging this pull request may close these issues.

3 participants