[Impala-ASF-CR] IMPALA-12552: Fix Kerberos authentication issue that occurs in python 3 environment when kerberos host fqdn option is used

2023-11-14 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20691 )

Change subject: IMPALA-12552: Fix Kerberos authentication issue that occurs in 
python 3 environment when kerberos_host_fqdn option is used
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20691/3/fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
File 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java:

http://gerrit.cloudera.org:8080/#/c/20691/3/fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java@80
PS3, Line 80: /* shouldSucceed */ true, null);
> Sure, I'll update that.
Done


http://gerrit.cloudera.org:8080/#/c/20691/3/fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java@607
PS3, Line 607: testShellKerberosAuthWithUser( kerberosKdcEnvironment, 
"user",
> Yes, it works fine in my local development environment. It should use the "
I refactored the test so that it uses a dummy hostname in the service principal 
of the impala daemon,
impala-shell is connected to the daemon running on localhost, so the use of 
kerberos_host_fqdn option is required, in which the same hostname is specified 
as in the service principal.
I hope that in this way the test will go through without any issues.



--
To view, visit http://gerrit.cloudera.org:8080/20691
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b157d76824ad67faf531a529256a8afe2ab9d49
Gerrit-Change-Number: 20691
Gerrit-PatchSet: 3
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Tue, 14 Nov 2023 21:38:19 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12552: Fix Kerberos authentication issue that occurs in python 3 environment when kerberos host fqdn option is used

2023-11-14 Thread Gergely Farkas (Code Review)
Hello Michael Smith, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20691

to look at the new patch set (#4).

Change subject: IMPALA-12552: Fix Kerberos authentication issue that occurs in 
python 3 environment when kerberos_host_fqdn option is used
..

IMPALA-12552: Fix Kerberos authentication issue that occurs
in python 3 environment when kerberos_host_fqdn option is used

In Pyhton 2, the sasl layer does not accept unicode strings,
so we have to explicitly encode the kerberos_host_fqdn string
to ascii. However, this is not the case in python 3, where
we have to omit the encode, because if we don't do this,
impala-shell wants to use the following service principal
during Kerberos auth:
my_service_name/b'my.kerberos.host.fqdn'@MY.REALM
instead of the correct one, which is:
my_service_name/my.kerberos.host.fqdn@MY.REALM
(This is because the output of the encode function
is a byte array in python 3.)

Tested with new unit tests and with a snapshot build
manually in CDP PVC DS.

Change-Id: I8b157d76824ad67faf531a529256a8afe2ab9d49
---
M fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
M fe/src/test/java/org/apache/impala/customcluster/RunShellCommand.java
M infra/python/deps/py2-requirements.txt
M infra/python/deps/py3-requirements.txt
M shell/impala_client.py
A shell/kerberos_util.py
M shell/make_shell_tarball.sh
M shell/packaging/make_python_package.sh
A tests/shell/test_kerberos_util.py
A tests/shell/test_shell_commandline_kerberos_auth.py
11 files changed, 255 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/20691/4
--
To view, visit http://gerrit.cloudera.org:8080/20691
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b157d76824ad67faf531a529256a8afe2ab9d49
Gerrit-Change-Number: 20691
Gerrit-PatchSet: 4
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 


[Impala-ASF-CR] IMPALA-12552: Fix Kerberos authentication issue that occurs in python 3 environment when kerberos host fqdn option is used

2023-11-14 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20691 )

Change subject: IMPALA-12552: Fix Kerberos authentication issue that occurs in 
python 3 environment when kerberos_host_fqdn option is used
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20691/3/fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
File 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java:

http://gerrit.cloudera.org:8080/#/c/20691/3/fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java@80
PS3, Line 80: /* shouldSucceed */ true, null);
> Could avoid changing all these with a wrapper function signature.
Sure, I'll update that.


