Security / Wrapped - Propagation

[darranl]

I am just submitting this as a draft to begin with for discussion.

Trying to support context propagation in WildFly one of the issues we encounter is needing to wrap the task being executed so that the security "context" is established and cleared around the call, this pull request is based on some previous discussions to attempt to solve this problem.

Within Java the JAAS Subject and AccessControlContext have really been the original security context so I have included a test case illustrating the propagation of the Subject and it's associated AccessControlContext. Today for Kerberos based authentication this pattern is still the primary approach to pass Kerberos credentials.

Within this PR the approach has been to add methods to the ThreadContextSnapshot to indicate the need to wrap and to handle the actual wrapping. Another strategy could be to use the Object returned from ThreadContextSnapshot.begin() - either on the ThreadContextController API or a separate API to indicate the need for and to handle the wrapping.

The goal would be to propose something for the specification and related TCK but I believe the preferred path is to try it out in an implementation first.

[FroMage]

Hi,

Thanks for your contribution!

I am not sure this is the right approach, though, because it doesn't support having more than one provider that needs wrapping, so it doesn't compose.

The MP-CP API isn't well suited for wrapping, though we never had the need for this until now. If we had to support it, I guess we'd have to turn the current code from:

restoredState[0] = capturedState[0].begin();
...
restoredState[N] = capturedState[N].begin();
userCode.call();
restoredState[0].end();
...
restoredState[N].end();

into:

wrapped = userCode;
wrapped = () -> {
 restoredState[N] = capturedState[N].begin();
 wrapped.call();
 restoredState[N].end();
};
...
wrapped = () -> {
 restoredState[0] = capturedState[0].begin();
 wrapped.call();
 restoredState[0].end();
};

Just like Russian dolls.

But this will be even less efficient than what we have ATM, and this won't work well with the fast providers we're trying to promote either in the latest API.

I wonder if OpenLiberty ran into this issue.

I assume for WF you implement Security yourselves, right? So perhaps you can rather make something WF-specific and allow access to mutating the state instead of going via the JDK APIs?

[darranl]

Hi @FroMage

The code that I have contributed here should support wrapping from multiple providers, but what I have tried to do here is make sure the wrapping is only applied for the providers that actually need it so it does not force a wrapping for the remaining providers.

The reason we went for this pattern within WildFly was to provide an API where we could be sure any ThreadLocal state was reliably cleaned up without relying on the caller as we had had a number of problems in the past where thread specific manipulations of a stack of identities had ended up out of sync leading to threads being corrupted. We could consider options with our APIs but it wouldn't eliminate the need to propagate the Subject the propagation of the JAAS Subject still has some specific use cases out of our control. As an example for a task establishing a database connection the JAAS Subject is associated with the AccessControlContext and accessed by the JDBC driver - at that point the JDBC driver would be using the Java APIs which in turn access the Subject.

[darranl]

I have been thinking a little further, most importantly the time the security "context" is captured causes us no issues at all so we can always create the ThreadContextSnapshot populated with the items we need for security - our only sticking point is the wrapped nature of the call.

An alternative idea could be to add some new form of wrapper SPI independent of the ThreadContextProvider for the wrapping of the task e.g. ContextAwareTaskWrapper. This will be discovered using the ServiceLoader API, so overall the call now becomes.

wrapper = {
  AccessControlContext.doAs(useCode::call, accessControlContext);
}

restoredState[0] = capturedState[0].begin();
...
restoredState[N] = capturedState[N].begin();
wrapper.call();
restoredState[0].end();
...
restoredState[N].end();

capturedState["Security"].begin() would then just set up the values on a ThreadLocal ready for the wrapper to make use of them.

[FroMage]

The code that I have contributed here should support wrapping from multiple providers

You're right, my mistake.

capturedState["Security"].begin() would then just set up the values on a ThreadLocal ready for the wrapper to make use of them.

But, inside your security implementation, where do you store the current security context? In a ThreadLocal I assume, right? So this would make it use two ThreadLocals, which doesn't seem very nice.

[darranl]

But, inside your security implementation, where do you store the current security context? In a ThreadLocal I assume, right? So this would make it use two ThreadLocals, which doesn't seem very nice.

+1 I am more in favour of going straight to the wrapped call and cut our the intermediate step, that suggestion was more to be less intrusive to the CP API.

Or the concept of wrapping could entirely be independent of the ThreadContextProvider API but I think that now risks a parallel API which can provide the same.

[FroMage]

And I would cut another middle man and just let CP access and set the ThreadLocal ;)

Now, if the Subject is outside of your control, that's another issue. I didn't know JDBC would access this. I don't think we're propagating it for Quarkus. Do you know which drivers do this and why?

[darranl]

Regarding the setting of the ThreadLocal we chose to follow the same pattern as is used by Subject and AccessControlContext to guarantee clean up. In prior security solutions we did have vulnerabilities where after many years code was refactored and clean up ended up out of sync so we wanted the API which prevented that. So our real reluctance is in relaxing that clean up.

The JAAS Subject although has been used server side (I believe because it existed) was actually a client side API for a client to establish credentials and associate them with the current thread for later use.

Regarding the JDBC drivers, they tend to be able to take a username and password provided directly for username / password based authentication but where they support Kerberos authentication they delegate to the GSSAPI apis which in turn are able to access the Subject from the AccessControlContext and subsequently use the credentials it contains. For the drivers I would need to dig out some paperwork to get the definitive list but I believe it is most if not all, certainly Microsoft and Oracle - I believe DB2. May need to double check for the open source ones such as MySQL and PostgreSQL. About five years back we did work through a big list of drivers for Kerberos support.

But the JDBC drivers are also not the exclusive users, other APIs such as DirContext - we have used this approach of associating the Subject with the DirContext to establish a connection to Microsoft's Active Directory using Kerberos authentication. Really the JAAS Subject is a general API to associate credentials that can be passed over without affecting the API but in real terms Kerberos authentication seems to be the place it is still used although it could in theory be used for many more.

Other Jakarta specifications such as JACC also have some expectations around the availability of a Subject although at that point not in relation to how it is propagated, in our case we can take our propagated SecurityIdentity and convert it to a JAAS Subject but that only works where the JACC APIs are used to access the Subject.

I did also think if we could provide some kind of API ourselves to just wrap the task but really that becomes a duplication of the contextual methods already on ThreadContext which already perform their own level of wrapping but this time it becomes a propitiatory API.

[njr-11]

I wonder if OpenLiberty ran into this issue.

It does have the same issue. It hadn't been reported before but after @FroMage made me aware of this, I just tried it. It seems like the ideal solution would be some additional API in the JDK to allow the establishment/removal of context to be done in separate steps. Do we have any way to request that?

[FroMage]

That's a slim chance. I can ask our OpenJDK team about this, though.

[FroMage]

I mean, there's a good chance they'll argue the other way and that our API should change…