[TEST] Add permission for kerberos test

[bizybot]

The test SimpleKdcLdapServerTests#testPrincipalCreationAndSearchOnLdap
fails intermittently and I could not reproduce this locally.

There were two exceptions from the console logs of which one might be
the reason for the failure. When simple Kdc LDAP server starts,
internally it starts Kdc server and LDAP server. Kdc server then tries
to connect to configured LDAP backend and during this process it
waits for the connection to succeed, meanwhile checking for deadlocks
in between. During this deadlock check, it needs permission to
accessClassInPackage.sun.reflect, the fix here is to add the
required permission so that the check does not throw an exception.

I think once we fix this issue, we may have something to look forward
if there is indeed a deadlock or its just waiting for the process
to complete on slow test runs.
Added a null check in after test method.

See #32739

[elasticmachine]

Pinging @elastic/es-security

[jasontedor]

I left a question.

[jasontedor]

This adds the permission to production too, yet this is only for a test. I understand that Mina is a test-only dependency. Is there anything that we can do to avoid this?

[bizybot]

Yes, you are right the change is being done only for the test but adds the permission to prod even when not required. The change that has been done here actually was required as the underlying library Apache mina needs those permissions when checking for deadlock. I could not find a way to bypass that check as it tries to see if there is any deadlock after the certain timeout, the timeout is hardcoded to 100 ms and is not configurable.

What do you think of adding support for say test-plugin-security.policy in addition to plugin-security.policy capturing the test only permissions? We will need to modify BootstrapForTesting to scan for these additional policy files. But then the test environment settings differ from those in a production environment and we would not want that. Thanks for your inputs.

[jaymode]

+1 to not modifying production permissions.

The failure is in a test for a test class. What about moving these classes to their own project where the permission can be added and then security has a test dependency on this project? This limits the scope of the permission and security is tested without additional permissions.

[bizybot]

Thanks, @jaymode I think we can do that and have permissions in a separate project. I will try this.

[jasontedor]

@bizybot Can you link to the offending Apache Mina code?

[jasontedor]

Note that we have evil-tests that disable the security manager in tests, that could be an option here too. I would like to see the Apache Mina code first, to see if we can fix this upstream or through an outrageous hack.

[bizybot]

Hi @jasontedor
LdapNetworkConnection writes a request and then waits for completion. The wait is 100ms with retry till configured timeout (30s). For every writeFuture.awaitUninterruptibly it checks for dead lock DefaultIoFuture#checkDeadLock() which is code that requires permissions.

[jasontedor]

Thanks so much for that, this helps a lot. The critical loop is this:

        for (StackTraceElement s : stackTrace) {
            try {
                Class<?> cls = DefaultIoFuture.class.getClassLoader().loadClass(s.getClassName());
                
                if (IoProcessor.class.isAssignableFrom(cls)) {
                    throw new IllegalStateException("DEAD LOCK: " + IoFuture.class.getSimpleName()
                            + ".await() was invoked from an I/O processor thread.  " + "Please use "
                            + IoFutureListener.class.getSimpleName()
                            + " or configure a proper thread model alternatively.");
                }
            } catch (ClassNotFoundException cnfe) {
                // Ignore
            }
        }

They already account for CNFE here. They should account for SME too, if they can not load the class they should assume that it is not an IoProcessor implementation and skip it like they skip CNFE. Can we contribute this upstream please? Then we need a mina release, then we need a ldap-directory-api release incorporating that. Can you own this @bizybot?

In the meantime, let us make this an evil test since it will take some time for the above to take place.

[bizybot]

Thanks, Jason, I will take it up and will also move the tests to evil-tests.

[jaymode]

@bizybot do you plan to open a new PR? if so can we close this one?

[bizybot]

Thanks, Jay, will raise a new PR for the changes to move tests to evil-tests. Closing this PR.