http://gerrit.cloudera.org:8080/#/c/20691/3/fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java@607
PS3, Line 607: testShellKerberosAuthWithUser( kerberosKdcEnvironment, 
"user",
> Failed with: Server impala/ip6-localh...@myorg.com not found in Kerberos da
Yes, it works fine in my local development environment. It should use the 
"impala/localh...@myorg.com" service principal, but there is a reverse dns 
check - probably in the kerberos layer under the python - and it wants to use 
the "impala/ip6-localh...@myorg.com" service principal because of that, and it 
can't get the Kerberos ticket from them the KDC because the principal does not 
exist. (Even if such a principal existed, the Kerbros auth testcase would not 
succeed, because the impala daemon was not started with this service principal.)
I'm checking it, but I don't know if it can be fixed somehow.



--
To view, visit http://gerrit.cloudera.org:8080/20691
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8b157d76824ad67faf531a529256a8afe2ab9d49
Gerrit-Change-Number: 20691
Gerrit-PatchSet: 3
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 
Gerrit-Comment-Date: Tue, 14 Nov 2023 16:50:52 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12505: Add flag for trusted domain check to use 'origin' if xff header is not set

2023-11-14 Thread Gergely Farkas (Code Review)
Hello Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20591

to look at the new patch set (#4).

Change subject: IMPALA-12505: Add flag for trusted domain check to use 'origin' 
if xff header is not set
..

IMPALA-12505: Add flag for trusted domain check to use 'origin'
if xff header is not set

This change defines a new impala flag called
'trusted_domain_empty_xff_header_use_origin', which modifies
the trusted domain check to work as follows if the trusted_domain
and trusted_domain_use_xff_header flags are set:
If there is an X-Forwarded-For header in the request, the trusted
domain check runs to the value derived from it, if there is no such
header, then the check runs to the origin (the address sending the
request).
Note: If there is an X-Forwarded-For header in the request or
the trusted_domain_use_xff_header flag or trusted_domain flag is
not set, then the behavior is not changed.

Tested with new custom cluster tests.

Change-Id: I58e5d1119527139eafaa411b55517b10bf394bb2
---
M be/src/rpc/authentication.cc
M be/src/transport/THttpServer.cpp
M be/src/util/webserver.cc
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
5 files changed, 133 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/20591/4
--
To view, visit http://gerrit.cloudera.org:8080/20591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I58e5d1119527139eafaa411b55517b10bf394bb2
Gerrit-Change-Number: 20591
Gerrit-PatchSet: 4
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12505: Add flag for trusted domain check to use 'origin' if xff header is not set

2023-11-14 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20591 )

Change subject: IMPALA-12505: Add flag for trusted domain check to use 'origin' 
if xff header is not set
..


Patch Set 3:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20591/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20591/2//COMMIT_MSG@7
PS2, Line 7: Add flag for trusted domain check to use 'origin'
   : if xff header is not set
   :
> It would be nice to have a shorter first line, e.g. "Add flag for trusted d
Sure, I've updated it. Thanks for the suggestion and the review!


http://gerrit.cloudera.org:8080/#/c/20591/2//COMMIT_MSG@11
PS2, Line 11: use_
> The flag name could be mentioned
You are right!


http://gerrit.cloudera.org:8080/#/c/20591/2/fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
File fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java:

http://gerrit.cloudera.org:8080/#/c/20591/2/fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java@506
PS2, Line 506:
> Can you add some negative tests too (also in webserver tests)?
I added those testcases to hs2 and webserver tests. Thanks!


http://gerrit.cloudera.org:8080/#/c/20591/2/fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java@518
PS2, Line 518: verifyMetrics(0, 0);
> This could be also verified to be 0 at the beginning.
Done



--
To view, visit http://gerrit.cloudera.org:8080/20591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I58e5d1119527139eafaa411b55517b10bf394bb2
Gerrit-Change-Number: 20591
Gerrit-PatchSet: 3
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Tue, 14 Nov 2023 10:42:10 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12505: Add flag for trusted domain check to use 'origin' if xff header is not set

2023-11-14 Thread Gergely Farkas (Code Review)
Hello Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20591

to look at the new patch set (#3).

Change subject: IMPALA-12505: Add flag for trusted domain check to use 'origin' 
if xff header is not set
..

IMPALA-12505: Add flag for trusted domain check to use 'origin'
if xff header is not set

This change defines a new impala flag called
'trusted_domain_empty_xff_header_use_origin', which modifies
the trusted domain check to work as follows if the trusted_domain
and trusted_domain_use_xff_header flags are set:
If there is an X-Forwarded-For header in the request, the trusted
domain check runs to the value derived from it, if there is no such
header, then the check runs to the origin (the address sending the
request).
Note: If there is an X-Forwarded-For header in the request or
the trusted_domain_use_xff_header flag or trusted_domain flag is
not set, then the behavior is not changed.

Tested with new custom cluster tests.

Change-Id: I58e5d1119527139eafaa411b55517b10bf394bb2
---
M be/src/rpc/authentication.cc
M be/src/transport/THttpServer.cpp
M be/src/util/webserver.cc
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
5 files changed, 134 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/20591/3
--
To view, visit http://gerrit.cloudera.org:8080/20591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I58e5d1119527139eafaa411b55517b10bf394bb2
Gerrit-Change-Number: 20591
Gerrit-PatchSet: 3
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12552: Fix Kerberos authentication issue that occurs in python 3 environment when kerberos host fqdn option is used

2023-11-14 Thread Gergely Farkas (Code Review)
Hello Michael Smith, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20691

to look at the new patch set (#3).

Change subject: IMPALA-12552: Fix Kerberos authentication issue that occurs in 
python 3 environment when kerberos_host_fqdn option is used
..

IMPALA-12552: Fix Kerberos authentication issue that occurs
in python 3 environment when kerberos_host_fqdn option is used

In Pyhton 2, the sasl layer does not accept unicode strings,
so we have to explicitly encode the kerberos_host_fqdn string
to ascii. However, this is not the case in python 3, where
we have to omit the encode, because if we don't do this,
impala-shell wants to use the following service principal
during Kerberos auth:
my_service_name/b'my.kerberos.host.fqdn'@MY.REALM
instead of the correct one, which is:
my_service_name/my.kerberos.host.fqdn@MY.REALM
(This is because the output of the encode function
is a byte array in python 3.)

Tested with new unit tests and with a snapshot build
manually in CDP PVC DS.

Change-Id: I8b157d76824ad67faf531a529256a8afe2ab9d49
---
M 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
M fe/src/test/java/org/apache/impala/customcluster/RunShellCommand.java
M infra/python/deps/py2-requirements.txt
M infra/python/deps/py3-requirements.txt
M shell/impala_client.py
A shell/kerberos_util.py
M shell/make_shell_tarball.sh
M shell/packaging/make_python_package.sh
A tests/shell/test_kerberos_util.py
A tests/shell/test_shell_commandline_kerberos_auth.py
10 files changed, 221 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/20691/3
--
To view, visit http://gerrit.cloudera.org:8080/20691
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b157d76824ad67faf531a529256a8afe2ab9d49
Gerrit-Change-Number: 20691
Gerrit-PatchSet: 3
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 


[Impala-ASF-CR] IMPALA-12552: Fix Kerberos authentication issue that occurs in python 3 environment when kerberos host fqdn option is used

2023-11-14 Thread Gergely Farkas (Code Review)
Hello Michael Smith, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20691

to look at the new patch set (#2).

Change subject: IMPALA-12552: Fix Kerberos authentication issue that occurs in 
python 3 environment when kerberos_host_fqdn option is used
..

IMPALA-12552: Fix Kerberos authentication issue that occurs
in python 3 environment when kerberos_host_fqdn option is used

In Pyhton 2, the sasl layer does not accept unicode strings,
so we have to explicitly encode the kerberos_host_fqdn string
to ascii. However, this is not the case in python 3, where
we have to omit the encode, because if we don't do this,
impala-shell wants to use the following service principal
during Kerberos auth:
my_service_name/b'my.kerberos.host.fqdn'@MY.REALM
instead of the correct one, which is:
my_service_name/my.kerberos.host.fqdn@MY.REALM
(This is because the output of the encode function
is a byte array in python 3.)

Tested with new unit tests and with a snapshot build
manually in CDP PVC DS.

Change-Id: I8b157d76824ad67faf531a529256a8afe2ab9d49
---
M 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
M fe/src/test/java/org/apache/impala/customcluster/RunShellCommand.java
M infra/python/deps/py2-requirements.txt
M infra/python/deps/py3-requirements.txt
M shell/impala_client.py
A shell/kerberos_util.py
M shell/make_shell_tarball.sh
M shell/packaging/make_python_package.sh
A tests/shell/test_kerberos_util.py
A tests/shell/test_shell_commandline_kerberos_auth.py
10 files changed, 219 insertions(+), 24 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/20691/2
--
To view, visit http://gerrit.cloudera.org:8080/20691
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I8b157d76824ad67faf531a529256a8afe2ab9d49
Gerrit-Change-Number: 20691
Gerrit-PatchSet: 2
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Michael Smith 


[Impala-ASF-CR] IMPALA-12552: Fix Kerberos authentication issue that occurs in python 3 environment when kerberos host fqdn option is used

2023-11-09 Thread Gergely Farkas (Code Review)
Gergely Farkas has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20691


Change subject: IMPALA-12552: Fix Kerberos authentication issue that occurs in 
python 3 environment when kerberos_host_fqdn option is used
..

IMPALA-12552: Fix Kerberos authentication issue that occurs
in python 3 environment when kerberos_host_fqdn option is used

In Pyhton 2, the sasl layer does not accept unicode strings,
so we have to explicitly encode the kerberos_host_fqdn string
to ascii. However, this is not the case in python 3, where
we have to omit the encode, because if we don't do this,
impala-shell wants to use the following service principal
during Kerberos auth:
my_service_name/b'my.kerberos.host.fqdn'@MY.REALM
instead of the correct one, which is:
my_service_name/my.kerberos.host.fqdn@MY.REALM

Tested with a snapshot build manually in CDP PVC DS.

Change-Id: I8b157d76824ad67faf531a529256a8afe2ab9d49
---
M shell/impala_client.py
1 file changed, 9 insertions(+), 3 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/20691/1
--
To view, visit http://gerrit.cloudera.org:8080/20691
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I8b157d76824ad67faf531a529256a8afe2ab9d49
Gerrit-Change-Number: 20691
Gerrit-PatchSet: 1
Gerrit-Owner: Gergely Farkas 


[Impala-ASF-CR] IMPALA-12505: Define a new impala flag that runs the trusted domain check on the origin address if the trusted domain use xff header flag is enabled and no X-Forwarded-For header is re

2023-10-18 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20591

to look at the new patch set (#2).

Change subject: IMPALA-12505: Define a new impala flag that runs the trusted 
domain check on the origin address if the trusted_domain_use_xff_header flag is 
enabled and no X-Forwarded-For header is received
..

IMPALA-12505: Define a new impala flag that runs the trusted domain
check on the origin address if the trusted_domain_use_xff_header
flag is enabled and no X-Forwarded-For header is received

This change defines a new impala flag, which modifies the trusted
domain check to work as follows if the trusted_domain and
trusted_domain_use_xff_header flags are set:
If there is an X-Forwarded-For header in the request, the trusted
domain check runs to the value derived from it, if there is no such
header, then the check runs to the origin (the address sending the
request).
Note: If there is an X-Forwarded-For header in the request or
the trusted_domain_use_xff_header flag or trusted_domain flag is
not set, then the behavior is not changed.

Tested with new custom cluster tests.

Change-Id: I58e5d1119527139eafaa411b55517b10bf394bb2
---
M be/src/rpc/authentication.cc
M be/src/transport/THttpServer.cpp
M be/src/util/webserver.cc
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
5 files changed, 79 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/20591/2
--
To view, visit http://gerrit.cloudera.org:8080/20591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I58e5d1119527139eafaa411b55517b10bf394bb2
Gerrit-Change-Number: 20591
Gerrit-PatchSet: 2
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12505: Define a new impala flag that runs the trusted domain check on the origin address if the trusted domain use xff header flag is enabled and no X-Forwarded-For header is re

2023-10-18 Thread Gergely Farkas (Code Review)
Gergely Farkas has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20591


Change subject: IMPALA-12505: Define a new impala flag that runs the trusted 
domain check on the origin address if the trusted_domain_use_xff_header flag is 
enabled and no X-Forwarded-For header is received
..

IMPALA-12505: Define a new impala flag that runs the trusted domain
check on the origin address if the trusted_domain_use_xff_header
flag is enabled and no X-Forwarded-For header is received

This change defines a new impala flag, which modifies the trusted
domain check to work as follows if the trusted_domain and
trusted_domain_use_xff_header flags are set:
If there is an X-Forwarded-For header in the request, the trusted
domain check runs to the value derived from it, if there is no such
header, then the check runs to the origin (the address sending the
request).
Note: If there is an X-Forwarded-For header in the request or
the trusted_domain_use_xff_header flag or trusted_domain flag is
not set, then the behavior is not changed.

Tested with new custom cluster tests.

Change-Id: I58e5d1119527139eafaa411b55517b10bf394bb2
---
M be/src/rpc/authentication.cc
M be/src/transport/THttpServer.cpp
M be/src/util/webserver.cc
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
5 files changed, 77 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/91/20591/1
--
To view, visit http://gerrit.cloudera.org:8080/20591
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I58e5d1119527139eafaa411b55517b10bf394bb2
Gerrit-Change-Number: 20591
Gerrit-PatchSet: 1
Gerrit-Owner: Gergely Farkas 


[Impala-ASF-CR] IMPALA-12403: Fix Kerberos authentication when connecting with a proxy user that does not delegate another user

2023-09-14 Thread Gergely Farkas (Code Review)
Hello Norbert Luksa, Laszlo Gaal, Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20421

to look at the new patch set (#7).

Change subject: IMPALA-12403: Fix Kerberos authentication when connecting with 
a proxy user that does not delegate another user
..

IMPALA-12403: Fix Kerberos authentication when connecting with a proxy
user that does not delegate another user

This change improves LDAP checking in the Hiveserver2 API where
previously LDAP filter checks were run on the user principal received
in Kerberos authentication, but after the modification they are run on
the short username taken from the Kerberos principal.

Tested with new custom cluster tests.

Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
---
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-hs2-server.cc
M be/src/transport/TSasl.cpp
M be/src/transport/TSasl.h
M be/src/transport/TSaslTransport.cpp
M be/src/transport/TSaslTransport.h
M be/src/util/auth-util.cc
M be/src/util/auth-util.h
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java
12 files changed, 128 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/20421/7
--
To view, visit http://gerrit.cloudera.org:8080/20421
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
Gerrit-Change-Number: 20421
Gerrit-PatchSet: 7
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Norbert Luksa 


[Impala-ASF-CR] IMPALA-12403: Fix Kerberos authentication when connecting with a proxy user that does not delegate another user

2023-09-14 Thread Gergely Farkas (Code Review)
Hello Norbert Luksa, Laszlo Gaal, Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20421

to look at the new patch set (#6).

Change subject: IMPALA-12403: Fix Kerberos authentication when connecting with 
a proxy user that does not delegate another user
..

IMPALA-12403: Fix Kerberos authentication when connecting with a proxy
user that does not delegate another user

This change improves LDAP checking in the Hiveserver2 API where
previously LDAP filter checks were run on the user principal received
in Kerberos authentication, but after the modification they are run on
the short username taken from the Kerberos principal.

Tested with new custom cluster tests.

Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
---
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-hs2-server.cc
M be/src/transport/TSasl.cpp
M be/src/transport/TSasl.h
M be/src/transport/TSaslTransport.cpp
M be/src/transport/TSaslTransport.h
M be/src/util/auth-util.cc
M be/src/util/auth-util.h
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java
12 files changed, 128 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/20421/6
--
To view, visit http://gerrit.cloudera.org:8080/20421
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
Gerrit-Change-Number: 20421
Gerrit-PatchSet: 6
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Laszlo Gaal 
Gerrit-Reviewer: Norbert Luksa 


[Impala-ASF-CR] IMPALA-12403: Fix Kerberos authentication when connecting with a proxy user that does not delegate another user

2023-08-30 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20421 )

Change subject: IMPALA-12403: Fix Kerberos authentication when connecting with 
a proxy user that does not delegate another user
..


Patch Set 5:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/20421/2/be/src/service/impala-hs2-server.cc
File be/src/service/impala-hs2-server.cc:

http://gerrit.cloudera.org:8080/#/c/20421/2/be/src/service/impala-hs2-server.cc@404
PS2, Line 404: username
> The name is a bit ambiguous to me, as we do not change the actual user name
Done


http://gerrit.cloudera.org:8080/#/c/20421/2/be/src/service/impala-hs2-server.cc@406
PS2, Line 406: !connection_context->kerberos_user_principal.empty()) {
> nit, indentation: -2 for most lines
Thanks!


http://gerrit.cloudera.org:8080/#/c/20421/2/be/src/service/impala-hs2-server.cc@410
PS2, Line 410: OG(1) << "
> Maybe VLOG(1) is enough for this? If the tests use Impala in this way then
Done


http://gerrit.cloudera.org:8080/#/c/20421/2/fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java
File 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java:

http://gerrit.cloudera.org:8080/#/c/20421/2/fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java@190
PS2, Line 190:
> we use /cliservice in all other tests, maybe it is also needed here?
Good point! I've updated it.Thanks!



--
To view, visit http://gerrit.cloudera.org:8080/20421
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
Gerrit-Change-Number: 20421
Gerrit-PatchSet: 5
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 30 Aug 2023 07:18:31 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12403: Fix Kerberos authentication when connecting with a proxy user that does not delegate another user

2023-08-29 Thread Gergely Farkas (Code Review)
Hello Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20421

to look at the new patch set (#5).

Change subject: IMPALA-12403: Fix Kerberos authentication when connecting with 
a proxy user that does not delegate another user
..

IMPALA-12403: Fix Kerberos authentication when connecting with a proxy
user that does not delegate another user

This change improves LDAP checking in the Hiveserver2 API where
previously LDAP filter checks were run on the user principal received
in Kerberos authentication, but after the modification they are run on
the short username taken from the Kerberos principal.

Tested with new custom cluster tests.

Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
---
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-hs2-server.cc
M be/src/transport/TSasl.cpp
M be/src/transport/TSasl.h
M be/src/transport/TSaslTransport.cpp
M be/src/transport/TSaslTransport.h
M be/src/util/auth-util.cc
M be/src/util/auth-util.h
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java
12 files changed, 128 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/20421/5
--
To view, visit http://gerrit.cloudera.org:8080/20421
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
Gerrit-Change-Number: 20421
Gerrit-PatchSet: 5
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12403: Fix Kerberos authentication when connecting with a proxy user that does not delegate another user

2023-08-29 Thread Gergely Farkas (Code Review)
Hello Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20421

to look at the new patch set (#4).

Change subject: IMPALA-12403: Fix Kerberos authentication when connecting with 
a proxy user that does not delegate another user
..

IMPALA-12403: Fix Kerberos authentication when connecting with a proxy
user that does not delegate another user

This change improves LDAP checking in the Hiveserver2 API where
previously LDAP filter checks were run on the user principal received
in Kerberos authentication, but after the modification they are run on
the short username taken from the Kerberos principal.

Tested with new custom cluster tests.

Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
---
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-hs2-server.cc
M be/src/transport/TSasl.cpp
M be/src/transport/TSasl.h
M be/src/transport/TSaslTransport.cpp
M be/src/transport/TSaslTransport.h
M be/src/util/auth-util.cc
M be/src/util/auth-util.h
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java
12 files changed, 128 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/20421/4
--
To view, visit http://gerrit.cloudera.org:8080/20421
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
Gerrit-Change-Number: 20421
Gerrit-PatchSet: 4
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12403: Fix Kerberos authentication when connecting with a proxy user that does not delegate another user

2023-08-29 Thread Gergely Farkas (Code Review)
Hello Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20421

to look at the new patch set (#3).

Change subject: IMPALA-12403: Fix Kerberos authentication when connecting with 
a proxy user that does not delegate another user
..

IMPALA-12403: Fix Kerberos authentication when connecting with a proxy
user that does not delegate another user

This change improves LDAP checking in the Hiveserver2 API where
previously LDAP filter checks were run on the user principal received
in Kerberos authentication, but after the modification they are run on
the short username taken from the Kerberos principal.

Tested with new custom cluster tests.

Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
---
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-hs2-server.cc
M be/src/transport/TSasl.cpp
M be/src/transport/TSasl.h
M be/src/transport/TSaslTransport.cpp
M be/src/transport/TSaslTransport.h
M be/src/util/auth-util.cc
M be/src/util/auth-util.h
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java
12 files changed, 127 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/20421/3
--
To view, visit http://gerrit.cloudera.org:8080/20421
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
Gerrit-Change-Number: 20421
Gerrit-PatchSet: 3
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12403: Fix Kerberos authentication when connecting with a proxy user that does not delegate another user

2023-08-25 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20421

to look at the new patch set (#2).

Change subject: IMPALA-12403: Fix Kerberos authentication when connecting with 
a proxy user that does not delegate another user
..

IMPALA-12403: Fix Kerberos authentication when connecting with a proxy
user that does not delegate another user

This change improves LDAP checking in the Hiveserver2 API where
previously LDAP filter checks were run on the user principal received
in Kerberos authentication, but after the modification they are run on
the short username taken from the Kerberos principal.

Tested with new custom cluster tests.

Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
---
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-hs2-server.cc
M be/src/transport/TSasl.cpp
M be/src/transport/TSasl.h
M be/src/transport/TSaslTransport.cpp
M be/src/transport/TSaslTransport.h
M be/src/util/auth-util.cc
M be/src/util/auth-util.h
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java
12 files changed, 129 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/20421/2
--
To view, visit http://gerrit.cloudera.org:8080/20421
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
Gerrit-Change-Number: 20421
Gerrit-PatchSet: 2
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12403: Fix Kerberos authentication when connecting with a proxy user that does not delegate another user

2023-08-25 Thread Gergely Farkas (Code Review)
Gergely Farkas has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20421


Change subject: IMPALA-12403: Fix Kerberos authentication when connecting with 
a proxy user that does not delegate another user
..

IMPALA-12403: Fix Kerberos authentication when connecting with a proxy
user that does not delegate another user

This change improves LDAP checking in the Hiveserver2 API where
previously LDAP filter checks were run on the user principal received
in Kerberos authentication, but after the modification they are run on
the short username taken from the Kerberos principal.

Tested with new custom cluster tests.

Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
---
M be/src/rpc/authentication.cc
M be/src/rpc/thrift-server.h
M be/src/service/impala-hs2-server.cc
M be/src/transport/TSasl.cpp
M be/src/transport/TSasl.h
M be/src/transport/TSaslTransport.cpp
M be/src/transport/TSaslTransport.h
M be/src/util/auth-util.cc
M be/src/util/auth-util.h
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
M 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindImpalaShellTest.java
12 files changed, 130 insertions(+), 22 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/21/20421/1
--
To view, visit http://gerrit.cloudera.org:8080/20421
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0141cd341cda84ed40cd72f8a038c8d5a215e5e1
Gerrit-Change-Number: 20421
Gerrit-PatchSet: 1
Gerrit-Owner: Gergely Farkas 


[Impala-ASF-CR] IMPALA-12341: Fix http header parsing issue in thrift http server

2023-08-15 Thread Gergely Farkas (Code Review)
Hello Jason Fehr, Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#15).

Change subject: IMPALA-12341: Fix http header parsing issue in thrift http 
server
..

IMPALA-12341: Fix http header parsing issue in thrift http server

This change fixes the following http header parsing bug in
THttpServer: The THRIFT_strncasecmp() function used in the
THttpServer::parseHeader() function returns true even if the name
of the header being processed is a prefix of the header constant
that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
This can break authentication if the http request has a header
with a name that is a prefix to the word "Authorization".
If the length of the checked header is included in the condition,
this problem is avoided, so this fix refactors the if conditions,
so that this check is present everywhere.

Tested with new custom cluster tests.

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
3 files changed, 63 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/15
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 15
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 


[Impala-ASF-CR] IMPALA-12341: Fix http header parsing issue in thrift http server

2023-08-15 Thread Gergely Farkas (Code Review)
Hello Jason Fehr, Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#14).

Change subject: IMPALA-12341: Fix http header parsing issue in thrift http 
server
..

IMPALA-12341: Fix http header parsing issue in thrift http server

This change fixes the following http header parsing bug in
THttpServer: The THRIFT_strncasecmp() function used in the
THttpServer::parseHeader() function returns true even if the name
of the header being processed is a prefix of the header constant
that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
This can break authentication if the http request has a header
with a name that is a prefix to the word "Authorization".
If the length of the checked header is included in the condition,
this problem is avoided, so this fix refactors the if conditions,
so that this check is present everywhere.

Tested with new custom cluster tests.

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
3 files changed, 63 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/14
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 14
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 


[Impala-ASF-CR] IMPALA-12341: Fix http header parsing issue in thrift http server

2023-08-15 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12341: Fix http header parsing issue in thrift http 
server
..


Patch Set 13:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20301/12/be/src/transport/THttpServer.h
File be/src/transport/THttpServer.h:

http://gerrit.cloudera.org:8080/#/c/20301/12/be/src/transport/THttpServer.h@162
PS12, Line 162:   static const std::string HEADER_SAML2_TOKEN_RESPONSE_PORT;
> Not sure here, but prefixing with HEADER_ as the constants above would be m
Good point! I'll change the names to be more consistent.


http://gerrit.cloudera.org:8080/#/c/20301/12/be/src/transport/THttpServer.cpp
File be/src/transport/THttpServer.cpp:

http://gerrit.cloudera.org:8080/#/c/20301/12/be/src/transport/THttpServer.cpp@132
PS12, Line 132: inline const bool Matc
> We should simply use a function here IMO - THRIFT_strncasecmp is created wi
Okay, I'll introduce a new inline function instead of this.



--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 13
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Comment-Date: Tue, 15 Aug 2023 08:45:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12341: Fix http header parsing issue in thrift http server

2023-08-15 Thread Gergely Farkas (Code Review)
Hello Jason Fehr, Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#13).

Change subject: IMPALA-12341: Fix http header parsing issue in thrift http 
server
..

IMPALA-12341: Fix http header parsing issue in thrift http server

This change fixes the following http header parsing bug in
THttpServer: The THRIFT_strncasecmp() function used in the
THttpServer::parseHeader() function returns true even if the name
of the header being processed is a prefix of the header constant
that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
This can break authentication if the http request has a header
with a name that is a prefix to the word "Authorization".
If the length of the checked header is included in the condition,
this problem is avoided, so this fix refactors the if conditions,
so that this check is present everywhere.

Tested with new custom cluster tests.

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
3 files changed, 63 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/13
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 13
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 


[Impala-ASF-CR] IMPALA-12341: Fix http header parsing issue in thrift http server

2023-08-09 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12341: Fix http header parsing issue in thrift http 
server
..


Patch Set 12:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20301/11//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20301/11//COMMIT_MSG@7
PS11, Line 7: IMPALA-12341
> Just realized that this issue should also appear in Apache Thrift's c++ lib
I created a bug ticket in the Thrift project: 
https://issues.apache.org/jira/browse/THRIFT-5730


http://gerrit.cloudera.org:8080/#/c/20301/11/be/src/transport/THttpServer.cpp
File be/src/transport/THttpServer.cpp:

http://gerrit.cloudera.org:8080/#/c/20301/11/be/src/transport/THttpServer.cpp@143
PS11, Line 143: const std::string THttpServer::X_FORWARDED_FOR = 
"X-Forwarded-For";
> +1 for moving these to constants
I created the string constants and a new MATCHES_HEADER macro. (I didn't want 
to modify the original THRIFT_strncasecmp.)



--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 12
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Comment-Date: Wed, 09 Aug 2023 14:57:07 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12341: Fix http header parsing issue in thrift http server

2023-08-09 Thread Gergely Farkas (Code Review)
Hello Jason Fehr, Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#12).

Change subject: IMPALA-12341: Fix http header parsing issue in thrift http 
server
..

IMPALA-12341: Fix http header parsing issue in thrift http server

This change fixes the following http header parsing bug in
THttpServer: The THRIFT_strncasecmp() function used in the
THttpServer::parseHeader() function returns true even if the name
of the header being processed is a prefix of the header constant
that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
This can break authentication if the http request has a header
with a name that is a prefix to the word "Authorization".
If the length of the checked header is included in the condition,
this problem is avoided, so this fix refactors the if conditions,
so that this check is present everywhere.

Tested with new custom cluster tests.

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M be/src/transport/THttpServer.h
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
3 files changed, 61 insertions(+), 18 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/12
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 12
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 


[Impala-ASF-CR] IMPALA-12341: Fix http header parsing issue in thrift http server

2023-08-08 Thread Gergely Farkas (Code Review)
Hello Jason Fehr, Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#11).

Change subject: IMPALA-12341: Fix http header parsing issue in thrift http 
server
..

IMPALA-12341: Fix http header parsing issue in thrift http server

This change fixes the following http header parsing bug in
THttpServer: The THRIFT_strncasecmp() function used in the
THttpServer::parseHeader() function returns true even if the name
of the header being processed is a prefix of the header constant
that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
This can break authentication if the http request has a header
with a name that is a prefix to the word "Authorization".
If the length of the checked header is included in the condition,
this problem is avoided, so this fix refactors the if conditions,
so that this check is present everywhere.

Tested with new custom cluster tests.

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
2 files changed, 51 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/11
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 11
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20301/10/be/src/transport/THttpServer.cpp
File be/src/transport/THttpServer.cpp:

http://gerrit.cloudera.org:8080/#/c/20301/10/be/src/transport/THttpServer.cpp@150
PS10, Line 150:   && THRIFT_strncasecmp(header, TRANSFER_ENCODING, sz) == 
0) {
> Based on my reading of the docs and some experimentation with code, we shou
Unfortunately, when parsing the following header
"Transfer-Encoding-the: fake",
the condition that does not use the 'sz' variable will be true, because the 
first 17 characters will match:
THRIFT_strncasecmp(header, TRANSFER_ENCODING, strlen(TRANSFER_ENCODING)) == 0



--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 10
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Jason Fehr 
Gerrit-Comment-Date: Thu, 03 Aug 2023 20:03:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/20301/8/be/src/transport/THttpServer.cpp
File be/src/transport/THttpServer.cpp:

http://gerrit.cloudera.org:8080/#/c/20301/8/be/src/transport/THttpServer.cpp@139
PS8, Line 139:
> Okay!
Done.



--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 10
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 03 Aug 2023 16:14:46 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Gergely Farkas (Code Review)
Hello Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#9).

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 117 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/9
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 9
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Gergely Farkas (Code Review)
Hello Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#10).

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 118 insertions(+), 32 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/10
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 10
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20301 )

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..


Patch Set 8:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/20301/8//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20301/8//COMMIT_MSG@10
PS8, Line 10: This change introduces a new optional impala-shell option
: (send_auth_header_for_hiveserver2) to add an 'auth' header to http
: requests, which is expected in latest hiveserver2 builds
: when supporting multiple authentication modes at once
: (see HIVE-27352).
> There is already an option called strict_hs2_protocol which is used when we
I didn't dare to put it under strict_hs2_protocol, because then if someone uses 
a new impala-shell with an old impala build and sets strict_hs2_protocol when 
connecting to the coordinator, then the impala-shell will send the 'auth' 
header and authentication in hs2-http requests will not work because of the old 
impala build in the coordinator. I don't know much about impala users, if this 
is not a real use-case, then this can of course be added under 
strict_hs2_protocol.


http://gerrit.cloudera.org:8080/#/c/20301/8/be/src/transport/THttpServer.cpp
File be/src/transport/THttpServer.cpp:

http://gerrit.cloudera.org:8080/#/c/20301/8/be/src/transport/THttpServer.cpp@139
PS8, Line 139: _
> Thanks for fixing this bug!
Okay!
This length check was my original idea too, but the conditions seemed pretty 
ugly to me, so I moved to the current one. I'll revert back to that solution.



--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 8
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 03 Aug 2023 14:59:51 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-03 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#8).

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 106 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/8
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 8
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-02 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#7).

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 106 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/7
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 7
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-02 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#6).

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 106 insertions(+), 38 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/6
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 6
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-02 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#5).

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 106 insertions(+), 36 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/5
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 5
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-02 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#4).

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 124 insertions(+), 53 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/4
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 4
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-02 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#3).

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 91 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/3
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 3
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-02 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/20301

to look at the new patch set (#2).

Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 91 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/2
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 2
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-12328: New option for impala-shell to send "auth" header in thrift http request

2023-08-02 Thread Gergely Farkas (Code Review)
Gergely Farkas has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/20301


Change subject: IMPALA-12328: New option for impala-shell to send "auth" header 
in thrift http request
..

IMPALA-12328: New option for impala-shell to send "auth" header in thrift
http request

This change introduces a new optional impala-shell option
(send_auth_header_for_hiveserver2) to add an 'auth' header to http
requests, which is expected in latest hiveserver2 builds
when supporting multiple authentication modes at once
(see HIVE-27352).
The 'auth' header shouldn't be a problem for the impala coordinator,
but unfortunately it may prevent authentication due to a buggy header
parser code. This change will also fix that bug and introduces new
java frontend tests to validate the changes.

Some information on the http header parsing bug in THttpServer:
Unfortunately, the THRIFT_strncasecmp() function used in the original
implementation was true even if the name of the header being processed
was a prefix of the header name that is defined in the condition.
For example: In the original implementation when processing the
http header "auth: anyValue", we run into the code fragment where
the Authorization header content is processed, because the condition
THRIFT_strncasecmp("auth: anyValue", "Authorization", 4) == 0)
is true, since the first 4 characters of the two strings are the same.
Unfortunately, this can break authentication if the http request
has a header with a name that is a prefix to the word "Authorization".

Tested with a snapshot build in a CDP PVC DS environment where
both LDAP and Kerberos authentication was enabled:
- connected to Impala Coordinator using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment
- connected to Impala Coordinator using beeline and hive jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to Impala Coordinator using beeline and impala jdbc driver
with LDAP authentication and with Kerberos authentication,
- connected to hiveserver2 using the impala-shell client with
and without the new send_auth_header_for_hiveserver2 option
with LDAP authentication and with Kerberos authentication,
in python2 and also in python3 environment

Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
---
M be/src/transport/THttpServer.cpp
M fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
M shell/ImpalaHttpClient.py
M shell/impala_client.py
M shell/impala_shell.py
M shell/option_parser.py
7 files changed, 89 insertions(+), 20 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/01/20301/1
--
To view, visit http://gerrit.cloudera.org:8080/20301
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I754639cfe3eab0016d09f71ded4821caa357bf87
Gerrit-Change-Number: 20301
Gerrit-PatchSet: 1
Gerrit-Owner: Gergely Farkas 


[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-10 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19561 )

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..


Patch Set 10:

> Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/9131/

one dockerised test failed that doesn't seem to be related to the change :/
metadata.test_event_processing.TestEventProcessing.test_transactional_insert_events
 (from pytest)

stacktrace:
metadata/test_event_processing.py:38: in test_transactional_insert_events
self.run_test_insert_events(unique_database, is_transactional=True)
metadata/test_event_processing.py:137: in run_test_insert_events
EventProcessorUtils.wait_for_event_processing(self)
util/event_processor_utils.py:66: in wait_for_event_processing
within {1} seconds".format(current_event_id, timeout))
E   Exception: Event processor did not sync till last known event id 26421  
   within 10 seconds

https://jenkins.impala.io/job/ubuntu-16.04-dockerised-tests/7117/testReport/junit/metadata.test_event_processing/TestEventProcessing/test_transactional_insert_events/


--
To view, visit http://gerrit.cloudera.org:8080/19561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
Gerrit-Change-Number: 19561
Gerrit-PatchSet: 10
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Fri, 10 Mar 2023 13:48:57 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-09 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19561 )

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..


Patch Set 7:

(4 comments)

http://gerrit.cloudera.org:8080/#/c/19561/7/be/src/rpc/authentication-test.cc
File be/src/rpc/authentication-test.cc:

http://gerrit.cloudera.org:8080/#/c/19561/7/be/src/rpc/authentication-test.cc@27
PS7, Line 27: #include "util/kudu-status-util.h"
: #include "kudu/security/test/mini_kdc.h"
> includes are generally sorted ¬alphabetically in Impala
Done


http://gerrit.cloudera.org:8080/#/c/19561/7/fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java
File 
fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java:

http://gerrit.cloudera.org:8080/#/c/19561/7/fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java@102
PS7, Line 102:
> nit: extra line
Done


http://gerrit.cloudera.org:8080/#/c/19561/7/fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
File 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java:

http://gerrit.cloudera.org:8080/#/c/19561/7/fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java@273
PS7, Line 273: TEST_USER_1
> The proxy and the delegated user are the same here, which seems like a spec
I did not see that this case special, I thought that from the test point of 
view it doesn't matter who the delegated user is. To avoid confusion, I defined 
a new LDAP user and used it as the delegated user.


http://gerrit.cloudera.org:8080/#/c/19561/7/fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java@502
PS7, Line 502: enable_ldap_auth
> This seems to be exactly the same as the ldap flags in testShellKerberosAut
You are right! I removed the code duplication and created two new helper 
functions: getLdapSimpleBindFlags() and 
getCustomLdapSimpleBindSearchFilterFlags().



--
To view, visit http://gerrit.cloudera.org:8080/19561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
Gerrit-Change-Number: 19561
Gerrit-PatchSet: 7
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Thu, 09 Mar 2023 21:30:52 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-09 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19561 )

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..


Patch Set 9:

> Build Failed
 >
 > https://jenkins.impala.io/job/gerrit-code-review-checks/12600/ :
 > Initial code review checks failed. See linked job for details on
 > the failure.

Failed due to the AWS error below:

16:36:43 [ERROR] Failed to execute goal on project impala-frontend: Could not 
resolve dependencies for project 
org.apache.impala:impala-frontend:jar:4.3.0-SNAPSHOT: Failed to collect 
dependencies at org.apache.hive:hive-jdbc:jar:3.1.3000.7.2.17.0-127: Failed to 
read artifact descriptor for 
org.apache.hive:hive-jdbc:jar:3.1.3000.7.2.17.0-127: Could not transfer 
artifact org.apache.hive:hive-jdbc:pom:3.1.3000.7.2.17.0-127 from/to 
impala.cdp.repo 
(https://native-toolchain.s3.amazonaws.com/build/cdp_components/38235009/maven):
 Failed to transfer file: 
https://native-toolchain.s3.amazonaws.com/build/cdp_components/38235009/maven/org/apache/hive/hive-jdbc/3.1.3000.7.2.17.0-127/hive-jdbc-3.1.3000.7.2.17.0-127.pom.
 Return code is: 503 , ReasonPhrase:Service Unavailable. -> [Help 1]


--
To view, visit http://gerrit.cloudera.org:8080/19561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
Gerrit-Change-Number: 19561
Gerrit-PatchSet: 9
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Thu, 09 Mar 2023 21:21:40 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-09 Thread Gergely Farkas (Code Review)
Hello Tamas Mate, Csaba Ringhofer, Wenzhe Zhou, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/19561

to look at the new patch set (#9).

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..

IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

This change does two things for the Kerberos authentication support
for impala clients:

1) Introduces allow_custom_ldap_filters_with_kerberos_auth flag,
which removes the restriction that prevents to use LDAP group/user
search filters when Kerberos authentication is enabled. When the flag
is set both Kerberos and LDAP can work with impala clients
(impala-shell, jdbc, odbc, impyla) even if the group/user filters are
defined. The flag default value is false, which ensures backwards
compatibility.

2) Introduces enable_group_filter_check_for_authenticated_kerberos_user
flag, which allows group filters to be applied for non-proxy users
that belong to the authenticated Kerberos principals.
The verified username comes from the Kerberos principal: The username
is the first member of the authenticated Kerberos principal, where the
principal can be username/host@realm or username@realm.
Regardless of whether the flag is enabled or not, LDAP filters are not
applied for authorized proxy users (neither when using LDAP nor when
using Kerberos authentication). In case of delegation, filters are
applied for delegated users.
This flag makes sense if Kerberos and LDAP authentication is enabled
and the users in the KDC and LDAP are synchronized (e.g. Active
Directory provides both LDAP and Kerberos authentication).
The flag default value is false, which ensures backwards compatibility.

Notes:

If the allow_custom_ldap_filters_with_kerberos_auth flag is disabled,
it is still possible to use LDAP and Kerberos authentication together,
but in a limited way: Only LDAP search bind authentication mode can be
used, where there are default user and group search filters (that are
defined for Active Directory LDAP schema). One major limitation here
- apart from the AD directory schema assumed in the default filters -
is that the only possibility to control user access is to select the
appropriate user and group search base dn (e.g. granting LDAP access
to users/groups defined in a given subtree)
Even in this edge case, it is still allowed to enable the
enable_group_filter_check_for_authenticated_kerberos_user flag. If this
happens, then the default filters in LDAP search bind will be applied
for Kerberos authenticated non-proxy users.

Another edge case where the LDAP authentication is enabled, the
user access is controlled by custom LDAP filters (LDAP auth only),
and the external Kerberos authentication is also enabled, but the users
in KDC and LDAP are not in sync:
In this case the allow_custom_ldap_filters_with_kerberos_auth flag must
be set, but enable_group_filter_check_for_authenticated_kerberos_user
flag should be disabled, otherwise an unauthorized response may be
received during Kerberos authentication (depending on whether the
authenticated Kerberos user passes the custom LDAP filters or not).
In such cases, access to Kerberos users must be controlled by other
ways (e.g. within FreeIPA KDC with host-based access control rules).

Tests:
- New unit test created to check the behavior of AuthManager with
  and without allow_custom_ldap_filters_with_kerberos_auth flag.
- New custom cluster tests created:
  - impala-shell tests that validate existing LDAP search bind
and simple bind functionality with Kerberos authentication
enabled (LdapSearchBindImpalaShellTest and
LdapSimpleBindImpalaShellTest suites are now parameterized),
  - impala-shell tests that validate backwards compatibility
when allow_custom_ldap_filters_with_kerberos_auth flag and
enable_group_filter_check_for_authenticated_kerberos_user
flags are disabled
(LdapSearchBindDefaultFiltersKerberosImpalaShellTest)
  - various impala-shell tests that validate Kerberos
authentication in an environment where LDAP authentication
is also enabled (LdapKerberosImpalaShellTest)
- Manual tests with a snapshot build in CDP PVC DS with LDAP and
  Kerberos authentication enabled, user and group filters provided.

Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
---
M be/src/rpc/authentication-test.cc
M be/src/rpc/authentication.cc
M be/src/util/ldap-util.cc
M bin/rat_exclude_files.txt
A fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTestBase.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindDefaultFiltersKerberosImpalaShellTest.java

[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-09 Thread Gergely Farkas (Code Review)
Hello Tamas Mate, Csaba Ringhofer, Wenzhe Zhou, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/19561

to look at the new patch set (#8).

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..

IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

This change does two things for the Kerberos authentication support
for impala clients:

1) Introduces allow_custom_ldap_filters_with_kerberos_auth flag,
which removes the restriction that prevents to use LDAP group/user
search filters when Kerberos authentication is enabled. When the flag
is set both Kerberos and LDAP can work with impala clients
(impala-shell, jdbc, odbc, impyla) even if the group/user filters are
defined. The flag default value is false, which ensures backwards
compatibility.

2) Introduces enable_group_filter_check_for_authenticated_kerberos_user
flag, which allows group filters to be applied for non-proxy users
that belong to the authenticated Kerberos principals.
The verified username comes from the Kerberos principal: The username
is the first member of the authenticated Kerberos principal, where the
principal can be username/host@realm or username@realm.
Regardless of whether the flag is enabled or not, LDAP filters are not
applied for authorized proxy users (neither when using LDAP nor when
using Kerberos authentication). In case of delegation, filters are
applied for delegated users.
This flag makes sense if Kerberos and LDAP authentication is enabled
and the users in the KDC and LDAP are synchronized (e.g. Active
Directory provides both LDAP and Kerberos authentication).
The flag default value is false, which ensures backwards compatibility.

Notes:

If the allow_custom_ldap_filters_with_kerberos_auth flag is disabled,
it is still possible to use LDAP and Kerberos authentication together,
but in a limited way: Only LDAP search bind authentication mode can be
used, where there are default user and group search filters (that are
defined for Active Directory LDAP schema). One major limitation here
- apart from the AD directory schema assumed in the default filters -
is that the only possibility to control user access is to select the
appropriate user and group search base dn (e.g. granting LDAP access
to users/groups defined in a given subtree)
Even in this edge case, it is still allowed to enable the
enable_group_filter_check_for_authenticated_kerberos_user flag. If this
happens, then the default filters in LDAP search bind will be applied
for Kerberos authenticated non-proxy users.

Another edge case where the LDAP authentication is enabled, the
user access is controlled by custom LDAP filters (LDAP auth only),
and the external Kerberos authentication is also enabled, but the users
in KDC and LDAP are not in sync:
In this case the allow_custom_ldap_filters_with_kerberos_auth flag must
be set, but enable_group_filter_check_for_authenticated_kerberos_user
flag should be disabled, otherwise an unauthorized response may be
received during Kerberos authentication (depending on whether the
authenticated Kerberos user passes the custom LDAP filters or not).
In such cases, access to Kerberos users must be controlled by other
ways (e.g. within FreeIPA KDC with host-based access control rules).

Tests:
- New unit test created to check the behavior of AuthManager with
  and without allow_custom_ldap_filters_with_kerberos_auth flag.
- New custom cluster tests created:
  - impala-shell tests that validate existing LDAP search bind
and simple bind functionality with Kerberos authentication
enabled (LdapSearchBindImpalaShellTest and
LdapSimpleBindImpalaShellTest suites are now parameterized),
  - impala-shell tests that validate backwards compatibility
when allow_custom_ldap_filters_with_kerberos_auth flag and
enable_group_filter_check_for_authenticated_kerberos_user
flags are disabled
(LdapSearchBindDefaultFiltersKerberosImpalaShellTest)
  - various impala-shell tests that validate Kerberos
authentication in an environment where LDAP authentication
is also enabled (LdapKerberosImpalaShellTest)
- Manual tests with a snapshot build in CDP PVC DS with LDAP and
  Kerberos authentication enabled, user and group filters provided.

Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
---
M be/src/rpc/authentication-test.cc
M be/src/rpc/authentication.cc
M be/src/util/ldap-util.cc
M bin/rat_exclude_files.txt
A fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTestBase.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindDefaultFiltersKerberosImpalaShellTest.java

[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-07 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19561 )

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19561/6/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:

http://gerrit.cloudera.org:8080/#/c/19561/6/be/src/rpc/authentication.cc@988
PS6, Line 988: if (FLAGS_enable_ldap_auth || IsKerberosEnabled()) {
> Thank you for the detailed answer. Just trying to better understand the exe
Makes sense!

Callbacks in KERB_INT_CALLBACKS and KERB_EXT_CALLBACKS aren't chained, since 
the callbacks in KERB_INT_CALLBACKS are configured only on the ports used for 
(internal) communication between daemons. On the other hand, callbacks in 
KERB_EXT_CALLBACKS are configured on ports used for external communication.
Callbacks in KERB_EXT_CALLBACKS and LDAP_EXT_CALLBACKS are not chained, because 
they are involved in different authentication mechanisms: Callbacks in 
LDAP_EXT_CALLBACKS are used when the authentication mechanism is PLAIN, where 
we have a username and a password, while callbacks in KERB_EXT_CALLBACKS (or in 
KERB_INT_CALLBACKS) are used when the authentication mechanism is GSSAPI, where 
we don't have user and password, but only a user principal.

My first idea was to reuse the already implemented SaslLdapCheckPass function, 
and add a SASL_CB_SERVER_USERDB_CHECKPASS callback to the Kerberos callbacks. 
That's what I did, and I also created a new unit test for validating the 
behavior, but the test did not pass. It turned out that this callback does not 
play a role in GSSAPI auth scenario, so defining this callback in the Kerberos 
callbacks does not solve the problem.



--
To view, visit http://gerrit.cloudera.org:8080/19561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
Gerrit-Change-Number: 19561
Gerrit-PatchSet: 6
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Tue, 07 Mar 2023 11:31:03 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-06 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19561 )

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..


Patch Set 6:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/19561/6/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:

http://gerrit.cloudera.org:8080/#/c/19561/6/be/src/rpc/authentication.cc@284
PS6, Line 284:<< "to an impalad.";
> You are right. No change then
Ok, thanks!


http://gerrit.cloudera.org:8080/#/c/19561/6/be/src/rpc/authentication.cc@570
PS6, Line 570:  const char* requested_user, 
unsigned rlen,
 :  const char* auth_identity, 
unsigned alen,
 :  const char* def_realm, 
unsigned urlen,
 :  struct propctx* propctx) {
> nit: Could we keep the original formatting? It is closer to the one used in
Thanks for the review!
You are right! I've updated these lines.


http://gerrit.cloudera.org:8080/#/c/19561/6/be/src/rpc/authentication.cc@988
PS6, Line 988: if (FLAGS_enable_ldap_auth || IsKerberosEnabled()) {
> Do I understand it right that, this will evaluate to true, then the next co
Yes, if both LDAP and Kerberos authentication mode are enabled, the following 
code snippet will generate 3 callback sets: one for external Kerberos 
connections, one for internal Kerberos connections and one for Ldap 
authentication. These callback configurations are used by the 
GetServerTransportFactory when it creates AuthProvider instances to binary 
protocols. For Thrift Http protocols, they are not used, but the NegotiateAuth 
function starting on L714.



--
To view, visit http://gerrit.cloudera.org:8080/19561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
Gerrit-Change-Number: 19561
Gerrit-PatchSet: 6
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Tue, 07 Mar 2023 07:22:17 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-06 Thread Gergely Farkas (Code Review)
Hello Tamas Mate, Csaba Ringhofer, Wenzhe Zhou, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/19561

to look at the new patch set (#7).

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..

IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

This change does two things for the Kerberos authentication support
for impala clients:

1) Introduces allow_custom_ldap_filters_with_kerberos_auth flag,
which removes the restriction that prevents to use LDAP group/user
search filters when Kerberos authentication is enabled. When the flag
is set both Kerberos and LDAP can work with impala clients
(impala-shell, jdbc, odbc, impyla) even if the group/user filters are
defined. The flag default value is false, which ensures backwards
compatibility.

2) Introduces enable_group_filter_check_for_authenticated_kerberos_user
flag, which allows group filters to be applied for non-proxy users
that belong to the authenticated Kerberos principals.
The verified username comes from the Kerberos principal: The username
is the first member of the authenticated Kerberos principal, where the
principal can be username/host@realm or username@realm.
Regardless of whether the flag is enabled or not, LDAP filters are not
applied for authorized proxy users (neither when using LDAP nor when
using Kerberos authentication). In case of delegation, filters are
applied for delegated users.
This flag makes sense if Kerberos and LDAP authentication is enabled
and the users in the KDC and LDAP are synchronized (e.g. Active
Directory provides both LDAP and Kerberos authentication).
The flag default value is false, which ensures backwards compatibility.

Notes:

If the allow_custom_ldap_filters_with_kerberos_auth flag is disabled,
it is still possible to use LDAP and Kerberos authentication together,
but in a limited way: Only LDAP search bind authentication mode can be
used, where there are default user and group search filters (that are
defined for Active Directory LDAP schema). One major limitation here
- apart from the AD directory schema assumed in the default filters -
is that the only possibility to control user access is to select the
appropriate user and group search base dn (e.g. granting LDAP access
to users/groups defined in a given subtree)
Even in this edge case, it is still allowed to enable the
enable_group_filter_check_for_authenticated_kerberos_user flag. If this
happens, then the default filters in LDAP search bind will be applied
for Kerberos authenticated non-proxy users.

Another edge case where the LDAP authentication is enabled, the
user access is controlled by custom LDAP filters (LDAP auth only),
and the external Kerberos authentication is also enabled, but the users
in KDC and LDAP are not in sync:
In this case the allow_custom_ldap_filters_with_kerberos_auth flag must
be set, but enable_group_filter_check_for_authenticated_kerberos_user
flag should be disabled, otherwise an unauthorized response may be
received during Kerberos authentication (depending on whether the
authenticated Kerberos user passes the custom LDAP filters or not).
In such cases, access to Kerberos users must be controlled by other
ways (e.g. within FreeIPA KDC with host-based access control rules).

Tests:
- New unit test created to check the behavior of AuthManager with
  and without allow_custom_ldap_filters_with_kerberos_auth flag.
- New custom cluster tests created:
  - impala-shell tests that validate existing LDAP search bind
and simple bind functionality with Kerberos authentication
enabled (LdapSearchBindImpalaShellTest and
LdapSimpleBindImpalaShellTest suites are now parameterized),
  - impala-shell tests that validate backwards compatibility
when allow_custom_ldap_filters_with_kerberos_auth flag and
enable_group_filter_check_for_authenticated_kerberos_user
flags are disabled
(LdapSearchBindDefaultFiltersKerberosImpalaShellTest)
  - various impala-shell tests that validate Kerberos
authentication in an environment where LDAP authentication
is also enabled (LdapKerberosImpalaShellTest)
- Manual tests with a snapshot build in CDP PVC DS with LDAP and
  Kerberos authentication enabled, user and group filters provided.

Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
---
M be/src/rpc/authentication-test.cc
M be/src/rpc/authentication.cc
M be/src/util/ldap-util.cc
M bin/rat_exclude_files.txt
A fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTestBase.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindDefaultFiltersKerberosImpalaShellTest.java

[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-03 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19561 )

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..


Patch Set 6:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/19561/6/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:

http://gerrit.cloudera.org:8080/#/c/19561/6/be/src/rpc/authentication.cc@284
PS6, Line 284:<< "to an impalad.";
> return false to avoid accessing 'server'
Thanks for reviewing this change!
I thought that LOG(FATAL) would stop the program from running. Is that not 
correct?



--
To view, visit http://gerrit.cloudera.org:8080/19561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
Gerrit-Change-Number: 19561
Gerrit-PatchSet: 6
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Wenzhe Zhou 
Gerrit-Comment-Date: Fri, 03 Mar 2023 08:38:14 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-02 Thread Gergely Farkas (Code Review)
Hello Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/19561

to look at the new patch set (#6).

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..

IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

This change does two things for the Kerberos authentication support
for impala clients:

1) Introduces allow_custom_ldap_filters_with_kerberos_auth flag,
which removes the restriction that prevents to use LDAP group/user
search filters when Kerberos authentication is enabled. When the flag
is set both Kerberos and LDAP can work with impala clients
(impala-shell, jdbc, odbc, impyla) even if the group/user filters are
defined. The flag default value is false, which ensures backwards
compatibility.

2) Introduces enable_group_filter_check_for_authenticated_kerberos_user
flag, which allows group filters to be applied for non-proxy users
that belong to the authenticated Kerberos principals.
The verified username comes from the Kerberos principal: The username
is the first member of the authenticated Kerberos principal, where the
principal can be username/host@realm or username@realm.
Regardless of whether the flag is enabled or not, LDAP filters are not
applied for authorized proxy users (neither when using LDAP nor when
using Kerberos authentication). In case of delegation, filters are
applied for delegated users.
This flag makes sense if Kerberos and LDAP authentication is enabled
and the users in the KDC and LDAP are synchronized (e.g. Active
Directory provides both LDAP and Kerberos authentication).
The flag default value is false, which ensures backwards compatibility.

Notes:

If the allow_custom_ldap_filters_with_kerberos_auth flag is disabled,
it is still possible to use LDAP and Kerberos authentication together,
but in a limited way: Only LDAP search bind authentication mode can be
used, where there are default user and group search filters (that are
defined for Active Directory LDAP schema). One major limitation here
- apart from the AD directory schema assumed in the default filters -
is that the only possibility to control user access is to select the
appropriate user and group search base dn (e.g. granting LDAP access
to users/groups defined in a given subtree)
Even in this edge case, it is still allowed to enable the
enable_group_filter_check_for_authenticated_kerberos_user flag. If this
happens, then the default filters in LDAP search bind will be applied
for Kerberos authenticated non-proxy users.

Another edge case where the LDAP authentication is enabled, the
user access is controlled by custom LDAP filters (LDAP auth only),
and the external Kerberos authentication is also enabled, but the users
in KDC and LDAP are not in sync:
In this case the allow_custom_ldap_filters_with_kerberos_auth flag must
be set, but enable_group_filter_check_for_authenticated_kerberos_user
flag should be disabled, otherwise an unauthorized response may be
received during Kerberos authentication (depending on whether the
authenticated Kerberos user passes the custom LDAP filters or not).
In such cases, access to Kerberos users must be controlled by other
ways (e.g. within FreeIPA KDC with host-based access control rules).

Tests:
- New unit test created to check the behavior of AuthManager with
  and without allow_custom_ldap_filters_with_kerberos_auth flag.
- New custom cluster tests created:
  - impala-shell tests that validate existing LDAP search bind
and simple bind functionality with Kerberos authentication
enabled (LdapSearchBindImpalaShellTest and
LdapSimpleBindImpalaShellTest suites are now parameterized),
  - impala-shell tests that validate backwards compatibility
when allow_custom_ldap_filters_with_kerberos_auth flag and
enable_group_filter_check_for_authenticated_kerberos_user
flags are disabled
(LdapSearchBindDefaultFiltersKerberosImpalaShellTest)
  - various impala-shell tests that validate Kerberos
authentication in an environment where LDAP authentication
is also enabled (LdapKerberosImpalaShellTest)
- Manual tests with a snapshot build in CDP PVC DS with LDAP and
  Kerberos authentication enabled, user and group filters provided.

Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
---
M be/src/rpc/authentication-test.cc
M be/src/rpc/authentication.cc
M be/src/util/ldap-util.cc
M bin/rat_exclude_files.txt
A fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTestBase.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindDefaultFiltersKerberosImpalaShellTest.java
M 

[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-02 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19561 )

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..


Patch Set 4:

(9 comments)

http://gerrit.cloudera.org:8080/#/c/19561/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/19561/4//COMMIT_MSG@10
PS4, Line 10: impala-shell
> I think that this should affect any Impala client that uses Kerberos (e.g.
Good point!


http://gerrit.cloudera.org:8080/#/c/19561/4//COMMIT_MSG@12
PS4, Line 12: allow_custom_ldap_filters_with_kerberos_auth
> Do the flags make sense in any combination, e.g. allow_custom_ldap_filters_
I've added some notes on edge cases.


http://gerrit.cloudera.org:8080/#/c/19561/4//COMMIT_MSG@14
PS4, Line 14: search filters when Kerberos authentication is enabled. When the 
flag
> Can you mention proxy users? My understanding is that LDAP filters will be
I updated the commit message with some related thoughts and created two new 
tests: The first test validates that default LDAP search bind filters are 
applied for delegate users, even if the proxy user authenticated with Kerberos. 
This test should run green without the changes in this gerrit.
The other test validates that the custom LDAP search filters are applied for 
delegate users even if the 
enable_group_filter_check_for_authenticated_kerberos_user is disabled and the 
proxy user authenticated with Kerberos.


http://gerrit.cloudera.org:8080/#/c/19561/4/be/src/rpc/authentication.cc
File be/src/rpc/authentication.cc:

http://gerrit.cloudera.org:8080/#/c/19561/4/be/src/rpc/authentication.cc@290
PS4, Line 290:   if (!server->IsAuthorizedProxyUser(user)) {
 : return ldap->LdapCheckFilters(user);
 :   }
 :   return true;
> optional: IMO reversing the order and returning early if IsAuthorizedProxyU
Done


http://gerrit.cloudera.org:8080/#/c/19561/4/be/src/rpc/authentication.cc@302
PS4, Line 302:   if (success) {
 : return DoLdapCheckFilters(user);
 :   }
 :
 :   return false;
> optional: IMO reversing the order and returning early if !success would be
Done


http://gerrit.cloudera.org:8080/#/c/19561/4/be/src/rpc/authentication.cc@523
PS4, Line 523: SaslAuthorizeExternal
> maming: could be SaslKerberosAuthorizeExternal?
Done


http://gerrit.cloudera.org:8080/#/c/19561/4/be/src/rpc/authentication.cc@528
PS4, Line 528: IsKerberosEnabled
> We should only get here with Kerberos, so this could be a DCHECK
Done


http://gerrit.cloudera.org:8080/#/c/19561/4/be/src/rpc/authentication.cc@569
PS4, Line 569: SaslLdapAuthorizeExternal
> It is not clear to me why we check the LDAP filters for Kerberos during SAS
At first I wanted to use the SASL_CB_SERVER_USERDB_CHECKPASS callback, the code 
would have been much simpler in that way, but it seems that SASL checkpass is 
called only in PLAIN authentication mode, and in Kerberos authentication, where 
no password is present on the server side, it is not used.


http://gerrit.cloudera.org:8080/#/c/19561/4/be/src/rpc/authentication.cc@745
PS4, Line 745: IsKerberosEnabled
> This doesn't seem necessary as NegotiateAuth is only used with Kerberos and
Nice catch!



--
To view, visit http://gerrit.cloudera.org:8080/19561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
Gerrit-Change-Number: 19561
Gerrit-PatchSet: 4
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 02 Mar 2023 14:17:28 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-02 Thread Gergely Farkas (Code Review)
Hello Csaba Ringhofer, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/19561

to look at the new patch set (#5).

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..

IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

This change does two things for the Kerberos authentication support
for impala clients:

1) Introduces allow_custom_ldap_filters_with_kerberos_auth flag,
which removes the restriction that prevents to use LDAP group/user
search filters when Kerberos authentication is enabled. When the flag
is set both Kerberos and LDAP can work with impala clients
(impala-shell, jdbc, odbc, impyla) even if the group/user filters are
defined. The flag default value is false, which ensures backwards
compatibility.

2) Introduces enable_group_filter_check_for_authenticated_kerberos_user
flag, which allows group filters to be applied for non-proxy users
that belong to the authenticated Kerberos principals.
The verified username comes from the Kerberos principal: The username
is the first member of the authenticated Kerberos principal, where the
principal can be username/host@realm or username@realm.
Regardless of whether the flag is enabled or not, LDAP filters are not
applied for authorized proxy users (neither when using LDAP nor when
using Kerberos authentication). In case of delegation, filters are
applied for delegated users.
This flag makes sense if Kerberos and LDAP authentication is enabled
and the users in the KDC and LDAP are synchronized (e.g. Active
Directory provides both LDAP and Kerberos authentication).
The flag default value is false, which ensures backwards compatibility.

Notes:

If the allow_custom_ldap_filters_with_kerberos_auth flag is disabled,
it is still possible to use LDAP and Kerberos authentication together,
but in a limited way: Only LDAP search bind authentication mode can be
used, where there are default user and group search filters (that are
defined for Active Directory LDAP schema). One major limitation here
- apart from the AD directory schema assumed in the default filters -
is that the only possibility to control user access is to select the
appropriate user and group search base dn (e.g. granting LDAP access
to users/groups defined in a given subtree)
Even in this edge case, it is still allowed to enable the
enable_group_filter_check_for_authenticated_kerberos_user flag. If this
happens, then the default filters in LDAP search bind will be applied
for Kerberos authenticated non-proxy users.

Another edge case where the LDAP authentication is enabled, the
user access is controlled by custom LDAP filters (LDAP auth only),
and the external Kerberos authentication is also enabled, but the users
in KDC and LDAP are not in sync:
In this case the allow_custom_ldap_filters_with_kerberos_auth flag must
be set, but enable_group_filter_check_for_authenticated_kerberos_user
flag should be disabled, otherwise an unauthorized response may be
received during Kerberos authentication (depending on whether the
authenticated Kerberos user passes the custom LDAP filters or not).
In such cases, access to Kerberos users must be controlled by other
ways (e.g. within FreeIPA KDC with host-based access control rules).

Tests:
- New unit test created to check the behavior of AuthManager with
  and without allow_custom_ldap_filters_with_kerberos_auth flag.
- New custom cluster tests created:
  - impala-shell tests that validate existing LDAP search bind
functionality with Kerberos authentication enabled
(LdapSearchBindKerberosEnabledImpalaShellTest),
  - impala-shell tests that valudate existing LDAP simple bind
functionality with Kerberos authentication enabled
(LdapSimpleBindKerberosEnabledImpalaShellTest),
  - impala-shell tests that validate backwards compatibility
when allow_custom_ldap_filters_with_kerberos_auth flag and
enable_group_filter_check_for_authenticated_kerberos_user
flags are disabled
(LdapSearchBindDefaultFiltersKerberosImpalaShellTest)
  - various impala-shell tests that validate Kerberos
authentication in an environment where LDAP authentication
is also enabled (LdapKerberosImpalaShellTest)
- Manual tests with a snapshot build in CDP PVC DS with LDAP and
  Kerberos authentication enabled, user and group filters provided.

Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
---
M be/src/rpc/authentication-test.cc
M be/src/rpc/authentication.cc
M be/src/util/ldap-util.cc
M bin/rat_exclude_files.txt
A fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTestBase.java
A 

[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-03-01 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/19561

to look at the new patch set (#4).

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..

IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

This change does two things for the Kerberos authentication support
for impala-shell:

1) Introduces allow_custom_ldap_filters_with_kerberos_auth flag,
which removes the restriction that prevents to use LDAP group/user
search filters when Kerberos authentication is enabled. When the flag
is set both Kerberos and LDAP can work with impala-shell even if the
group/user filters are defined. The flag default value is false,
which ensures backwards compatibility.

2) Introduces enable_group_filter_check_for_authenticated_kerberos_user
flag, which enables the check of group filters with the authenticated
Kerberos principal. This flag makes sense if Kerberos and LDAP
authentication is enabled and the users in the KDC and LDAP are
synchronized (e.g. Active Directory provides both LDAP and Kerberos
authentication).
The flag default value is false, which ensures backwards compatibility.

Tests:
- New unit test created to check the behavior of AuthManager with
  and without allow_custom_ldap_filters_with_kerberos_auth flag.
- New custom cluster tests created:
  - impala-shell tests that validate existing LDAP search bind
functionality with Kerberos authentication enabled
(LdapSearchBindKerberosEnabledImpalaShellTest),
  - impala-shell tests that valudate existing LDAP simple bind
functionality with Kerberos authentication enabled
(LdapSimpleBindKerberosEnabledImpalaShellTest),
  - impala-shell tests that validate backwards compatibility
when allow_custom_ldap_filters_with_kerberos_auth flag is
disabled (LdapSearchBindDefaultFiltersKerberosImpalaShellTest)
  - various impala-shell tests that validate Kerberos
authentication in an environment where LDAP authentication
is also enabled (LdapKerberosImpalaShellTest)
- Manual tests with a snapshot build in CDP PVC DS with LDAP and
  Kerberos authentication enabled, user and group filters provided.

Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
---
M be/src/rpc/authentication-test.cc
M be/src/rpc/authentication.cc
M be/src/util/ldap-util.cc
M bin/rat_exclude_files.txt
A fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTestBase.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindDefaultFiltersKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindKerberosEnabledImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSimpleBindKerberosEnabledImpalaShellTest.java
M fe/src/test/java/org/apache/impala/customcluster/RunShellCommand.java
A fe/src/test/resources/adschema.ldif
A fe/src/test/resources/adusers.ldif
14 files changed, 1,458 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/19561/4
--
To view, visit http://gerrit.cloudera.org:8080/19561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
Gerrit-Change-Number: 19561
Gerrit-PatchSet: 4
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-02-28 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/19561

to look at the new patch set (#3).

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..

IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

This change does two things for the Kerberos authentication support
for impala-shell:

1) Introduces allow_custom_ldap_filters_with_kerberos_auth flag,
which removes the restriction that prevents to use LDAP group/user
search filters when Kerberos authentication is enabled. When the flag
is set both Kerberos and LDAP can work with impala-shell even if the
group/user filters are defined. The flag default value is false,
which ensures backwards compatibility.

2) Introduces enable_group_filter_check_for_authenticated_kerberos_user
flag, which enables the check of group filters with the authenticated
Kerberos principal. This flag makes sense if Kerberos and LDAP
authentication is enabled and the users in the KDC and LDAP are
synchronized (e.g. Active Directory provides both LDAP and Kerberos
authentication).
The flag default value is false, which ensures backwards compatibility.

Tests:
- New unit test created to check the behavior of AuthManager with
  and without allow_custom_ldap_filters_with_kerberos_auth flag.
- New custom cluster tests created:
  - impala-shell tests that validate existing LDAP search bind
functionality with Kerberos authentication enabled
(LdapSearchBindKerberosEnabledImpalaShellTest),
  - impala-shell tests that valudate existing LDAP simple bind
functionality with Kerberos authentication enabled
(LdapSimpleBindKerberosEnabledImpalaShellTest),
  - impala-shell tests that validate backwards compatibility
when allow_custom_ldap_filters_with_kerberos_auth flag is
disabled (LdapSearchBindDefaultFiltersKerberosImpalaShellTest)
  - various impala-shell tests that validate Kerberos
authentication in an environment where LDAP authentication
is also enabled (LdapKerberosImpalaShellTest)
- Manual tests with a snapshot build in CDP PVC DS with LDAP and
  Kerberos authentication enabled, user and group filters provided.

Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
---
M be/src/rpc/authentication-test.cc
M be/src/rpc/authentication.cc
M be/src/util/ldap-util.cc
M bin/rat_exclude_files.txt
A fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTestBase.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindDefaultFiltersKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindKerberosEnabledImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSimpleBindKerberosEnabledImpalaShellTest.java
M fe/src/test/java/org/apache/impala/customcluster/RunShellCommand.java
A fe/src/test/resources/adschema.ldif
A fe/src/test/resources/adusers.ldif
14 files changed, 1,451 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/19561/3
--
To view, visit http://gerrit.cloudera.org:8080/19561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
Gerrit-Change-Number: 19561
Gerrit-PatchSet: 3
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-02-28 Thread Gergely Farkas (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/19561

to look at the new patch set (#2).

Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..

IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

This change does two things for the Kerberos authentication support
for impala-shell:

1) Introduces allow_custom_ldap_filters_with_kerberos_auth flag,
which removes the restriction that prevents to use LDAP group/user
search filters when Kerberos authentication is enabled. When the flag
is set both Kerberos and LDAP can work with impala-shell even if the
group/user filters are defined. The flag default value is false,
which ensures backwards compatibility.

2) Introduces enable_group_filter_check_for_authenticated_kerberos_user
flag, which enables the check of group filters with the authenticated
Kerberos principal. This flag makes sense if Kerberos and LDAP
authentication is enabled and the users in the KDC and LDAP are
synchronized (e.g. Active Directory provides both LDAP and Kerberos
authentication).
The flag default value is false, which ensures backwards compatibility.

Tests:
- New unit test created to check the behavior of AuthManager with
  and without allow_custom_ldap_filters_with_kerberos_auth flag.
- New custom cluster tests created:
  - impala-shell tests that validate existing LDAP search bind
functionality with Kerberos authentication enabled
(LdapSearchBindKerberosEnabledImpalaShellTest),
  - impala-shell tests that valudate existing LDAP simple bind
functionality with Kerberos authentication enabled
(LdapSimpleBindKerberosEnabledImpalaShellTest),
  - impala-shell tests that validate backwards compatibility
when allow_custom_ldap_filters_with_kerberos_auth flag is
disabled (LdapSearchBindDefaultFiltersKerberosImpalaShellTest)
  - various impala-shell tests that validate Kerberos
authentication in an environment where LDAP authentication
is also enabled (LdapKerberosImpalaShellTest)
- Manual tests with a snapshot build in CDP PVC DS with LDAP and
  Kerberos authentication enabled, user and group filters provided.

Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
---
M be/src/rpc/authentication-test.cc
M be/src/rpc/authentication.cc
M be/src/util/ldap-util.cc
A fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTestBase.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindDefaultFiltersKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindKerberosEnabledImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSimpleBindKerberosEnabledImpalaShellTest.java
M fe/src/test/java/org/apache/impala/customcluster/RunShellCommand.java
A fe/src/test/resources/adschema.ldif
A fe/src/test/resources/adusers.ldif
13 files changed, 1,449 insertions(+), 22 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/19561/2
--
To view, visit http://gerrit.cloudera.org:8080/19561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
Gerrit-Change-Number: 19561
Gerrit-PatchSet: 2
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

2023-02-28 Thread Gergely Farkas (Code Review)
Gergely Farkas has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/19561


Change subject: IMPALA-11726: Allow LDAP user and group filter when Kerberos is 
enabled
..

IMPALA-11726: Allow LDAP user and group filter when Kerberos is enabled

This change does two things for the Kerberos authentication support
for impala-shell:

1) Introduces allow_custom_ldap_filters_with_kerberos_auth flag,
which removes the restriction that prevents to use LDAP group/user
search filters when Kerberos authentication is enabled. When the flag
is set both Kerberos and LDAP can work with impala-shell even if the
group/user filters are defined. The flag default value is false,
which ensures backwards compatibility.

2) Introduces enable_group_filter_check_for_authenticated_kerberos_user
flag, which enables the check of group filters with the authenticated
Kerberos principal. This flag makes sense if Kerberos and LDAP
authentication is enabled and the users in the KDC and LDAP are
synchronized (e.g. Active Directory provides both LDAP and Kerberos
authentication).
The flag default value is false, which ensures backwards compatibility.

Tests:
- New unit test created to check the behavior of AuthManager with
  and without allow_custom_ldap_filters_with_kerberos_auth flag.
- New custom cluster tests created:
  - impala-shell tests that validate existing LDAP search bind
functionality with Kerberos authentication enabled
(LdapSearchBindKerberosEnabledImpalaShellTest),
  - impala-shell tests that valudate existing LDAP simple bind
functionality with Kerberos authentication enabled
(LdapSimpleBindKerberosEnabledImpalaShellTest),
  - impala-shell tests that validate backwards compatibility
when allow_custom_ldap_filters_with_kerberos_auth flag is
disabled (LdapSearchBindDefaultFiltersKerberosImpalaShellTest)
  - various impala-shell tests that validate Kerberos
authentication in an environment where LDAP authentication
is also enabled (LdapKerberosImpalaShellTest)
- Manual tests with a snapshot build in CDP PVC DS with LDAP and
  Kerberos authentication enabled, user and group filters provided.

Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
---
M be/src/rpc/authentication-test.cc
M be/src/rpc/authentication.cc
M be/src/util/ldap-util.cc
A fe/src/test/java/org/apache/impala/customcluster/KerberosKdcEnvironment.java
M fe/src/test/java/org/apache/impala/customcluster/LdapImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapKerberosImpalaShellTestBase.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindDefaultFiltersKerberosImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSearchBindKerberosEnabledImpalaShellTest.java
A 
fe/src/test/java/org/apache/impala/customcluster/LdapSimpleBindKerberosEnabledImpalaShellTest.java
M fe/src/test/java/org/apache/impala/customcluster/RunShellCommand.java
A fe/src/test/resources/adschema.ldif
A fe/src/test/resources/adusers.ldif
13 files changed, 1,439 insertions(+), 22 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/61/19561/1
--
To view, visit http://gerrit.cloudera.org:8080/19561
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: If3ca9c4ff8a17167e5233afabdd14c948edb46de
Gerrit-Change-Number: 19561
Gerrit-PatchSet: 1
Gerrit-Owner: Gergely Farkas 


[Impala-ASF-CR] IMPALA-11897: Fix default LDAP group search filter

2023-02-07 Thread Gergely Farkas (Code Review)
Hello Tamas Mate, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/19442

to look at the new patch set (#4).

Change subject: IMPALA-11897: Fix default LDAP group search filter
..

IMPALA-11897: Fix default LDAP group search filter

When LDAP search bind authentication is enabled
(ldap_search_bind_authentication=true) and no custom user/group
filter is specified (ldap_user_filter and ldap_group_filter
flags are empty), then LDAP auth uses the default filters that
are defined for Active Directory.
This commit fixes the incorrect default group filter and solves
the problem.

Tested manually with a custom build.

Change-Id: I3fb4e9d81b5f39b5887a296579d2a9f5199acb6d
---
M be/src/util/ldap-search-bind.cc
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/19442/4
--
To view, visit http://gerrit.cloudera.org:8080/19442
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3fb4e9d81b5f39b5887a296579d2a9f5199acb6d
Gerrit-Change-Number: 19442
Gerrit-PatchSet: 4
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 


[Impala-ASF-CR] IMPALA-11897: Fix default LDAP group search filter

2023-02-07 Thread Gergely Farkas (Code Review)
Hello Tamas Mate, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/19442

to look at the new patch set (#3).

Change subject: IMPALA-11897: Fix default LDAP group search filter
..

IMPALA-11897: Fix default LDAP group search filter

When LDAP search bind authentication is enabled
(ldap_search_bind_authentication=true) and no custom user/group
filter is specified (ldap_user_filter and ldap_group_filter
flags are empty), then LDAP auth uses the default filters that
are defined for Active Directory.
This commit fixes the incorrect default group filter and solves the problem.

Tested manually with a custom build.

Change-Id: I3fb4e9d81b5f39b5887a296579d2a9f5199acb6d
---
M be/src/util/ldap-search-bind.cc
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/19442/3
--
To view, visit http://gerrit.cloudera.org:8080/19442
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3fb4e9d81b5f39b5887a296579d2a9f5199acb6d
Gerrit-Change-Number: 19442
Gerrit-PatchSet: 3
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 


[Impala-ASF-CR] IMPALA-11897: Fix default LDAP group search filter

2023-02-03 Thread Gergely Farkas (Code Review)
Gergely Farkas has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19442 )

Change subject: IMPALA-11897: Fix default LDAP group search filter
..


Patch Set 2:

Hi Tamas!
Thanks for the hints! I've created a bug ticket and updated the commit message.


--
To view, visit http://gerrit.cloudera.org:8080/19442
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3fb4e9d81b5f39b5887a296579d2a9f5199acb6d
Gerrit-Change-Number: 19442
Gerrit-PatchSet: 2
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 
Gerrit-Comment-Date: Fri, 03 Feb 2023 13:44:08 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-11897: Fix default LDAP group search filter

2023-02-03 Thread Gergely Farkas (Code Review)
Hello Tamas Mate, Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/19442

to look at the new patch set (#2).

Change subject: IMPALA-11897: Fix default LDAP group search filter
..

IMPALA-11897: Fix default LDAP group search filter

When LDAP search bind authentication is enabled
(ldap_search_bind_authentication=true) and no custom user/group
filter is specified (ldap_user_filter and ldap_group_filter
flags are empty), then LDAP auth uses the default filters that
are defined for Active Directory.
However, due to a forgotten parenthesis, the following error
appeared in the log when trying to connect to AD:
"LDAP search failed with base DN=DC=qe-ad-1,DC=cloudera,DC=com and
filter=(&(objectClass=group)(member=CN=cm-admin,CN=Users,DC=qe-ad-1,
DC=cloudera,DC=com) : Bad search filter"
This commit fixes the incorrect group filter and solves the problem.

Tested manually with a custom build.

Change-Id: I3fb4e9d81b5f39b5887a296579d2a9f5199acb6d
---
M be/src/util/ldap-search-bind.cc
1 file changed, 1 insertion(+), 1 deletion(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/19442/2
--
To view, visit http://gerrit.cloudera.org:8080/19442
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3fb4e9d81b5f39b5887a296579d2a9f5199acb6d
Gerrit-Change-Number: 19442
Gerrit-PatchSet: 2
Gerrit-Owner: Gergely Farkas 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tamas Mate 


[Impala-ASF-CR] Fix default LDAP group search filter

2023-01-26 Thread Gergely Farkas (Code Review)
Gergely Farkas has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/19442


Change subject: Fix default LDAP group search filter
..

Fix default LDAP group search filter

Due to a forgotten parenthesis, the following errors appeared in the log when 
trying to connect to AD:
"LDAP search failed with base DN=DC=qe-ad-1,DC=cloudera,DC=com and 
filter=(&(objectClass=group)(member=CN=cm-admin,CN=Users,DC=qe-ad-1,DC=cloudera,DC=com)
 : Bad search filter"
This commit fixes the incorrect group filter and solves the problem.

Change-Id: I3fb4e9d81b5f39b5887a296579d2a9f5199acb6d
---
M be/src/util/ldap-search-bind.cc
1 file changed, 1 insertion(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/42/19442/1
--
To view, visit http://gerrit.cloudera.org:8080/19442
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3fb4e9d81b5f39b5887a296579d2a9f5199acb6d
Gerrit-Change-Number: 19442
Gerrit-PatchSet: 1
Gerrit-Owner: Gergely Farkas