[Improvement] Package sun.security.krb5 is not visible in Java 11 and 17

[kaijchen]

Code of Conduct

• I agree to follow this project's Code of Conduct

Search before asking

• I have searched in the issues and found no similar issues.

What would you like to be improved?

Sun Kerberos binding, i.e. package sun.security.krb5 is blocking us to build Uniffle in Java 11 and Java 17.

# With JDK 11
mvn package -Djava.version=11

# With JDK 17
mvn package -Djava.version=17

How should we improve?

Apache Kerby, as an Apache Directory sub project, is a Java Kerberos binding. It provides a rich, intuitive and interoperable implementation, library, KDC and various facilities that integrates PKI, OTP and token (OAuth2) as desired in modern environments such as cloud, Hadoop and mobile.

We can replace sun.security.krb5 by Apache Kerby.

Are you willing to submit PR?

• Yes I am willing to submit a PR!

[advancedxy]

@zuston would you like to take a look at this?

@kaijchen would you also post some compiling failure logs when with JDK 11 or JDK 17?

[zuston]

@zuston would you like to take a look at this?

Yes. But I don't have much time in this month, perhaps next month.

[advancedxy]

@zuston would you like to take a look at this?

Yes. But I don't have much time in this month, perhaps next month.

No worries. It's not urgent.

[kaijchen]

would you also post some compiling failure logs when with JDK 11 or JDK 17?

https://github.com/kaijchen/incubator-uniffle/actions/runs/4228912219/jobs/7344901292#step:5:2884

Error:  Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.7.0:compile (default-compile) on project rss-common: Compilation failure
Error:  /home/runner/work/incubator-uniffle/incubator-uniffle/common/src/main/java/org/apache/uniffle/common/security/HadoopSecurityContext.java:[59,18] error: package sun.security.krb5 is not visible
Error:    (package sun.security.krb5 is declared in module java.security.jgss, which does not export it to the unnamed module)
Error:  -> [Help 1]

[slfan1989]

@kaijchen Can you assign this to me?

[kaijchen]

@kaijchen Can you assign this to me?

Assigned, thanks for taking this issue.

[slfan1989]

@kaijchen Thank you very much!

[slfan1989]

I read this part of the code carefully, and I summarized the relevant information as follows:

We have 2 ways to solve the issue.

• Remove sun.security.krb5.Config.refresh(); code.
• Add --add-exports java.security.jgss/sun.security.krb5=ALL-UNNAMED in the pom.xml of the project.

Remove sun.security.krb5.Config.refresh(); code.

The code is based on the hadoop trunk branch. The code is different from hadoop-2.8.5, but this does not affect the conclusion.

1. From my personal point of view, this part of the code can be removed, because this part of the code should only work when the location of the krb5.conf configuration file changes in the same JVM.
2. We can find that when using loginUserFromKeytabAndReturnUGI, refreshKrb5Config=true will be added by default. Because the refreshKrb5Config=true parameter is set to true, if the content of krb5.conf changes, it will be automatically reloaded. But if the location of krb5.conf changes during the running of the program, this part of the logic will have no effect.
3. I think we should not modify the location of krb5.conf during the running of the program. If the location of krb5.conf will not change, I think we can remove sun.security.krb5.Config.refresh();

RefreshKrb5Config=true, The code call is as follows:

UserGroupInformation#loginUserFromKeytabAndReturnUGI
    \--> UserGroupInformation#doSubjectLogin
          \--> HadoopConfiguration#new HadoopLoginContext()
                \--> HadoopLoginContext#constructor
                      \-->  LoginContext#constructor#init
                           \--> LoginContext#constructor#init#getAppConfigurationEntry(name);
                                \--> HadoopConfiguration#getAppConfigurationEntry

We use UserGroupInformation.loginUserFromKeytabAndReturnUGI(principal, keytabFile) to verify the identity of the user and ensure that legitimate users can access HDFS. We can find that refreshKrb5Config=true is added when initializing access to Kerberos configuration in the Hadoop code.

UserGroupInformation#loginUserFromKeytabAndReturnUGI

public
  static UserGroupInformation loginUserFromKeytabAndReturnUGI(String user,
                                  String path
                                  ) throws IOException {
    if (!isSecurityEnabled())
      return UserGroupInformation.getCurrentUser();

    LoginParams params = new LoginParams();
    params.put(LoginParam.PRINCIPAL, user);
    params.put(LoginParam.KEYTAB, path);
    return doSubjectLogin(null, params);
  }

UserGroupInformation#doSubjectLogin

private static UserGroupInformation doSubjectLogin(
      Subject subject, LoginParams params) throws IOException {
    ensureInitialized();
    // initial default login.
    if (subject == null && params == null) {
      params = LoginParams.getDefaults();
    }
    HadoopConfiguration loginConf = new HadoopConfiguration(params);
    try {
      // *****
      // We need to focus on this code
      // *****
      HadoopLoginContext login = newLoginContext(
        authenticationMethod.getLoginAppName(), subject, loginConf);
      login.login();
      UserGroupInformation ugi = new UserGroupInformation(login.getSubject());
      // attach login context for relogin unless this was a pre-existing
      // subject.
      if (subject == null) {
        params.put(LoginParam.PRINCIPAL, ugi.getUserName());
        ugi.setLogin(login);
        ugi.setLastLogin(Time.now());
      }
      return ugi;
    } catch (LoginException le) {
      KerberosAuthException kae =
        new KerberosAuthException(FAILURE_TO_LOGIN, le);
      if (params != null) {
        kae.setPrincipal(params.get(LoginParam.PRINCIPAL));
        kae.setKeytabFile(params.get(LoginParam.KEYTAB));
        kae.setTicketCacheFile(params.get(LoginParam.CCACHE));
      }
      throw kae;
    }
  }

HadoopConfiguration#new HadoopLoginContext()

private static HadoopLoginContext
  newLoginContext(String appName, Subject subject,
                  HadoopConfiguration loginConf)
      throws LoginException {
    // Temporarily switch the thread's ContextClassLoader to match this
    // class's classloader, so that we can properly load HadoopLoginModule
    // from the JAAS libraries.
    Thread t = Thread.currentThread();
    ClassLoader oldCCL = t.getContextClassLoader();
    t.setContextClassLoader(HadoopLoginModule.class.getClassLoader());
    try {
      return new HadoopLoginContext(appName, subject, loginConf);
    } finally {
      t.setContextClassLoader(oldCCL);
    }
  }

HadoopLoginContext#constructor

HadoopLoginContext(String appName, Subject subject,
                       HadoopConfiguration conf) throws LoginException {
      super(appName, subject, null, conf);
      this.appName = appName;
      this.conf = conf;
    }

LoginContext#constructor(Configuration config is HadoopConfiguration)

public LoginContext(String name, Subject subject,
                        CallbackHandler callbackHandler,
                        Configuration config) throws LoginException {
        this.config = config;
        if (config != null) {
            creatorAcc = java.security.AccessController.getContext();
        }
        // 
        init(name);
        if (subject != null) {
            this.subject = subject;
            subjectProvided = true;
        }
        if (callbackHandler == null) {
            loadDefaultCallbackHandler();
        } else if (creatorAcc == null) {
            this.callbackHandler = new SecureCallbackHandler
                                (java.security.AccessController.getContext(),
                                callbackHandler);
        } else {
            this.callbackHandler = callbackHandler;
        }
    }

LoginContext#constructor#init#getAppConfigurationEntry

private void init(String name) throws LoginException {

        .....

        // get the LoginModules configured for this application
        // We can view HadoopConfiguration's getAppConfigurationEntry.
        AppConfigurationEntry[] entries = config.getAppConfigurationEntry(name);
        if (entries == null) {

            if (sm != null && creatorAcc == null) {
                sm.checkPermission(new AuthPermission
                                ("createLoginContext." + OTHER));
            }

            entries = config.getAppConfigurationEntry(OTHER);
            if (entries == null) {
                MessageFormat form = new MessageFormat(ResourcesMgr.getString
                        ("No.LoginModules.configured.for.name"));
                Object[] source = {name};
                throw new LoginException(form.format(source));
            }
        }
        moduleStack = new ModuleInfo[entries.length];
        for (int i = 0; i < entries.length; i++) {
            // clone returned array
            moduleStack[i] = new ModuleInfo
                                (new AppConfigurationEntry
                                        (entries[i].getLoginModuleName(),
                                        entries[i].getControlFlag(),
                                        entries[i].getOptions()),
                                null);
        }
        .....
    }

add --add-exports java.security.jgss/sun.security.krb5=ALL-UNNAMED

after JDK11, package sun.security.krb5 is not visible. I refer to hbase and flink. These two projects added --add-exports java.security.jgss/sun.security.krb5=ALL-UNNAMED when compiling maven, which can ensure that the compilation passes.

no need to use apache kerby

Apache Kerby is a good project, which is used in the code of Hadoop MiniKDC, but only in test classes. When Hadoop performs Kerberos authentication, it is still implemented based on Java JAAS. We rely on hadoop for kerberos authentication, there should be no need to use Apache Kerby. Hbase and Flink also have the need to verify Kerberos, but without using Apache Kerby, I think we can also directly rely on hadoop without using Apache Kerby.

[jerqi]

cc @zuston

[slfan1989]

cc @zuston

@jerqi @zuston @kaijchen I still need to add some information. after the information is completed, please help to review it.

[slfan1989]

@jerqi @zuston @kaijchen Can you please check my comment above? Thank you so much!

I think adding --add-exports java.security.jgss/sun.security.krb5=ALL-UNNAMED in the pom file is a better option. But I still want to ask sun.security.krb5.Config.refresh(); the purpose of this.

[zuston]

Wow, really thanks for your great work. @slfan1989

no need to use apache kerby

+1.

But I still want to ask sun.security.krb5.Config.refresh(); the purpose of this.

This part of code is introduced in #184 . If I remember correctly, this is just for unit test to simulate setting the profile w/o, because all tests run in one JVM.

If test cases could pass, I prefer removing this.

[kaijchen]

But I still want to ask sun.security.krb5.Config.refresh(); the purpose of this.

Thanks @slfan1989 for the investigation, +1 for this proposal. I'll change the title of this issue.