[GitHub] flink issue #5896: [FLINK-8286][Security] Fix kerberos security configuratio...

[aljoscha <gi...@git.apache.org>]

Github user aljoscha commented on the issue:

    https://github.com/apache/flink/pull/5896
  
    @suez1224 I'll now merge the actual fix, but I'm not 100 % sure the refactoring is correct.
    
    After the fix, we have roughly this path through the code:
    ```
    if (keytabPath != null && remoteKeytabPrincipal != null) {
    	configuration.setString(SecurityOptions.KERBEROS_LOGIN_KEYTAB, keytabPath);
    	configuration.setString(SecurityOptions.KERBEROS_LOGIN_PRINCIPAL, remoteKeytabPrincipal);
    }
    
    SecurityConfiguration sc = new SecurityConfiguration(configuration);
    
    SecurityUtils.install(sc);
    
    SecurityUtils.getInstalledContext().runSecured(new Callable<Void>() {
    	@Override
    	public Void call() throws Exception {
    		TaskManagerRunner.runTaskManager(configuration, new ResourceID(containerId));
    		return null;
    	}
    });
    ```
    
    after the fix, that becomes
    
    ```
    // in main()
    SecurityUtils.getInstalledContext().runSecured(
       YarnTaskExecutorRunnerFactory.create(System.getenv()));
    
    // in YarnTaskExecutorRunnerFactory.create()
    if (keytabPath != null && remoteKeytabPrincipal != null) {
    	configuration.setString(SecurityOptions.KERBEROS_LOGIN_KEYTAB, keytabPath);
    	configuration.setString(SecurityOptions.KERBEROS_LOGIN_PRINCIPAL, remoteKeytabPrincipal);
    }
    
    SecurityConfiguration sc = new SecurityConfiguration(configuration);
    
    SecurityUtils.install(sc);
    
    return new Runner()
    ```
    
    Meaning, that if someone messes with how things are called it can happen that `SecurityUtils.getInstalledContext()` is called before `SecurityUtils.install(sc)` is called in `YarnTaskExecutorRunnerFactory.create`. I think this can potentially lead to problems and the old path is much clearer. What do you think?



---