Re: [wildfly-dev] Regression after WFLY-5298; request#authenticate does nothing now

On Wed, Oct 21, 2015 at 10:04 AM, Stuart Douglas <stuart.w.douglas(a)gmail.com Okay, great to hear it's more clear now ;) > > Stuart > > > On Wed, 21 Oct 2015 at 18:24 arjan tijms <arjan.tijms(a)gmail.com> >> Hi, >> >> On Wed, Oct 21, 2015 at 2:19 AM, Stuart Douglas < >> stuart.w.douglas(a)gmail.com>> >>> 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(a)gmail.com>>> >>>> On Wed, Oct 21, 2015 at 12:26 AM, Stuart Douglas < >>>> stuart.w.douglas(a)gmail.com>>>> >>>>> 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: >>>> >>>> >>>> https://github.com/wildfly/wildfly/commit/8a41c10eb56a827332355b921834c85... >>>> >>>> Kind regards, >>>> Arjan Tijms >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>> >>>>> >>>>> >>>>> Stuart >>>>> >>>>> On Wed, 21 Oct 2015 at 09:02 arjan tijms <arjan.tijms(a)gmail.com&gt; >>>>>>>>> >>>>>> 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(a)gmail.com&gt; >>>>>>>>>>> >>>>>>> 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(a)gmail.com&gt; >>>>>>>>>>>>> > 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(a)gmail.com>>>>>>> >> Hi, >>>>>>> >> >>>>>>> >> On Mon, Sep 28, 2015 at 7:49 AM, Stuart Douglas >>>>>>> >> <stuart.w.douglas(a)gmail.com>>>>>>> >>> 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(a)redhat.com&gt; >>>>>>> >>>>>>>>> >>>> >>>>>>> >>>> The MIT license also grants the ability to relicense. >>>>>>> >>>> >>>>>>> >>>> On Sep 27, 2015, at 6:50 PM, Stuart Douglas < >>>>>>> stuart.w.douglas(a)gmail.com&gt; >>>>>>> >>>>>>>>>> >>>> >>>>>>> >>>> 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(a)gmail.com&gt; >>>>>>>>>>>>> >>>>> >>>>>>> >>>>> 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(a)gmail.com&gt; >>>>>>> >>>>>>>>>>> >>>>> > 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(a)gmail.com&gt; >>>>>>> >>>>> >>>>>>> >>>>> >> 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(a)gmail.com&gt; >>>>>>> >>>>> >>>>>>>> >>>>> >>> Hi, >>>>>>> >>>>> >>> >>>>>>> >>>>> >>> It looks like that after WFLY-5298 (this commit >>>>>>> specifically >>>>>>> >>>>> >>> >>>>>>> >>>>> >>> >>>>>>> https://github.com/wildfly/wildfly/commit/121a305c59c3619bb747681c62d099d... >>>>>>> ) >>>>>>> >>>>> >>> 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(a)lists.jboss.org >>>>>>> >>>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev >>>>>>> >>>> >>>>>>> >>>> _______________________________________________ >>>>>>> >>>> wildfly-dev mailing list >>>>>>> >>>> wildfly-dev(a)lists.jboss.org >>>>>>> >>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev >>>>>>> >>>>>> >>>>>>