This is an automated email from the ASF dual-hosted git repository.
michaelsmith pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
The following commit(s) were added to refs/heads/master by this push:
new 19d2425bb IMPALA-12505: Add flag for trusted domain check to use
'origin' if xff header is not set
19d2425bb is described below
commit 19d2425bb0f4593e3d10a072d67e6cacc037f436
Author: Gergely Farkas <[email protected]>
AuthorDate: Wed Oct 18 16:24:45 2023 +0200
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
Reviewed-on: http://gerrit.cloudera.org:8080/20591
Reviewed-by: Csaba Ringhofer <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
Reviewed-by: Michael Smith <[email protected]>
---
be/src/rpc/authentication.cc | 10 +++
be/src/transport/THttpServer.cpp | 6 ++
be/src/util/webserver.cc | 6 ++
.../apache/impala/customcluster/LdapHS2Test.java | 80 ++++++++++++++++++++++
.../impala/customcluster/LdapWebserverTest.java | 31 +++++++++
5 files changed, 133 insertions(+)
diff --git a/be/src/rpc/authentication.cc b/be/src/rpc/authentication.cc
index 7de625c8c..60c662bf7 100644
--- a/be/src/rpc/authentication.cc
+++ b/be/src/rpc/authentication.cc
@@ -132,6 +132,16 @@ DEFINE_bool(trusted_domain_use_xff_header, false,
"domain. Only used if '--trusted_domain' is specified. Warning: Only use
this if you "
"trust the incoming connection to have this set correctly.");
+DEFINE_bool(trusted_domain_empty_xff_header_use_origin, false,
+ "If set to true and the 'X-Forwarded-For' HTML header value is empty in
the request, "
+ "then the origin of the the underlying transport is used while attempting
to "
+ "verify if the connection request originated from a trusted domain. Only
used "
+ "if '--trusted_domain' and '--trusted_domain_use_xff_header' flags are
specified. "
+ "Warning: In case the 'X-Forwarded-For' HTML header is empty or not in the
request, "
+ "this flag allows a fallback to the default behavior in trusted domain
check "
+ "(where '--trusted_domain' flag is specified, but
'--trusted_domain_use_xff_header' "
+ "flag is not set).");
+
DEFINE_bool(trusted_domain_strict_localhost, true,
"If set to true and trusted_domain='localhost', this will not use reverse
DNS to "
"determine if something is from localhost. It will only match 127.0.0.1.
This is "
diff --git a/be/src/transport/THttpServer.cpp b/be/src/transport/THttpServer.cpp
index ce76d3e38..3a1f5d60a 100644
--- a/be/src/transport/THttpServer.cpp
+++ b/be/src/transport/THttpServer.cpp
@@ -36,6 +36,7 @@
#include "common/logging.h"
DECLARE_bool(trusted_domain_use_xff_header);
+DECLARE_bool(trusted_domain_empty_xff_header_use_origin);
DECLARE_bool(saml2_ee_test_mode);
DECLARE_string(trusted_auth_header);
@@ -367,6 +368,11 @@ void THttpServer::headersDone() {
string origin =
FLAGS_trusted_domain_use_xff_header ? origin_ :
transport_->getOrigin();
StripWhiteSpace(&origin);
+ if (origin.empty() && FLAGS_trusted_domain_use_xff_header &&
+ FLAGS_trusted_domain_empty_xff_header_use_origin) {
+ origin = transport_->getOrigin();
+ StripWhiteSpace(&origin);
+ }
if (!origin.empty()) {
if (callbacks_.trusted_domain_check_fn(origin, auth_value_)) {
authorized = true;
diff --git a/be/src/util/webserver.cc b/be/src/util/webserver.cc
index f74df5255..1289bde9d 100644
--- a/be/src/util/webserver.cc
+++ b/be/src/util/webserver.cc
@@ -158,6 +158,7 @@ DECLARE_string(ssl_cipher_list);
DECLARE_string(tls_ciphersuites);
DECLARE_string(trusted_domain);
DECLARE_bool(trusted_domain_use_xff_header);
+DECLARE_bool(trusted_domain_empty_xff_header_use_origin);
DECLARE_bool(trusted_domain_strict_localhost);
DECLARE_bool(jwt_token_auth);
DECLARE_bool(jwt_validate_signature);
@@ -743,6 +744,11 @@ sq_callback_result_t
Webserver::BeginRequestCallback(struct sq_connection* conne
xff_origin_string :
GetRemoteAddress(request_info).ToString();
StripWhiteSpace(&origin);
+ if (origin.empty() && FLAGS_trusted_domain_use_xff_header &&
+ FLAGS_trusted_domain_empty_xff_header_use_origin) {
+ origin = GetRemoteAddress(request_info).ToString();
+ StripWhiteSpace(&origin);
+ }
if (!origin.empty()) {
if (TrustedDomainCheck(origin, connection, request_info)) {
total_trusted_domain_check_success_->Increment(1);
diff --git a/fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
b/fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
index 164b2e65b..325f1a148 100644
--- a/fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
+++ b/fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
@@ -494,6 +494,86 @@ public class LdapHS2Test {
hiveserver2TrustedDomainAuthTestBody(false);
}
+ @Test
+ public void testHiveserver2TrustedDomainEmptyXffHeaderUseOrigin() throws
Exception {
+ setUp("--trusted_domain=localhost --trusted_domain_use_xff_header=true " +
+ "--trusted_domain_empty_xff_header_use_origin=true");
+ verifyMetrics(0, 0);
+ verifyTrustedDomainMetrics(0);
+ THttpClient transport = new THttpClient("http://localhost:28000";);
+ Map<String, String> headers = new HashMap<String, String>();
+
+ // Case 1: Authenticate as 'Test1Ldap' without password, send
X-Forwarded-For header
+ headers.put("Authorization", "Basic VGVzdDFMZGFwOg==");
+ headers.put("X-Forwarded-For", "127.0.0.1");
+ transport.setCustomHeaders(headers);
+ transport.open();
+ TCLIService.Iface client = new TCLIService.Client(new
TBinaryProtocol(transport));
+
+ // Open a session which will get username 'Test1Ldap'.
+ TOpenSessionReq openReq = new TOpenSessionReq();
+ TOpenSessionResp openResp = client.OpenSession(openReq);
+ // One successful authentication.
+ verifyMetrics(0, 0);
+ verifyTrustedDomainMetrics(1);
+ // Running a query should succeed.
+ TOperationHandle operationHandle = execAndFetch(
+ client, openResp.getSessionHandle(), "select logged_in_user()",
"Test1Ldap");
+ // Two more successful authentications - for the Exec() and the Fetch().
+ verifyMetrics(0, 0);
+ verifyTrustedDomainMetrics(3);
+
+ // Case 2: Authenticate as 'Test1Ldap' without password, do not send
X-Forwarded-For
+ // header
+ headers.put("Authorization", "Basic VGVzdDFMZGFwOg==");
+ headers.remove("X-Forwarded-For");
+ transport.setCustomHeaders(headers);
+ openResp = client.OpenSession(openReq);
+ verifyMetrics(0, 0);
+ verifyTrustedDomainMetrics(4);
+ operationHandle = execAndFetch(client, openResp.getSessionHandle(),
+ "select logged_in_user()", "Test1Ldap");
+ verifyMetrics(0, 0);
+ // Two more successful authentications - for the Exec() and the Fetch().
+ verifyTrustedDomainMetrics(6);
+
+ // Case 3: Authenticate as 'Test1Ldap' without password, send
X-Forwarded-For header
+ // that does not match trusted_domain
+ headers.put("Authorization", "Basic VGVzdDFMZGFwOg==");
+ headers.put("X-Forwarded-For", "126.0.23.1");
+ transport.setCustomHeaders(headers);
+ try {
+ openResp = client.OpenSession(openReq);
+ fail("Authentication should fail without password.");
+ } catch (Exception e) {
+ assertEquals(e.getMessage(), "HTTP Response code: 401");
+ // One basic auth failure
+ verifyMetrics(0, 1);
+ // And no more successful authentication
+ verifyTrustedDomainMetrics(6);
+ }
+
+ // Case 4: Authenticate as 'Test1Ldap' without password, do not send
X-Forwarded-For
+ // header and the origin does not match trusted_domain
+ setUp("--trusted_domain=any.domain --trusted_domain_use_xff_header=true " +
+ "--trusted_domain_empty_xff_header_use_origin=true");
+ headers.put("Authorization", "Basic VGVzdDFMZGFwOg==");
+ headers.remove("X-Forwarded-For");
+ transport.setCustomHeaders(headers);
+ verifyMetrics(0, 0);
+ verifyTrustedDomainMetrics(0);
+ try {
+ openResp = client.OpenSession(openReq);
+ fail("Authentication should fail without password.");
+ } catch (Exception e) {
+ assertEquals(e.getMessage(), "HTTP Response code: 401");
+ // One basic auth failure
+ verifyMetrics(0, 1);
+ // No successful trusted domain authentications
+ verifyTrustedDomainMetrics(0);
+ }
+ }
+
/**
* Tests if authentication is skipped when connections to the HTTP
hiveserver2
* endpoint have trusted auth header.
diff --git
a/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
b/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
index 62ae0e9e6..4f6e4dddf 100644
--- a/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
+++ b/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
@@ -320,6 +320,37 @@ public class LdapWebserverTest {
webserverTrustedDomainTestBody(false);
}
+ @Test
+ public void testWebserverTrustedDomainEmptyXffHeaderUseOrigin() throws
Exception {
+ setUp("--trusted_domain=localhost --trusted_domain_use_xff_header=true " +
+ "--trusted_domain_empty_xff_header_use_origin=true", "", "", "", "");
+
+ // Case 1: Authenticate as 'Test1Ldap' without password, send
X-Forwarded-For header
+ attemptConnection("Basic VGVzdDFMZGFwOg==", "127.0.0.1", false);
+
+ // Case 2: Authenticate as 'Test1Ldap' without password, do not send
X-Forwarded-For
+ // header
+ attemptConnection("Basic VGVzdDFMZGFwOg==", null, false);
+
+ // Case 3: Authenticate as 'Test1Ldap' without password, send
X-Forwarded-For header
+ // that does not match trusted_domain
+ try {
+ attemptConnection("Basic VGVzdDFMZGFwOg==", "126.0.23.1", false);
+ } catch (IOException e) {
+ assertTrue(e.getMessage().contains("Server returned HTTP response code:
401"));
+ }
+
+ // Case 4: Authenticate as 'Test1Ldap' without password, do not send
X-Forwarded-For
+ // header and the origin does not match trusted_domain
+ setUp("--trusted_domain=any.domain --trusted_domain_use_xff_header=true " +
+ "--trusted_domain_empty_xff_header_use_origin=true", "", "", "",
"");
+ try {
+ attemptConnection("Basic VGVzdDFMZGFwOg==", null, false);
+ } catch (IOException e) {
+ assertTrue(e.getMessage().contains("Server returned HTTP response code:
401"));
+ }
+ }
+
@Test
public void testWebserverTrustedAuthHeader() throws Exception {
setUp("--trusted_auth_header=X-Trusted-Proxy-Auth-Header", "", "", "", "");
