[jboss-jira] [JBoss JIRA] (SECURITY-664) jndi lookups for a security domain should be cached

[Stefan Guilhen (JIRA)]

    [ https://issues.jboss.org/browse/SECURITY-664?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12932373#comment-12932373 ] 

Stefan Guilhen commented on SECURITY-664:
-----------------------------------------

I have analysed the case and I have to say I don't agree with the conclusions that point to caches in DefaultSecurityManagement and JBossSecurityContext as solutions.

1- DefaultSecurityManagement has nothing to do with JNDI. It is just a default implementation that is not supposed to be used by JBoss AS. Yes, it lacks a cache and it does create instances all the time but it doesn't trigger JNDI lookups. A cache would indeed prevent unnecessary creation of multiple managers but it wouldn't affect JNDI at all.

2- JNDIBasedSecurityManagement is the implementation used by JBoss AS and AFAIK there is a single instance of this class that is injected in all services that need access to the SecurityManagement (see bootstrap-jboss-beans.xml where this bean is configured). Now, if we examine this implementation we will see that it does have local caches for all managers, which means that a JNDI lookup should be performed only once for a given security domain.

3- JBossSecurityContext is only initialized with the DefaultSecurityManagement, but both the EJB PreSecurityInterceptor and the Web SecurityContextEstablishmentValve inject the JNDIBasedSecurityManagement instance in the constructed JBossSecurityContext. Also note that the security context lives for the duration of the request and adding a cache there won't do anything as it delegates all get*Manager() calls to the SecurityManagement implementation. As explained above the JNDI based implementation already provides a cache.

4- As noted in the case, SecurityUtil has some lookup methods and JaasSecurityManagerBase uses it to retrieve the AuthorizationManager. However, the manager is already provided to JaasSecurityManagerBase and the util is not called. Plus, none of the other util methods are used by the AS code so it can't be responsible for the auditMgr and mappingMgr lookups.

Having said that, the 4 lookups described in the case are consistent with the authorization check performed by the EJBAuthorizationHelper:
a- it calls context.getAuthorizationManager(securityDomain) to retrieve the AuthorizationManager for the domain -  a JNDI lookup (domain/authorizationMgr) should be seen the first time this is done.
b- it calls authzManager.getSubjectRoles(). This method attempts to get a RoleMappingManager from the context to map roles - a JNDI lookup (domain/mappingMgr) should be seen the first time this is done.
c- it sets the ProviderRegistration instance in the resource and it retrieves the registration using a JNDI lookup. This has nothing to do with the context and will always be performed. I need to check with Anil if we can cache this in the helper to prevent more lookups.
d- after the user is authorized the helper calls the auditAuthorization() method, which in turn attempts to get an AuditManager from the context - a JNDI lookup (domain/auditMgr) should be seen the first time this is done.

Out of the 4 JNDI lookups, 3 should happen only once (or once per node in a clustered env). If that is not happening we need to further investigate what could be causing a cache miss. If there is more than one JNDIBasedSecurityManagement instance (there shouldn't be, the bean is created once and injected everywhere) than something is really wrong. Also, I've noticed the caches are transient and would be cleared if the JNDIBasedSecurityManagement is serialized - again, a situation that should never happen.
                
> jndi lookups for a security domain should be cached
> ---------------------------------------------------
>
>                 Key: SECURITY-664
>                 URL: https://issues.jboss.org/browse/SECURITY-664
>             Project: PicketBox 
>          Issue Type: Feature Request
>      Security Level: Public(Everyone can see) 
>          Components: JBossSX
>    Affects Versions: JBossSecurity_2.0.4.SP8
>            Reporter: Tom Fonteyne
>            Assignee: Anil Saldhana
>
> In certain scenarios, there are many JNDI lookups being done, which are taking up to 10-20% of performance.
> org/​ jboss/​ security/​ plugins/​ DefaultSecurityManagement.java
> Should instead of creating new instances all the time be changed to cache per security domain.
> Quote from the customer:
> ------------------------
> The getter methods in the default security management have a factory method character. The instances change after creation (actually) no more, because security domain does not change in context. Therefore, I think it is better if the instances AuditManager, AuthenticationManager, AuthorizationManager, IdentityManager, TrustManager and MappingManager are stored in the JBossSecurityContext instance itself.
> Also in the same time the jndi lookup java:/policyRegistration is performed. I have not found the code location where the lookup is performed. But this source code should also be changed.
> ------------------------
> The customer did quite some investigation, there are more details in the case notes.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira