This is an automated email from the ASF dual-hosted git repository.
arawat 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 139c74bcf IMPALA-11298: Allow proxy users to share hs2 session from
different hosts or realms
139c74bcf is described below
commit 139c74bcf34fe29c4e88cd7bd1a98b78eacce89f
Author: Abhishek Rawat <[email protected]>
AuthorDate: Wed Sep 25 09:32:05 2024 -0700
IMPALA-11298: Allow proxy users to share hs2 session from different hosts
or realms
Some proxy clients like Hue could reuse hs2 session across multiple
hosts. This patch relaxes the check which enforces kerberos username of
connected user to match session username. This is because the username
could include the hostname and realm such as 'user/instance@REALM' or
'user@REALM'. It's okay to allow the same proxy 'user' to share
the hs2 session irrespective of its 'instance' or 'realm'.
ImpalaServer::AuthorizeProxyUser() uses kerberos short name for
delegation. In this patch we compare the short user name of connected
user with session user when session user is a proxy user i.e., session
has a 'do_as_user'.
The side effects are that 'Connected User:' in query profile and
FunctionContext::user() uses the long username from the session state
which could be different from connected user.
Testing:
- Running exhaustive tests.
Change-Id: Ib9c539cda8c760c8667a2e8cbb6d5c7902888de9
Reviewed-on: http://gerrit.cloudera.org:8080/21925
Reviewed-by: Impala Public Jenkins <[email protected]>
Tested-by: Impala Public Jenkins <[email protected]>
---
be/src/rpc/authentication.cc | 6 ++++--
be/src/rpc/thrift-server.h | 3 +++
be/src/service/impala-hs2-server.cc | 5 +++++
be/src/service/impala-server.cc | 41 +++++++++++++++++++++++++++++++++++++
be/src/service/impala-server.h | 29 +++++---------------------
5 files changed, 58 insertions(+), 26 deletions(-)
diff --git a/be/src/rpc/authentication.cc b/be/src/rpc/authentication.cc
index c27187d77..828952d84 100644
--- a/be/src/rpc/authentication.cc
+++ b/be/src/rpc/authentication.cc
@@ -782,11 +782,10 @@ bool NegotiateAuth(ThriftServer::ConnectionContext*
connection_context,
<<
TNetworkAddressToString(connection_context->network_address)
<< ": " << spnego_status.ToString();
} else {
+ string short_user = GetShortUsernameFromKerberosPrincipal(username);
if (FLAGS_enable_ldap_auth &&
FLAGS_enable_group_filter_check_for_authenticated_kerberos_user) {
- string short_user = GetShortUsernameFromKerberosPrincipal(username);
-
LOG(INFO) << "Checking LDAP group filters for "
<< "username \"" << short_user << "\" "
<< "parsed from user principal \""
@@ -807,6 +806,7 @@ bool NegotiateAuth(ThriftServer::ConnectionContext*
connection_context,
connection_context->username = username;
// Save the username as Kerberos user principal in the connection
context.
connection_context->kerberos_user_principal = username;
+ connection_context->kerberos_user_short = short_user;
// Create a cookie to return.
connection_context->return_headers.push_back(
Substitute("Set-Cookie: $0", GenerateCookie(username, hash)));
@@ -1405,6 +1405,8 @@ void SecureAuthProvider::SetupConnectionContext(
// Save the username as Kerberos user principal in the connection
context
// if the actual auth mechanism is Kerberos.
connection_ptr->kerberos_user_principal = connection_ptr->username;
+ connection_ptr->kerberos_user_short =
+ GetShortUsernameFromKerberosPrincipal(connection_ptr->username);
}
break;
}
diff --git a/be/src/rpc/thrift-server.h b/be/src/rpc/thrift-server.h
index ddb1917de..58b792a3c 100644
--- a/be/src/rpc/thrift-server.h
+++ b/be/src/rpc/thrift-server.h
@@ -120,6 +120,9 @@ class ThriftServer {
// Used in case of Kerberos authentication only to store the authenticated
Kerberos
// user principal
std::string kerberos_user_principal;
+ // Used in case of Kerberos authentication only to store the authenticated
Kerberos
+ // user principal's short name
+ std::string kerberos_user_short;
};
/// Interface class for receiving connection creation / termination events.
diff --git a/be/src/service/impala-hs2-server.cc
b/be/src/service/impala-hs2-server.cc
index 8d3dab78e..1e6cf407a 100644
--- a/be/src/service/impala-hs2-server.cc
+++ b/be/src/service/impala-hs2-server.cc
@@ -368,6 +368,11 @@ void ImpalaServer::OpenSession(TOpenSessionResp&
return_val,
} else {
state->connected_user = FLAGS_anonymous_user_name;
}
+ // Set the 'connected_user_short' member in the SessionState. This is only
used if
+ // the 'connected_user' is a kerberos principal.
+ if (!connection_context->kerberos_user_short.empty()) {
+ state->connected_user_short = connection_context->kerberos_user_short;
+ }
// Process the supplied configuration map.
state->database = "default";
diff --git a/be/src/service/impala-server.cc b/be/src/service/impala-server.cc
index 073610ba6..214c98276 100644
--- a/be/src/service/impala-server.cc
+++ b/be/src/service/impala-server.cc
@@ -3475,4 +3475,45 @@ TUniqueId ImpalaServer::RandomUniqueID() {
return conn_id;
}
+
+Status ImpalaServer::ScopedSessionState::WithSession(const TUniqueId&
session_id,
+ const SecretArg& secret, std::shared_ptr<SessionState>* session) {
+ DCHECK(session_.get() == NULL);
+ RETURN_IF_ERROR(impala_->GetSessionState(
+ session_id, secret, &session_, /* mark_active= */ true));
+ if (session != NULL) (*session) = session_;
+
+ // We won't have a connection context in the case of ChildQuery, which calls
into
+ // hiveserver2 functions directly without going through the Thrift stack.
+ if (ThriftServer::HasThreadConnectionContext()) {
+ // Check that the session user matches the user authenticated on the
connection.
+ const ThriftServer::Username& connection_username =
+ ThriftServer::GetThreadConnectionContext()->username;
+ const string& kerberos_user_principal =
+ ThriftServer::GetThreadConnectionContext()->kerberos_user_principal;
+ // Compare only the short user name if the connected user is a proxy and
using
+ // kerberos AuthN.
+ if (!session_->do_as_user.empty() && !kerberos_user_principal.empty()) {
+ // This is the connected/authenticated user
+ const string connection_user_short =
+ ThriftServer::GetThreadConnectionContext()->kerberos_user_short;
+ // This is the user which created the original session
+ const string session_user_short = session_->connected_user_short;
+ if (connection_user_short != session_user_short) {
+ return Status::Expected(TErrorCode::UNAUTHORIZED_SESSION_USER,
+ connection_user_short, session_user_short);
+ }
+ } else if (!connection_username.empty()
+ && session_->connected_user != connection_username) {
+ return Status::Expected(TErrorCode::UNAUTHORIZED_SESSION_USER,
+ connection_username, session_->connected_user);
+ }
+
+ // Try adding the session id to the connection's set of sessions in case
this is
+ // the first time this session has been used on this connection.
+ impala_->AddSessionToConnection(session_id, session_.get());
+ }
+ return Status::OK();
+}
+
} // namespace impala
diff --git a/be/src/service/impala-server.h b/be/src/service/impala-server.h
index d382491de..38d2d2e43 100644
--- a/be/src/service/impala-server.h
+++ b/be/src/service/impala-server.h
@@ -613,6 +613,10 @@ class ImpalaServer : public ImpalaServiceIf,
/// Connected user for this session, i.e. the user which originated this
session.
std::string connected_user;
+ /// Short username for connected user. Only applies when connected_user is
a kerberos
+ /// principal. Only applicable for hs2 sessions.
+ std::string connected_user_short;
+
/// The user to delegate to. Empty for no delegation.
std::string do_as_user;
@@ -1378,30 +1382,7 @@ class ImpalaServer : public ImpalaServiceIf,
/// 'secret' must be provided and is validated against the stored secret
for the
/// session. Must only be called once per ScopedSessionState.
Status WithSession(const TUniqueId& session_id, const SecretArg& secret,
- std::shared_ptr<SessionState>* session = NULL) WARN_UNUSED_RESULT {
- DCHECK(session_.get() == NULL);
- RETURN_IF_ERROR(impala_->GetSessionState(
- session_id, secret, &session_, /* mark_active= */ true));
- if (session != NULL) (*session) = session_;
-
- // We won't have a connection context in the case of ChildQuery, which
calls into
- // hiveserver2 functions directly without going through the Thrift stack.
- if (ThriftServer::HasThreadConnectionContext()) {
- // Check that the session user matches the user authenticated on the
connection.
- const ThriftServer::Username& connection_username =
- ThriftServer::GetThreadConnectionContext()->username;
- if (!connection_username.empty()
- && session_->connected_user != connection_username) {
- return Status::Expected(TErrorCode::UNAUTHORIZED_SESSION_USER,
- connection_username, session_->connected_user);
- }
-
- // Try adding the session id to the connection's set of sessions in
case this is
- // the first time this session has been used on this connection.
- impala_->AddSessionToConnection(session_id, session_.get());
- }
- return Status::OK();
- }
+ std::shared_ptr<SessionState>* session = NULL) WARN_UNUSED_RESULT;
/// Same as WithSession(), except:
/// * It should only be called from beeswax with a 'connection_id'
obtained from
