glassfish
  1. glassfish
  2. GLASSFISH-20930

JASPIC 1.1's register session makes authenticated identity available when SAM chose not to inherit the session

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0
    • Fix Version/s: 4.1
    • Component/s: security
    • Labels:
      None

      Description

      JASPIC 1.1 introduced a feature where the container remembers the authenticated session. However, the SAM decides whether the application gets to see the identity that was stored in the container session or something else (see GLASSFISH-2031). (note though that the session should remain intact even when a SAM choses not the inherit/join it for a particular request)

      When the SAM decides that the session should NOT be applied to the request, e.g. by invoking the handler with a CallerPrincipalCallback and a null for the Principal argument, then on GlassFish 4 a protected resource for which the identity in the container session would have access is correctly denied.

      However, for a public resource the Servlet's request.getUserPrincipal incorrectly returns the principal from the container session. In case the SAM passed a null for the Principal in the CallerPrincipalCallback and invoked the handler with that, the unauthenticated user should have been associated with the request and hence request.getUserPrincipal should have returned a null.

      I've created a unit test at https://github.com/javaee-samples/javaee7-samples/tree/master/jaspic/register-session that shows the incorrect behavior. I've also asked Ron Monzillo about the behavior and he confirmed that if the SAM really passes a null into CallerPrincipalCallback then the application should indeed not see anything else than a null from request.getUserPrincipal for that specific request.

        Activity

        Hide
        arjan tijms added a comment - - edited

        Jeff, is there still a chance to change "future release' to 4.0.1?

        The problem here is that JASPIC is apparently a rather difficult spec for vendors to comprehend and implement correctly, and the TCK sure isn't helping.

        This leaves GlassFish as pretty much the only option left to convince vendors that things should be implemented in a certain way. If the spec isn't clear, the TCK non-existing for a certain range of features and GlassFish not implementing it correctly, then there's little hope that any other vendor will know how to implement it.

        For the "remember authenticated session" feature this is especially important since it's not really clear at all how this should be implemented. We had a long discussion with Ron and a vendor representative about this and the confusion about the exact behavior was indeed rather big.

        Show
        arjan tijms added a comment - - edited Jeff, is there still a chance to change "future release' to 4.0.1? The problem here is that JASPIC is apparently a rather difficult spec for vendors to comprehend and implement correctly, and the TCK sure isn't helping. This leaves GlassFish as pretty much the only option left to convince vendors that things should be implemented in a certain way. If the spec isn't clear, the TCK non-existing for a certain range of features and GlassFish not implementing it correctly, then there's little hope that any other vendor will know how to implement it. For the "remember authenticated session" feature this is especially important since it's not really clear at all how this should be implemented. We had a long discussion with Ron and a vendor representative about this and the confusion about the exact behavior was indeed rather big.
        Hide
        reza_rahman added a comment -

        This honestly sounds like an issue to solve in a security JSR for Java EE 8+ which we've gotten quite a bit of community feedback on. I'm not sure but perhaps it's best to wait a bit more and tackle it properly there?

        Show
        reza_rahman added a comment - This honestly sounds like an issue to solve in a security JSR for Java EE 8+ which we've gotten quite a bit of community feedback on. I'm not sure but perhaps it's best to wait a bit more and tackle it properly there?
        Hide
        arjan tijms added a comment -

        This honestly sounds like an issue to solve in a security JSR for Java EE 8+ which we've gotten quite a bit of community feedback on. I'm not sure but perhaps it's best to wait a bit more and tackle it properly there?

        The entire concept of remembering a session definitely should be given another thought and this may indeed be done best in whatever discussion comes up for Java EE 8 and beyond (be it a new security JSR, a MR for JASPIC, a new major JASPIC release, or something else). It's perhaps a shame that the discussion I referred to above didn't took place on a public mailing list, but the summary of it is that the current specification for this is rather vague to say the least.

        That said, this particular JIRA issue is concerned with just a small behavioral bug in the GlassFish code. Most likely a simple field isn't correctly cleared out somewhere at the right time.

        Show
        arjan tijms added a comment - This honestly sounds like an issue to solve in a security JSR for Java EE 8+ which we've gotten quite a bit of community feedback on. I'm not sure but perhaps it's best to wait a bit more and tackle it properly there? The entire concept of remembering a session definitely should be given another thought and this may indeed be done best in whatever discussion comes up for Java EE 8 and beyond (be it a new security JSR, a MR for JASPIC, a new major JASPIC release, or something else). It's perhaps a shame that the discussion I referred to above didn't took place on a public mailing list, but the summary of it is that the current specification for this is rather vague to say the least. That said, this particular JIRA issue is concerned with just a small behavioral bug in the GlassFish code. Most likely a simple field isn't correctly cleared out somewhere at the right time.
        Hide
        reza_rahman added a comment -

        If this is indeed a small amount of work that could be very helpful, I'd recommend digging into it and contributing a patch if practical? That may be great background for any Java EE 8+ security discussion anyway?

        Show
        reza_rahman added a comment - If this is indeed a small amount of work that could be very helpful, I'd recommend digging into it and contributing a patch if practical? That may be great background for any Java EE 8+ security discussion anyway?
        Hide
        Nithya Ramakrishnan added a comment -

        This issue is now resolved. The register session test case now passes.

        Sending webintegration/src/main/java/com/sun/web/security/RealmAdapter.java
        Transmitting file data .
        Committed revision 63277.

        Show
        Nithya Ramakrishnan added a comment - This issue is now resolved. The register session test case now passes. Sending webintegration/src/main/java/com/sun/web/security/RealmAdapter.java Transmitting file data . Committed revision 63277.

          People

          • Assignee:
            Nithya Ramakrishnan
            Reporter:
            arjan tijms
          • Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: