Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple keytab kerberos issue #674

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

zeun0725
Copy link

Problem

If multiple key tabs are distributed on the connect server, authentication errors may occur.

      UserGroupInformation.loginUserFromKeytab(principal, connectorConfig.connectHdfsKeytab()); // problem point
      final UserGroupInformation ugi = UserGroupInformation.getLoginUser();  // problem point
      log.info("Login as: " + ugi.getUserName());

      isRunning = true;
      ticketRenewThread = new Thread(() -> renewKerberosTicket(ugi));

example) A connector uses the test user keytab (write path: /hdfs/user/test), and B connector uses the test2 user keytab (write path: /hdfs/user/test2).
If both connectors are restarted due to certain circumstances, different key tab information can be imported with thread-based behavior.
If the B connector calls loginUserFromKeytab('test2@EXAMPLE.COM' , '/../test2.keytab') when the A connector calls getLoginUser(), A login as test2 user. So you can face the error as below.
Caused by: org.apache.ranger.authorization.hadoop.exceptions.RangerAccessControlException: Permission denied: user=test2, access=EXECUTE, inode="/hdfs/user/test"

Solution

final UserGroupInformation ugi = UserGroupInformation.loginUserFromKeytabAndReturnUGI(principal, connectorConfig.connectHdfsKeytab());

When receiving an ugi instance, it was changed to authenticate with a keytab and receive it.
Change to use loginUserFromKeytabAndReturnUGI() func.

Does this solution apply anywhere else?
  • yes
  • [ X ] no
If yes, where?

Test Strategy

Testing done:
  • [ X ] Unit tests
  • Integration tests
  • System tests
  • [ X ] Manual tests

Release Plan

@zeun0725 zeun0725 requested a review from a team as a code owner November 25, 2023 05:58
Copy link

cla-assistant bot commented Nov 25, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


박지은 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@lala7573
Copy link
Contributor

lala7573 commented Dec 20, 2023

@kkonstantine @snehashisp
Could you please review this PR? Related to this issue #325

@zeun0725
Please sign the cla and fix checkstyle issue to make this valid pr.

@@ -274,7 +274,7 @@ private void configureKerberosAuthentication(Configuration hadoopConfiguration)
hostname
);
UserGroupInformation.loginUserFromKeytab(principal, connectorConfig.connectHdfsKeytab());
final UserGroupInformation ugi = UserGroupInformation.getLoginUser();
final UserGroupInformation ugi = UserGroupInformation.loginUserFromKeytabAndReturnUGI(principal, connectorConfig.connectHdfsKeytab());
Copy link
Contributor

Choose a reason for hiding this comment

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

If the line exceeds 100 characters, mvn checkstyle error occurs, so it would be a good idea to change it like this.

Suggested change
final UserGroupInformation ugi = UserGroupInformation.loginUserFromKeytabAndReturnUGI(principal, connectorConfig.connectHdfsKeytab());
final UserGroupInformation ugi = UserGroupInformation.loginUserFromKeytabAndReturnUGI(
principal,
connectorConfig.connectHdfsKeytab()
);

@lala7573
Copy link
Contributor

@zeun0725
I build this and tested it.
It seems that there are other problems besides this issue. This changes only affect on relogin.
When I try to create a connector with different user, lease permission error still occurs.

@zeun0725 zeun0725 requested a review from a team as a code owner March 5, 2024 01:53
@zeun0725
Copy link
Author

zeun0725 commented Mar 5, 2024

@lala7573
Can you share the test code or config value?

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

Successfully merging this pull request may close these issues.

2 participants