Bug 1166719

This issue causes that Narayana's LRCO does not work when Kerberos authentication is in use. 

This case is when when Hibernate is involved. 
The datasource connection is enlisted to transaction first when it's used. Then at time of transaction commit the beforeCompletition hook is called. Then Hibernate does some handling with database and asks for connection. Because of the issue on pool mapping a new connection is created instead of using old one. This new connection is then tried to be enlisted to transaction but as there can't be two standard datasources involved in one XA transaction the transaction fails.



Adding some observation taken when I tried to find the cause of the problem.
----------
Subject taken from Picketbox is propagated to getManagedConnectiom method (https://github.com/ironjacamar/ironjacamar/blob/1.0/core/src/main/java/org/jboss/jca/core/connectionmanager/pool/AbstractPool.java#L172) where is used as key of map that defines if connection to resource is known (then it's returned) or unknown (then new connection is created). The key in this case is class SubjectKey that wraps the instance of Subject taken from Picketbox (https://github.com/ironjacamar/ironjacamar/blob/1.0/core/src/main/java/org/jboss/jca/core/connectionmanager/pool/strategy/SubjectKey.java#L65). It seems to me that problem of this class is different behavior of method hashCode and equals. I mean that if hashCode of two SubjectKey instances is the same the equals method could return 'false'. This is caused by fact that Subject's method equals takes care of private credentials part of Subject (works with principals, public credentials and private credentials). But method hashCode works only with principals and public credentials. This fact causes that AbstractPool's map mcpPools will map the same subject with two different private parts as two different entities. Which would cause creation of a new connection for each such subject.

We are not changing security model for external systems in an EAP minor/patch release.

This BZ will remain in devel_ack- until a valid test case against the IronJacamar 1.2 branch has been submitted.

What is invalid in test case that I mentioned in description. It shows how IJ behaves with two similar Subjects, both with the same public parts and different private parts. I can clean the code and send PR if you say that it is valid and can be added to TS.  

git clone -b subject_matching_test https://github.com/simkam/ironjacamar.git
ant -Dmodule=core one-test -Dtest=org.jboss.jca.core.connectionmanager.pool.TestConnectionTestCase

Leak in test is intentional, it checks that exception

ResourceException: IJ000655: No managed connections available within configured blocking timeout (30000 [ms])

is thrown when pool is exhausted. But in this case, with credentials obtained through SPNEGO, it is not thrown. The exception is thrown in all other cases - static kerberos credentials, username/password. 

It is because MCPs are keyed on Subject equals() and with SPNEGO Subject is always different, some parts of GssCredential inside Subject#privateCredentials differs (e.g., ticket expiration). This can also affect authentication with static kerberos credentials when cache on security domain is disabled.
 
New Subject causes that new MCP is always created. In other scenarios Subject doesn't change and existing MCP is reused and it is detected that MCP is exhausted and exception is thrown. 

Class SubjectKey [1] delegates equals() to jdk specific Subject.equals(). Subject.equals() checks principals, public credentials and private credentials. On other hand Subject.hashCode() counts only with principals and public credentials, not with private credentials. So there is inconsistency between .equals() and .hashCode() methods.
 
org.jboss.jca.core.connectionmanager.pool.strategy.SubjectKey.equals()

@Override
public boolean equals(Object obj)
{
   if (this == obj)
   {
      return true;  
   }
   if (obj == null || !(obj instanceof SubjectKey))
   {
      return false;  
   }
   SubjectKey other = (SubjectKey) obj;
   
   return SecurityActions.equals(subject, other.subject)
      && separateNoTx == other.separateNoTx;
}

org.jboss.jca.core.connectionmanager.pool.strategy.SecurityActions.equals()

static boolean equals(final Subject s1, final Subject s2)
{
   if (System.getSecurityManager() == null)
      return s1 != null ? s1.equals(s2) : s2 == null;
   Boolean equals = AccessController.doPrivileged(new PrivilegedAction<Boolean>() 
   {
      public Boolean run()
      {
         return s1 != null ? s1.equals(s2) : s2 == null;
      }
   });
   return equals.booleanValue();
}

We tried to change SubjectKey.equals() to ignore private credentials by checking equality of Subjects' hash codes. Some smoke tests we ran with this change show no difference in scenarios like username/password, static kerberos credentials and it also fixed SPNEGO scenario.

-      return SecurityActions.equals(subject, other.subject)
-         && separateNoTx == other.separateNoTx;
+      return hashCode() == other.hashCode()
+              && separateNoTx == other.separateNoTx;

[1] https://github.com/ironjacamar/ironjacamar/blob/1.0/core/src/main/java/org/jboss/jca/core/connectionmanager/pool/strategy/SubjectKey.java#L65

While Jesper is on PTO, just to point out that two things that are completely unrelated can hash to the same thing. If you key the pool on hashCode it will result in bad things eventually.

Yes, I do not suggest using hashCode as key. It was meant as an example that omitting private credentials from key can solve issues that we're seeing.

Kabir Khan <kabir.khan> updated the status of jira EAP6-215 to Resolved

Subject based pools are split based on the contracts of javax.security.auth.Subject hashCode() and equals() methods, where private credentials matter.

Test case:

https://github.com/ironjacamar/ironjacamar/commit/8d29fb667331ad70fe0ca617bc8f821d6bdbb15a

Splitting Subject based pools on other criterias is a feature request, and it needs to be investigated if it is at all possible, since the resource adapter instance won't know about this special split. Minimum requirement is that the contracts of createManagedConnection() and matchManagedConnections() are maintained.

ok, closing

Note: there is known issue when enlisting non-xa resource to xa transaction when security domain cache is disabled, see comment 1.

John Doyle <jdoyle> updated the status of jira EAP6-215 to Closed