Unfortunately I would not know how it should work with an async servlet. This is certainly a good thing to have specified and/or clarified.

I have a test for async in the suite, but it only tests if the presence of a SAM (that does nothing for that test) doesn't disturb an async dispatch from happening.

What now seems to happen in most servers is the same thing as what happens with a Servlet Filter. The SAM's secureResponse method is invoked after the last Filter has returned, but at that point the request is not necessarily done and another thread that has the AsyncContext can still write to the response.

E.g.

1. Request starts
2. SAM validateRequest invoked
2. Filter doFilter() invoked, before/in chain.doFilter()
3. Servlet invoked, starts async, returns
4. Filter after chain.doFilter(), returns
5. SAM secureResponse invoked
6. Other thread writes to response, calls AsyncContext#complete()
7. Request ends

Just like an existing non-async aware GZIP filter would not just work with an async servlet, a SAM that collects the response via a response wrapper would not work either.

Kind regards,
Arjan Tijms









On Wed, Oct 21, 2015 at 11:46 AM, Stuart Douglas <stuart.w.douglas@gmail.com> wrote:
One thing I am still not clear on is how this should work with async servlet, do you have any ideas?

Normally when a request has returned to the container it means it is done, however with async requests this is not the case.

Stuart

On Wed, 21 Oct 2015 at 19:51 arjan tijms <arjan.tijms@gmail.com> wrote:
On Wed, Oct 21, 2015 at 10:04 AM, Stuart Douglas <stuart.w.douglas@gmail.com> wrote:
Ah, I just re-read the spec and we are invoking this in the wrong place, it should be invoked on call stack return like you say, and not on response commit. The spec explicitly states that this may be called after the response has already been commited, but recommends a SAM buffer the full response using a HttpServletResponseWrapper if this is a problem (section B.9 ).

Okay, great to hear it's more clear now ;)


 

Stuart


On Wed, 21 Oct 2015 at 18:24 arjan tijms <arjan.tijms@gmail.com> wrote:
Hi,

On Wed, Oct 21, 2015 at 2:19 AM, Stuart Douglas <stuart.w.douglas@gmail.com> wrote:
I had a quick look into this, and I think your test is invalid.

Basically secureResponse() is called when the message is about to be sent, in the case of your example this happens when the call stack returns and the writer is closed. The means that secureResponse can't use the writer, as it has already been closed.

To be 100% sure, I'll verify this with the sped lead, Ron Monzillo. Indeed, I noticed earlier that Undertow was unable to write to the response at that point. You're right that the test isn't fully clear on this, and I already wanted to split it up in a "secureResponse called" (tested via some other method) and "able to write to the response from secureResponse".

My feeling says Undertow should not have closed the writer at this point and writing to the response should be possible. It's what the RI allows and what each and every other server including JBoss EAP 6 allows.

  
Even if the writer was not closed and the message was being commited due to the buffer being full this would still be wrong, as the writer would basically be writing to some random point in the middle of the stream. 

I'm not sure this is correct. secureResponse is not called whenever the buffer is full, but only after the (Servlet) resource has been invoked (meaning, after the target Servlet has executed and control returns from the last Filter).

One usecase for this is capturing the response fully via a wrapped response, and then in secureResponse, well, securing this in some way (perhaps encrypting it). As such it's quite like how a GZIP filter functions. Clearly the SAM has to be able to write to the response at that point.

As said, to be 100% sure I'll verify this with Ron.

Kind regards,
Arjan Tijms



 

Stuart

On Wed, 21 Oct 2015 at 10:04 arjan tijms <arjan.tijms@gmail.com> wrote:
On Wed, Oct 21, 2015 at 12:26 AM, Stuart Douglas <stuart.w.douglas@gmail.com> wrote:
Is this with proactive auth disabled?

It's with as much of the defaults as is possible.

Just unzipped a clean download of CR3, added the dummy activation code to standalone.xml, which is:

<security-domain name="jaspitest" cache-type="default">
    <authentication-jaspi>
        <login-module-stack name="dummy">
            <login-module code="Dummy" flag="optional"/>
        </login-module-stack>
        <auth-module code="Dummy"/>
    </authentication-jaspi>
