pingidentity / ldapsdk

UnboundID LDAP SDK for Java
Other
334 stars 81 forks source link

RFE: Make either SaslHelper ir applySASLQoP public #95

Open asaf-romano opened 3 years ago

asaf-romano commented 3 years ago

In order to implement a "chatty" SASL mechanism similar to the built-in gssapi jaas-based mechanism, one would have to duplicate all of the logic in the package-private SASLHelper. That's OK, I guess, but one evident issue is that LDAPConnection::.applySASLQoP is also private, making such implementation impossible without some reflection.

dirmgr commented 3 years ago

I have just committed a change that renames SASLHelper to SASLClientBindHandler and makes that class and its methods public. I also added a couple of clarifying statements to the documentation.

The reason that the class needed to be renamed is that classes that end with "Helper" are generally intended for internal use within the LDAP SDK and are specifically excluded from the generated Javadoc documentation.

asaf-romano commented 3 years ago

First of all, thanks for the swift response and commit! 🙏

However, after testing a little bit with the help of reflection etc, it seems that my flow is going to be slightly different - that is, I'll need direct access to applySASLQoP and sendBindRequest - the former required reflection at the moment, whereas the earlier is merely protected, so one can get around it by overriding the method or exposing a public version.

dirmgr commented 3 years ago

Could you explain more about what you're doing and why the updated version won't work? I would expect that it should work with anything that uses a Java SaslClient and that uses the Sasl.QOP property with a value of auth-int or auth-conf to indicate that there is a negotiated security layer. Does your implementation use a different property or property value? Would it help if I updated the SASLBindRequest class to provide a method that you can override to indicate whether there is a negotiated security layer?

asaf-romano commented 3 years ago

I'm implementing a native GSS-SPNEGO flow (no jaas and such fun), which is not a 4-way bind the way GSSAPI is. In this flow, the initial BindResult is SUCCESS, even though there's still more work to do (but no further bind messages!): (1) evaluateChallenge should be called with serverSASLCredentials (the value is need by native api context object) (2) QOP has to be applied

Now, neither happen because of this early return in SASLHelper:

      if (! bindResult.getResultCode().equals(ResultCode.SASL_BIND_IN_PROGRESS))
      {
        return bindResult;
      }

I think (a) is broken "as expected" (that is, it makes sense in the context of regular GSSAPI), but (b) is more of a plain bug: QOP, in theory, could be applied before the first call to evaluateChallenge.

Would it help if I updated the SASLBindRequest class to provide a method that you can override to indicate whether there is a negotiated security layer?

This sounds promising, moreover - a flag in GenericSASLBindRequest might have worked as well. So far I've done my (successful) testing by inheriting from SASLBindRequest and invoking applyQOP with reflection. I didn't use GenericSASLBindRequest because I still have to call applyQOP on each concrete connection (and not on the pool I'm creating, that is).

asaf-romano commented 3 years ago

Actually, I take back my comment regarding GenericSASLBindRequest. I need to create a native security context for each bind request, so it all has to happen within process. Anyway, for the SASLHelper to work in this case, an option to apply QOP by overriding a method in SASLBindRequest would fix (b), but not (a).

dirmgr commented 3 years ago

OK. I have added a public LDAPConnection.applySASLSecurityLayer(SaslClient) method that can be used to apply a security layer that was negotiated using the provided SaslClient object. The former applySASLQoP method is still there (the new applySASLSecurityLayer method just calls it to do its work) in case you still need to use it via reflection for some reason.