You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2023/03/16 21:40:46 UTC
[impala] branch master updated: IMPALA-11942: Restrict trusted_domain=localhost to 127.0.0.1 by default
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell 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 c2287823e IMPALA-11942: Restrict trusted_domain=localhost to 127.0.0.1 by default
c2287823e is described below
commit c2287823e095e3affd1a99982a57c5df623a95b6
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Thu Mar 9 18:14:34 2023 -0800
IMPALA-11942: Restrict trusted_domain=localhost to 127.0.0.1 by default
The trusted_domain startup parameter uses reverse DNS to determine
if a connection is coming from a trusted domain. For
trusted_domain=localhost, reverse DNS can be unreliable, because
some non-local IP ranges map to localhost. This can also cause
issues with our test cases. In some test environments (Ubuntu 20.04
on AWS), IP addresses like 127.23.0.1 resolve to localhost.
This adds a new startup option trusted_domain_strict_localhost,
which defaults to true. When true, Impala does not do a reverse
DNS request to determine if an IP address is localhost. Instead,
it compares to 127.0.0.1 directly. When false, localhost uses
the same reverse DNS logic as before.
Testing:
- Modified the existing trusted_domain tests to test with
trusted_domain_strict_localhost=true and false.
- Ubuntu 20.04 tests pass on an AWS machine.
Change-Id: I5915cdd812d461366a421a739c18afecef44fb5b
Reviewed-on: http://gerrit.cloudera.org:8080/19607
Reviewed-by: Wenzhe Zhou <wz...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/rpc/authentication-util.cc | 7 ++++-
be/src/rpc/authentication-util.h | 12 +++++---
be/src/rpc/authentication.cc | 11 +++++++-
be/src/util/webserver.cc | 6 +++-
.../apache/impala/customcluster/LdapHS2Test.java | 32 ++++++++++++++++++----
.../impala/customcluster/LdapWebserverTest.java | 32 ++++++++++++++++++----
6 files changed, 82 insertions(+), 18 deletions(-)
diff --git a/be/src/rpc/authentication-util.cc b/be/src/rpc/authentication-util.cc
index 36c67a6fa..6a9ec41c7 100644
--- a/be/src/rpc/authentication-util.cc
+++ b/be/src/rpc/authentication-util.cc
@@ -182,7 +182,8 @@ string GetDeleteCookie() {
return Substitute("$0=;HttpOnly;Max-Age=0", COOKIE_NAME);
}
-bool IsTrustedDomain(const std::string& origin, const std::string& trusted_domain) {
+bool IsTrustedDomain(const std::string& origin, const std::string& trusted_domain,
+ bool strict_localhost) {
if (trusted_domain.empty()) return false;
vector<string> split = Split(origin, delimiter::Limit(",", 1));
if (split.empty()) return false;
@@ -196,6 +197,10 @@ bool IsTrustedDomain(const std::string& origin, const std::string& trusted_domai
vector<string> host_n_port = Split(split[0], delimiter::Limit(":", 1));
host_name = host_n_port[0];
} else {
+ // If using strict localhost checks, only allow localhost to match 127.0.0.1
+ if (trusted_domain == "localhost" && strict_localhost) {
+ return sock_addr.host() == "127.0.0.1";
+ }
s = sock_addr.LookupHostname(&host_name);
if (!s.ok()) {
LOG(ERROR) << "DNS reverse-lookup failed for " << split[0]
diff --git a/be/src/rpc/authentication-util.h b/be/src/rpc/authentication-util.h
index c901beea8..efd3ae5da 100644
--- a/be/src/rpc/authentication-util.h
+++ b/be/src/rpc/authentication-util.h
@@ -42,10 +42,14 @@ std::string GetDeleteCookie();
// Takes a comma separated list of ip address/hostname with or without a port, picks the
// left most on the list (this assumes the list follows the http standard for
// 'X-Forwarded-For' header where the left-most IP address is the IP address of the
-// originating client), does a reverse DNS lookup on it if its a valid ip address and
-// finally checks if it originates from the 'trusted_domain'. Returns true if the origin
-// is from the trusted domain.
-bool IsTrustedDomain(const std::string& origin, const std::string& trusted_domain);
+// originating client), then checks whether this corresponds to the 'trusted_domain'.
+// If the 'trusted_domain' is 'localhost' and 'strict_localhost' is true, this
+// returns true if the origin is the 127.0.0.1 IP address. If 'strict_localhost' is
+// false or 'trusted_domain' is not 'localhost', then this does a reverse DNS lookup
+// if it's a valid ip address and checks if it originates from the 'trusted_domain'.
+// Returns true if the origin is from the trusted domain.
+bool IsTrustedDomain(const std::string& origin, const std::string& trusted_domain,
+ bool strict_localhost);
// Takes in the base64 encoded token and returns the username and password via the input
// arguments. Returns an OK status if the token is a valid base64 encoded string of the
diff --git a/be/src/rpc/authentication.cc b/be/src/rpc/authentication.cc
index 4f3b4ad94..16bf5acf7 100644
--- a/be/src/rpc/authentication.cc
+++ b/be/src/rpc/authentication.cc
@@ -132,6 +132,12 @@ 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_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 "
+ "important for security, because reverse DNS can resolve other non-local addresses "
+ "to localhost.");
+
// This flag must be used with caution to avoid security risks.
DEFINE_string(trusted_auth_header, "",
"If set as non empty string, Impala will look for this header in the HTTP headers. "
@@ -629,7 +635,10 @@ bool GetUsernameFromBasicAuthHeader(
bool TrustedDomainCheck(ThriftServer::ConnectionContext* connection_context,
const AuthenticationHash& hash, const std::string& origin, string auth_header) {
- if (!IsTrustedDomain(origin, FLAGS_trusted_domain)) return false;
+ if (!IsTrustedDomain(origin, FLAGS_trusted_domain,
+ FLAGS_trusted_domain_strict_localhost)) {
+ return false;
+ }
if (!GetUsernameFromBasicAuthHeader(connection_context, auth_header)) return false;
// Create a cookie to return.
diff --git a/be/src/util/webserver.cc b/be/src/util/webserver.cc
index 2eb9a8242..5051d17f5 100644
--- a/be/src/util/webserver.cc
+++ b/be/src/util/webserver.cc
@@ -156,6 +156,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_strict_localhost);
DECLARE_bool(jwt_token_auth);
DECLARE_bool(jwt_validate_signature);
DECLARE_string(jwt_custom_claim_username);
@@ -989,7 +990,10 @@ bool Webserver::GetUsernameFromAuthHeader(struct sq_connection* connection,
bool Webserver::TrustedDomainCheck(const string& origin, struct sq_connection* connection,
struct sq_request_info* request_info) {
- if (!IsTrustedDomain(origin, FLAGS_trusted_domain)) return false;
+ if (!IsTrustedDomain(origin, FLAGS_trusted_domain,
+ FLAGS_trusted_domain_strict_localhost)) {
+ return false;
+ }
string err_msg;
if (!GetUsernameFromAuthHeader(connection, request_info, err_msg)) {
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 24c6d3986..fc031d4b9 100644
--- a/fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
+++ b/fe/src/test/java/org/apache/impala/customcluster/LdapHS2Test.java
@@ -346,11 +346,16 @@ public class LdapHS2Test {
/**
* Tests if authentication is skipped when connections to the HTTP hiveserver2
- * endpoint originate from a trusted domain.
+ * endpoint originate from a trusted domain. This is a shared test function
+ * that is used for both the trusted_domain_strict_localhost=true and false
+ * cases.
*/
- @Test
- public void testHiveserver2TrustedDomainAuth() throws Exception {
- setUp("--trusted_domain=localhost --trusted_domain_use_xff_header=true");
+ private void hiveserver2TrustedDomainAuthTestBody(boolean strictLocalhost)
+ throws Exception {
+ String strictLocalhostArgs = "--trusted_domain_strict_localhost=" +
+ String.valueOf(strictLocalhost);
+ setUp("--trusted_domain=localhost --trusted_domain_use_xff_header=true "
+ + strictLocalhostArgs);
verifyMetrics(0, 0);
THttpClient transport = new THttpClient("http://localhost:28000");
Map<String, String> headers = new HashMap<String, String>();
@@ -405,7 +410,10 @@ public class LdapHS2Test {
// Case 3: Case 1: Authenticate as 'Test1Ldap' with the right password
// '12345' but with a non trusted address in X-Forwarded-For header
headers.put("Authorization", "Basic VGVzdDFMZGFwOjEyMzQ1");
- headers.put("X-Forwarded-For", "127.23.0.1");
+ // Sometimes RDNS resolves 127.* addresses as localhost, so this uses a 126.*
+ // address for non-strict localhost to avoid RDNS issues.
+ String nontrustedIp = strictLocalhost ? "127.0.23.1" : "126.0.23.1";
+ headers.put("X-Forwarded-For", nontrustedIp);
transport.setCustomHeaders(headers);
openResp = client.OpenSession(openReq);
verifyMetrics(1, 0);
@@ -430,7 +438,7 @@ public class LdapHS2Test {
// Case 5: Case 1: Authenticate as 'Test1Ldap' with the no password
// and a non trusted address in X-Forwarded-For header
headers.put("Authorization", "Basic VGVzdDFMZGFwOg==");
- headers.put("X-Forwarded-For", "127.23.0.1");
+ headers.put("X-Forwarded-For", nontrustedIp);
transport.setCustomHeaders(headers);
try {
openResp = client.OpenSession(openReq);
@@ -452,6 +460,18 @@ public class LdapHS2Test {
verifyTrustedDomainMetrics(9);
}
+ @Test
+ public void testHiveserver2TrustedDomainAuthStrict() throws Exception {
+ // Test variant with trusted_domain_strict_localhost=true
+ hiveserver2TrustedDomainAuthTestBody(true);
+ }
+
+ @Test
+ public void testHiveserver2TrustedDomainAuthNonstrict() throws Exception {
+ // Test variant with trusted_domain_strict_localhost=false
+ hiveserver2TrustedDomainAuthTestBody(false);
+ }
+
/**
* 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 fb354291f..c17b3f73a 100644
--- a/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
+++ b/fe/src/test/java/org/apache/impala/customcluster/LdapWebserverTest.java
@@ -251,9 +251,16 @@ public class LdapWebserverTest {
}
}
- @Test
- public void testWebserverTrustedDomain() throws Exception {
- setUp("--trusted_domain=localhost --trusted_domain_use_xff_header=true", "", "", "");
+ /**
+ * Tests if authentication is skipped when connections to the webserver originate
+ * from a trusted domain. This is a shared test function that is used for both
+ * trusted_domain_strict_localhost=true and false cases.
+ */
+ private void webserverTrustedDomainTestBody(boolean strictLocalhost) throws Exception {
+ String strictLocalhostArgs = "--trusted_domain_strict_localhost=" +
+ String.valueOf(strictLocalhost);
+ setUp("--trusted_domain=localhost --trusted_domain_use_xff_header=true " +
+ strictLocalhostArgs, "", "", "");
// Case 1: Authenticate as 'Test1Ldap' with the right password '12345'
attemptConnection("Basic VGVzdDFMZGFwOjEyMzQ1", "127.0.0.1", false);
@@ -265,7 +272,10 @@ public class LdapWebserverTest {
// Case 3: Authenticate as 'Test1Ldap' with the right password
// '12345' but with a non trusted address in X-Forwarded-For header
- attemptConnection("Basic VGVzdDFMZGFwOjEyMzQ1", "127.0.23.1", false);
+ // Sometimes RDNS resolves 127.* addresses as localhost, so this uses a 126.*
+ // address for non-strict localhost to avoid RDNS issues.
+ String nontrustedIp = strictLocalhost ? "127.0.23.1" : "126.0.23.1";
+ attemptConnection("Basic VGVzdDFMZGFwOjEyMzQ1", nontrustedIp, false);
verifyTrustedDomainMetrics(Range.closed(2L, 2L));
// Case 4: No auth header, does not work
@@ -279,7 +289,7 @@ public class LdapWebserverTest {
// Case 5: Authenticate as 'Test1Ldap' with the no password
// and a non trusted address in X-Forwarded-For header
try {
- attemptConnection("Basic VGVzdDFMZGFwOg==", "127.0.23.1", false);
+ attemptConnection("Basic VGVzdDFMZGFwOg==", nontrustedIp, false);
} catch (IOException e) {
assertTrue(e.getMessage().contains("Server returned HTTP response code: 401"));
}
@@ -293,6 +303,18 @@ public class LdapWebserverTest {
verifyTrustedDomainMetrics(Range.closed(successMetricBefore, successMetricBefore));
}
+ @Test
+ public void testWebserverTrustedDomainStrict() throws Exception {
+ // Test variant with trusted_domain_strict_localhost=true
+ webserverTrustedDomainTestBody(true);
+ }
+
+ @Test
+ public void testWebserverTrustedDomainNonstrict() throws Exception {
+ // Test variant with trusted_domain_strict_localhost=false
+ webserverTrustedDomainTestBody(false);
+ }
+
@Test
public void testWebserverTrustedAuthHeader() throws Exception {
setUp("--trusted_auth_header=X-Trusted-Proxy-Auth-Header", "", "", "");