</security-domain>

Started WildFly via JBoss tools, then in Java EE 7 samples project removed all modules in pom.xml except the JASPIC and support ones:

    <modules>
      <module>test-utils</module>
      <module>jaspic</module>
      <module>util</module>
    </modules>

Then in the root of the Java EE 7 project executed the following command:

mvn --fail-at-end test

So this is with all defaults, which I guess is true (enabled) then for what you call "proactive" according to this commit: 


Kind regards,
Arjan Tijms











 


Stuart

On Wed, 21 Oct 2015 at 09:02 arjan tijms <arjan.tijms@gmail.com> wrote:
Hi,

I just tested WildFly 10.0.0 rc3 and the revert seems to have gone well, since the behaviour is back to that of WildFly 9.

Meaning, most tests from the EE 7 samples now pass again, but calling of secureResponse (which 5298 was supposed to fix I think) now fails again:

testBasicSAMMethodsCalled(org.javaee7.jaspic.lifecycle.AuthModuleMethodInvocationTest): SAM method secureResponse not called, but should have been.


The JSF includes still fail too, but those were failing in WildFly 9 as well:

testJSFwithCDIIncludeViaPublicResource(org.javaee7.jaspictest.dispatching.JSFCDIIncludeTest): Response did not contain output from JSF view that SAM included.

testJSFIncludeViaPublicResource(org.javaee7.jaspictest.dispatching.JSFIncludeTest): Response did not contain output from JSF view that SAM included.

Kind regards,
Arjan Tijms







On Tue, Oct 13, 2015 at 1:16 PM, arjan tijms <arjan.tijms@gmail.com> wrote:
Hi,

Sorry to ask again, but any news on this topic yet?

If there's anything I can do to help just let me know.

Kind regards,
Arjan Tijms






On Mon, Oct 5, 2015 at 3:46 PM, arjan tijms <arjan.tijms@gmail.com> wrote:
> Hi,
>
> Just wondering, any updates here? Did you tried to run the JASPIC
> tests against the latest nightly?
>
> Kind regards,
> Arjan Tijms
>
> On Mon, Sep 28, 2015 at 4:58 PM, arjan tijms <arjan.tijms@gmail.com> wrote:
>> Hi,
>>
>> On Mon, Sep 28, 2015 at 7:49 AM, Stuart Douglas
>> <stuart.w.douglas@gmail.com> wrote:
>>> Can you verify that this PR: https://github.com/wildfly/wildfly/pull/8204
>>> fixes your issues?
>>
>> I did a quick test run, but now really everything breaks.
>>
>> In UndertowDeploymentInfoService the following line occurs:
>>
>>  deploymentInfo.setJaspiAuthenticationMechanism(new
>> JASPIAuthenticationMechanism(authMethod, securityDomain));
>>
>> But the constructor arguments are reversed.
>>
>> The arguments are defined as:
>>
>>  public JASPIAuthenticationMechanism(final String securityDomain,
>> final String configuredAuthMethod) {
>>
>> When I reverse the arguments to their correct order, the same tests as
>> before fail unfortunately.
>>
>> This individual case can be rather easily debugged manually by cloning
>> the project using Eclipse/JBoss tools from
>> https://github.com/javaee-samples/javaee7-samples Then importing just
>> the jaspic and test-utils directories as Maven projects, and then
>> deploying "jaspic-basic-authentication" using Add and Remove on the
>> server.
>>
>> If you then request:
>>
>> http://localhost:8080/jaspic-basic-authentication/public/servlet?doLogin
>>
>> You should see the same output as:
>>
>> http://localhost:8080/jaspic-basic-authentication/protected/servlet?doLogin
>>
>> Hope this helps
>>
>> Kind regards,
>> Arjan Tijms
>>
>>
>>
>>
>>
>>
>>
>>>
>>> Basically it reverts the recent changes, and instead installs a handler
>>> after authenticate to handle requests that do not require authentication.
>>>
>>> Stuart
>>>
>>> On Mon, 28 Sep 2015 at 09:54 Jason T. Greene <jason.greene@redhat.com>
>>> wrote:
>>>>
>>>> The MIT license also grants the ability to relicense.
>>>>
>>>> On Sep 27, 2015, at 6:50 PM, Stuart Douglas <stuart.w.douglas@gmail.com>
>>>> wrote:
>>>>
>>>> This was deliberately disabled to match the behavior of EAP6. It can be
>>>> controlled by the proactive-authentication attribute on the servlet
>>>> container. These JAPIC changes have been driven by this change, in order to
>>>> get the TCK to pass without proactive auth. I will look into it today.
>>>>
>>>> If you can contribute your tests it would be greatly appreciated, we do
>>>> not have much coverage (and apparently neither does the TCK, as it does not
>>>> pick up these issues). As I understand it if you are the sole author you
>>>> retain copyright and can re-license them under whatever license you like.
>>>>
>>>> Stuart
>>>>
>>>> On Sat, 26 Sep 2015 at 01:15 arjan tijms <arjan.tijms@gmail.com> wrote:
>>>>>
>>>>> Hi,
>>>>>
>>>>> I discovered some more issues originating from 5298:
>>>>>
>>>>> pre-emptive authentication on a public page doesn't work anymore
>>>>> either. It still worked fine in WildFly 9.0.1.
>>>>>
>>>>> This can be easily seen when running the JASPIC tests from
>>>>> https://github.com/javaee-samples/javaee7-samples/tree/master/jaspic
>>>>>
>>>>> For the basic authentication tests, the following now fail:
>>>>>
>>>>> Failed tests:
>>>>>
>>>>> testPublicPageNotRememberLogin(org.javaee7.jaspic.basicauthentication.BasicAuthenticationPublicTest)
>>>>>
>>>>> testPublicPageLoggedin(org.javaee7.jaspic.basicauthentication.BasicAuthenticationPublicTest)
>>>>>
>>>>> These tests don't rely on request#authenticate, but depend on
>>>>> automatic calling of a SAM at the beginning of a request. After manual
>>>>> inspection it's clear that the SAM is called, but its outcome is not
>>>>> being applied.
>>>>>
>>>>> Kind regards,
>>>>> Arjan
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> On Fri, Sep 25, 2015 at 3:18 PM, arjan tijms <arjan.tijms@gmail.com>
>>>>> wrote:
>>>>> > Hi,
>>>>> >
>>>>> > I checked again on the just released WildFly 10.0 CR2, but
>>>>> > unfortunately the code is still severely broken now.
>>>>> >
>>>>> > There are two main issues, and they're both in this fragment in
>>>>> > JASPIAuthenticationMechanism:
>>>>> >
>>>>> > if(isValid == null) {
>>>>> >     isValid = createJASPIAuthenticationManager().isValid(messageInfo,
>>>>> > new Subject(), JASPI_HTTP_SERVLET_LAYER,
>>>>> > attachment.getApplicationIdentifier(), new JBossCallbackHandler());
>>>>> > }
>>>>> >
>>>>> > The first problem is the "isValid == null" check. After the first call
>>>>> > to request#authenticate in a given request this will always be
>>>>> > non-null. The result is that a request for programmatic authentication
>>>>> > will effectively be ignored the first time.
>>>>> >
>>>>> > The second problem is that this passes in the JBossCallbackHandler,
>>>>> > but this doesn't know how to handle JASPIC callbacks and this will
>>>>> > result in an exception like the following:
>>>>> >
>>>>> > javax.security.auth.callback.UnsupportedCallbackException: PBOX00014:
>>>>> > org.jboss.security.auth.callback.JBossCallbackHandler does not handle
>>>>> > a callback of type
>>>>> > javax.security.auth.message.callback.CallerPrincipalCallback
>>>>> >     at
>>>>> > org.jboss.security.auth.callback.JBossCallbackHandler.handleCallBack(JBossCallbackHandler.java:138)
>>>>> >     at
>>>>> > org.jboss.security.auth.callback.JBossCallbackHandler.handle(JBossCallbackHandler.java:87)
>>>>> >
>>>>> > The code should pass in a JASPICallbackHandler here.
>>>>> >
>>>>> > Hope this can be fixed. Perhaps it's just a matter of removing the
>>>>> > "isValid == null" check and passing in the right callback handler.
>>>>> >
>>>>> > Kind regards,
>>>>> > Arjan Tijms
>>>>> >
>>>>> >
>>>>> >
>>>>> >
>>>>> >
>>>>> >
>>>>> >
>>>>> >
>>>>> > On Wed, Sep 23, 2015 at 5:58 PM, arjan tijms <arjan.tijms@gmail.com>
>>>>> > wrote:
>>>>> >> p.s. if I just revert JASPIAuthenticationMechanism to the previous
>>>>> >> version, but leaving in the new JASPICInitialHandler, then everything
>>>>> >> seems to work again. This is a bit of hacky workaround perhaps, but in
>>>>> >> some quick testing it does do the trick.
>>>>> >>
>>>>> >> On Wed, Sep 23, 2015 at 3:31 PM, arjan tijms <arjan.tijms@gmail.com>
>>>>> >> wrote:
>>>>> >>> Hi,
>>>>> >>>
>>>>> >>> It looks like that after WFLY-5298 (this commit specifically
>>>>> >>>
>>>>> >>> https://github.com/wildfly/wildfly/commit/121a305c59c3619bb747681c62d099dfddd82709#diff-540388fb45365d1d79353d8b4552bcf6)
>>>>> >>> HttpServletRequest#authenticate does not longer do anything.
>>>>> >>>
>>>>> >>> HttpServletRequest#authenticate calls though to
>>>>> >>> JASPIAuthenticationMechanism#authenticate.
>>>>> >>>
>>>>> >>> There it now obtains the attachment that was set by the new
>>>>> >>> JASPICInitialHandler, which calls the SAM at the beginning of the
>>>>> >>> request. And then uses the stored "isValid" outcome directly, without
>>>>> >>> calling the SAM again.
>>>>> >>>
>>>>> >>> See the code below:
>>>>> >>>
>>>>> >>>   public AuthenticationMechanismOutcome authenticate(final
>>>>> >>> HttpServerExchange exchange, final SecurityContext sc) {
>>>>> >>>         JASPICAttachment attachment =
>>>>> >>> exchange.getAttachment(JASPICAttachment.ATTACHMENT_KEY);
>>>>> >>>
>>>>> >>>         AuthenticationMechanismOutcome outcome;
>>>>> >>>         Account authenticatedAccount = null;
>>>>> >>>
>>>>> >>>         boolean isValid = attachment.isValid();
>>>>> >>>         final ServletRequestContext requestContext =
>>>>> >>> attachment.getRequestContext();
>>>>> >>>         final JASPIServerAuthenticationManager sam =
>>>>> >>> attachment.getSam();
>>>>> >>>         final JASPICallbackHandler cbh = attachment.getCbh();
>>>>> >>>
>>>>> >>>         GenericMessageInfo messageInfo = attachment.getMessageInfo();
>>>>> >>>         if (isValid) {
>>>>> >>>             // The CBH filled in the JBOSS SecurityContext, we need
>>>>> >>> to
>>>>> >>> create an Undertow account based on that
>>>>> >>>             org.jboss.security.SecurityContext jbossSct =
>>>>> >>> SecurityActions.getSecurityContext();
>>>>> >>>             authenticatedAccount =
>>>>> >>> createAccount(attachment.getCachedAccount(), jbossSct);
>>>>> >>>         }
>>>>> >>>
>>>>> >>> This is not correct I think. The code should call the SAM once again
>>>>> >>> and use the outcome from that call.
>>>>> >>>
>>>>> >>> Am I missing something, or was the new call to the SAM simply
>>>>> >>> forgotten at this point?
>>>>> >>>
>>>>> >>> Kind regards,
>>>>> >>> Arjan Tijms
>>>>> _______________________________________________
>>>>> wildfly-dev mailing list
>>>>> wildfly-dev@lists.jboss.org
>>>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>>>
>>>> _______________________________________________
>>>> wildfly-dev mailing list
>>>> wildfly-dev@lists.jboss.org
>>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev