1 2 3 4 Previous Next 48 Replies Latest reply: Feb 8, 2012 6:30 AM by sivagurut RSS

Major security leak PicketLink and testing on ADFSv2

Pipo Pipo Newbie

Hello,

 

I have tried to get PicketLink to work with Microsoft ADFSv2 and I have found a few issues and bugs with PicketLink. After
fixing these bugs and problems all seem to work well. You need to setup ADFSv2 so it will match PicketLink. With these changes PicketLink seems to work with ADFSv2. Only the POST profile seems to work, there is a problem with the redirect profile because ADFS answers the redirect request with a POST-profile anwser;

 

org.picketlink.identity.federation.bindings.tomcat.sp.SPPostSignatureFormAuthenticator
org.picketlink.identity.federation.bindings.tomcat.sp.SPPostFormAuthenticator

 

Claim:
    SAM-Account-Name => Name ID
    Token-Groups-Unqualified-Names => Role

 

Relying party identifier: https://webtest.personnelview.nl:8443/employee/
End-point to: https://webtest.personnelview.nl:8443/employee/

 

Advanced setting: SHA-1 (needed when you use the SPPostSignatureFormAuthenticator)

 

<IdentityURL>https://test4.nietdus.nl/adfs/ls/</IdentityURL>
<ServiceURL>https://webtest.personnelview.nl:8443/employee/</ServiceURL>

 

<KeyProvider ClassName="org.picketlink.identity.federation.core.impl.KeyStoreKeyManager">
   <Auth Key="KeyStoreURL" Value="/jbid_test_keystore.jks" />
   <Auth Key="KeyStorePass" Value="store123" />
   <Auth Key="SigningKeyPass" Value="pipo" />
   <Auth Key="SigningKeyAlias" Value="pipo" />
   <ValidatingAlias Key="test4.nietdus.nl" Value="test4.nietdus.nl"/>
</KeyProvider>

 

You need a valid certificate with a valid root CA on your service provider, as ADFSv2 will check this!

 

 

- It looks like in SPPostSignatureFormAuthenticator the method verifySignature is never been called to verify the signing of the IDP (the SPFilter does check the signing of the IDP) !!! This is a very very big security leak!!!!

 

I have changed the SPPostFormAuthenticator so it will call the verifySignature method which is already present;

 

if(destination != null &&
                  samlResponseDocument != null)
            {
               sendRequestToIDP(destination, samlResponseDocument, relayState, response, willSendRequest);
            }
            else
            {
                 // TODO EPD Check the signing of the IDP XML document
                 byte[] qbase64DecodedResponse = PostBindingUtil.base64Decode(samlResponse);
                 InputStream is = new ByteArrayInputStream(qbase64DecodedResponse);
                 SAML2Response qsaml2Response = new SAML2Response();
                 SAML2Object qsamlObject = qsaml2Response.getSAML2ObjectFromStream(is);
                 SAMLDocumentHolder documentHolder = qsaml2Response.getSamlDocumentHolder();
                 if(!verifySignature(documentHolder))
                      throw new ServletException("Cannot verify sender");

 

- ADFSv2 needs the Destination attribute with a AuthNRequest. The call saml2HandlerResponse.setDestination() is being set after the document has been formatted. I have changed the code so the identityURL has been passed on to the process;

 

saml2HandlerResponse = baseProcessor.process(httpContext, handlers, chainLock, identityURL);

 

- The IDP logout request still has a problem, it throws an error on ADFSv2. I have to look into this.

 

- The role response of ADFSv2 cannot be parsed correctly;

 

        <AttributeStatement>
            <Attribute Name="http://schemas.microsoft.com/ws/2008/06/identity/claims/role">
                <AttributeValue>Domain Users</AttributeValue>
                <AttributeValue>manager</AttributeValue>
            </Attribute>
           
To parse the response I have changed SAML2AuthenticationHandler to;

 

//  Let us get the roles
AttributeStatementType attributeStatement = (AttributeStatementType) assertion.getStatementOrAuthnStatementOrAuthzDecisionStatement().get(0);
List<Object> attList = attributeStatement.getAttributeOrEncryptedAttribute();
for(Object obj:attList)
{
    AttributeType attr = (AttributeType) obj;
    List<Object> objects = attr.getAttributeValue() ;
   
    for(Object o : objects)
    {
        if (o instanceof String)
        {
            // PicketLink IDP gives a string
            String roleName = (String) o ;
            roles.add(roleName);
        }
        else
        {
            // For ADFSv2 you can not cast it a String but you will get a NODE
            Node n = (Node) o ;
            Node value = n.getFirstChild() ;
       
            String roleName = value.getNodeValue() ;
            roles.add(roleName);
        }
    }
}

 

 

Hopefully you will add these changes to PicketLink, let me know if you need any more information!

 

Thanks,

 

Edwin

  • 1. Re: Major security leak PicketLink and testing on ADFSv2
    Pipo Pipo Newbie

    I have changed one more thing to get PicketLink working with ADFSv2;

     

    The signature of Picketlink can not be parsed bu ADFSv2 because ADFSv2 does not support the xml-exc-c14n#with-comments directive, and needs the envelopped signature not to be last in the transform list. So I have changed the XMLSignatureUtil class;

     


         Transform transform1 = fac.newTransform(Transform.ENVELOPED,
               (TransformParameterSpec) null);
         Transform transform2 = fac.newTransform("http://www.w3.org/2001/10/xml-exc-c14n#",
                 (TransformParameterSpec) null);
        
         List<Transform> transformList = new ArrayList<Transform>() ;
         transformList.add(transform1);
         transformList.add(transform2);
         Reference ref = fac.newReference
           ( referenceURI, digestMethodObj,transformList,null, null);

        String canonicalizationMethodType = CanonicalizationMethod.EXCLUSIVE;

     

    Hopefully you guys can put this in the next release of PicketLink!

     

    Thanks,

     

    Edwin

  • 2. Re: Major security leak PicketLink and testing on ADFSv2
    Anil Saldhana Master

    The POST profile is recommended.

     

    We will take a look at the changes that need to be made for ADFS2 interop.

  • 3. Re: Major security leak PicketLink and testing on ADFSv2
    Anil Saldhana Master

    Pipo, would you be willing to create a cheatsheet of instructions for PicketLink to work with ADFS2?  You can use the wiki to create the cheatsheet.

     

    The PicketLink community will appreciate your gesture.

     

    I have created a JIRA issue to make the XMLSignatureUtil class algorithm configurable.

    https://jira.jboss.org/browse/PLFED-91

     

     

    Additional bugs that have been entered include PLFED-92 and PLFED-93

  • 4. Re: Major security leak PicketLink and testing on ADFSv2
    Pipo Pipo Newbie

    It is no problem to write a how to wiki page for ADFS2, however without all the changes I proposed within the PicketLink source code it will not work with ADFS2. All the changes I have proposed are needed, not just the one about the enc-with-comments.

     

    Can you please also comment on the the security leak I possible have found (not checking the signing of the IDP) as this seems like a big thing to me and my customers!

     

    Kind regards,

     

    Edwin

  • 5. Re: Major security leak PicketLink and testing on ADFSv2
    Anil Saldhana Master

    Trunk has all the fixes now.  You want to give it a shot?  Some of the fixes to the problems you mentioned are not the way you show above.

     

    To customize xml signature canonicalization method, use the system property "picketlink.xmlsig.canonicalization"

     

    I need to test the fixes a bit better.  But I thought I will give it to you.

  • 6. Re: Major security leak PicketLink and testing on ADFSv2
    Pipo Pipo Newbie

    I have checked the trunk but you forgot one thing with the canonicalization. Microsoft ADFS2 does not approve of only a Transform transform = fac.newTransform(Transform.ENVELOPED in the package list. It needs another entry or you get the following error in the event log of ADFS2;

     

    Exception details:
    System.NotSupportedException: ID6027: Enveloped Signature Transform cannot be the last transform in the chain. The last transform must compute the digest which Enveloped Signature transform is not capable of.

     

    Thats why I added;

     

    Transform transform1 = fac.newTransform(Transform.ENVELOPED,
                (TransformParameterSpec) null);
         Transform transform2 =  fac.newTransform("http://www.w3.org/2001/10/xml-exc-c14n#",
                  (TransformParameterSpec) null);
        
         List<Transform>  transformList = new ArrayList<Transform>() ;
          transformList.add(transform1);
         transformList.add(transform2);
          Reference ref = fac.newReference
           ( referenceURI,  digestMethodObj,transformList,null, null);

     

    to the XMLSignatureUtil class.


    Also I do not like your solution of a system propery. I think it is MUCH nicer to put these configuration parameters in the xml files.


    Let me know,


    Edwin


  • 7. Re: Major security leak PicketLink and testing on ADFSv2
    Anil Saldhana Master

    The system property is just a temporary solution until we get a formal configuration parameter (which will be in a future release).

     

    I think it is time to get in some configuration parameters for this usecase.   That will include the Transform fix also.

  • 8. Re: Major security leak PicketLink and testing on ADFSv2
    Ray Cardillo Newbie

    I hate to bump this thread, but I just found a similar problem with the METRO stack (using JBossWS-Metro), so I'm going to contribute a comment here in case it helps others when they start searching for a solution.  I just spent about one full week debugging a related problem (relating to CanonicalizationMethod) and didn't get a solid answer until I downloaded source code and started stepping through the METRO implementation.  I don't know about the most recent release, but the version that the current release of JBossWS-Metro uses is based upon Metro 1.3.1, so that's what I am working with.  In that source code, if the CanonicalizationMethod has the #WithComments then it throws a NullPointerException that is deeply nested through so many dynamic runtime calls that the true source of the problem is not captured in the stack trace.  It would have been nice if they handled this gracefully, but instead, one XML parsing step creates a Canonicalization object based on this setting (and it gets set to null because #WithComments is not supported) and then a subsequent parsing step tries to use that null pointer assuming that it was previously set.  Here are some references for those interested:

     

    Reference 1:

         SignedInfoProcessor.java, line 224 and line 267 for details:

         https://xwss.dev.java.net/source/browse/xwss/xwss-ri/src/com/sun/xml/ws/security/opt/impl/incoming/processor/SignedInfoProcessor.java?annotate=1.22

     

    Reference 2:

         An old (but highly relevant) discussion on a WSIT forum:

         http://forums.java.net/jive/message.jspa?messageID=224596#224596

     

    Reference 3:

         On the JBossWS-Metro stack, it manifests as a SOAPFault without relevant details:

    <S:Envelope xmlns:S="http://schemas.xmlsoap.org/soap/envelope/">
       <S:Body>
          <S:Fault xmlns:ns4="http://www.w3.org/2003/05/soap-envelope">
             <faultcode xmlns:wsse="http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-secext-1.0.xsd">wsse:InvalidSecurity</faultcode>
             <faultstring>java.lang.NullPointerException</faultstring>
             <detail>
                <ns2:exception note="To disable this feature, set com.sun.xml.ws.fault.SOAPFaultBuilder.disableCaptureStackTrace system property to false" xmlns:ns2="http://jax-ws.dev.java.net/">
                   <message>java.lang.NullPointerException</message>
                   <ns2:stackTrace>
                      <ns2:frame file="SOAPUtil.java" line="193" method="getSOAPFaultException"/>
                      <ns2:frame file="SecurityServerTube.java" line="219" method="processRequest"/>
                      <ns2:frame file="Fiber.java" line="598" method="__doRun"/>
                      <ns2:frame file="Fiber.java" line="557" method="_doRun"/>
                      <ns2:frame file="Fiber.java" line="542" method="doRun"/>
                      <ns2:frame file="Fiber.java" line="439" method="runSync"/>
                      <ns2:frame file="WSEndpointImpl.java" line="243" method="process"/>
                      <ns2:frame file="HttpAdapter.java" line="471" method="handle"/>
                      <ns2:frame file="HttpAdapter.java" line="244" method="handle"/>
                      <ns2:frame file="ServletAdapter.java" line="135" method="handle"/>
                      <ns2:frame file="WSServletDelegate.java" line="129" method="doGet"/>
                      <ns2:frame file="WSServletDelegate.java" line="160" method="doPost"/>
                      <ns2:frame file="WSServlet.java" line="75" method="doPost"/>
                      <ns2:frame file="HttpServlet.java" line="637" method="service"/>
                      <ns2:frame file="HttpServlet.java" line="717" method="service"/>
                      <ns2:frame file="ApplicationFilterChain.java" line="290" method="internalDoFilter"/>
                      <ns2:frame file="ApplicationFilterChain.java" line="206" method="doFilter"/>
                      <ns2:frame file="ReplyHeaderFilter.java" line="96" method="doFilter"/>
                      <ns2:frame file="ApplicationFilterChain.java" line="235" method="internalDoFilter"/>
                      <ns2:frame file="ApplicationFilterChain.java" line="206" method="doFilter"/>
                      <ns2:frame file="StandardWrapperValve.java" line="235" method="invoke"/>
                      <ns2:frame file="StandardContextValve.java" line="191" method="invoke"/>
                      <ns2:frame file="SecurityAssociationValve.java" line="190" method="invoke"/>
                      <ns2:frame file="AuthenticatorBase.java" line="525" method="invoke"/>
                      <ns2:frame file="JaccContextValve.java" line="92" method="invoke"/>
                      <ns2:frame file="SecurityContextEstablishmentValve.java" line="126" method="process"/>
                      <ns2:frame file="SecurityContextEstablishmentValve.java" line="70" method="invoke"/>
                      <ns2:frame file="StandardHostValve.java" line="127" method="invoke"/>
                      <ns2:frame file="ErrorReportValve.java" line="102" method="invoke"/>
                      <ns2:frame file="CachedConnectionValve.java" line="158" method="invoke"/>
                      <ns2:frame file="StandardEngineValve.java" line="109" method="invoke"/>
                      <ns2:frame file="CoyoteAdapter.java" line="330" method="service"/>
                      <ns2:frame file="Http11Processor.java" line="829" method="process"/>
                      <ns2:frame file="Http11Protocol.java" line="598" method="process"/>
                      <ns2:frame file="JIoEndpoint.java" line="447" method="run"/>
                      <ns2:frame file="Thread.java" line="619" method="run"/>
                   </ns2:stackTrace>
                </ns2:exception>
             </detail>
          </S:Fault>
       </S:Body>
    </S:Envelope>

     


    To pursue a solution, I am going to build the latest PicketLink source and see if the aforementioned configuration option does the trick.  Based on the forum posting I referenced above, it does not sound like XWSS or METRO is going to fix this any time soon because apparently the relevant specification(s) do not cover the topic explicitly (regarding support of #WithComments), and the latest source code does not appear to have any changes in this regard.  So it looks like the only solution is going to be in the STS implementations themselves, as PicketLink has done.

  • 9. Re: Major security leak PicketLink and testing on ADFSv2
    Pipo Pipo Newbie

    ANIL SALDHANA wrote:

     

    The system property is just a temporary solution until we get a formal configuration parameter (which will be in a future release).

     

    I think it is time to get in some configuration parameters for this usecase.   That will include the Transform fix also.

     

    Is there a new test version which I can try on ADFSv2 ?

     

    Many Thanks!

  • 10. Re: Major security leak PicketLink and testing on ADFSv2
    Anil Saldhana Master

    Not yet. In few days.

  • 11. Re: Major security leak PicketLink and testing on ADFSv2
    Anil Saldhana Master

    Edwin,  I am not sure your transform (the second one) is correct in the XMLSignatureUtil class. You basically are including a transform of a canonicalization method.

     

    Based on http://java.sun.com/javase/6/docs/api/javax/xml/crypto/dsig/Transform.html, there are potentially 4-5 transforms that can be applied and the one you are adding seems to be a hack for ADFS2.

     

    I will dig further.

  • 12. Re: Major security leak PicketLink and testing on ADFSv2
    Anil Saldhana Master

    Edwin,  I added the transform change as part of https://jira.jboss.org/browse/PLFED-100

     

    Also, for the canonicalizationMethod customization,  https://jira.jboss.org/browse/PLFED-91 has the system property removed and you can configure the algorith, via attributes.

     

    The trunk should have the changes.

     

    Tell us if the ADFS2 interop goes fine after these changes and please add a wiki article.

  • 13. Re: Major security leak PicketLink and testing on ADFSv2
    Pipo Pipo Newbie

    Hello,

     

    I have tested the latest TRUNK and sadly it does not completely work;

     

    1. When using 'org.picketlink.identity.federation.bindings.tomcat.sp.SPPostSignatureFormAuthenticator' valve: The method verifySignature throws a null pointer error because the keyManager has not been set on the ServiceProviderBaseProcessor:

     

    15:09:23,055 TRACE [SPPostFormAuthenticator] Server Exception:
    java.lang.NullPointerException
        at org.picketlink.identity.federation.web.process.ServiceProviderSAMLResponseProcessor.verifySignature(ServiceProviderSAMLResponseProcessor.java:203)
        at org.picketlink.identity.federation.web.process.ServiceProviderSAMLResponseProcessor.process(ServiceProviderSAMLResponseProcessor.java:131)
        at org.picketlink.identity.federation.bindings.tomcat.sp.SPPostFormAuthenticator.authenticate(SPPostFormAuthenticator.java:196)
        at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:528)
        at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:127)
        at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:102)
        at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:109)
        at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:298)
        at org.apache.coyote.http11.Http11Processor.process(Http11Processor.java:857)
        at org.apache.coyote.http11.Http11Protocol$Http11ConnectionHandler.process(Http11Protocol.java:588)
        at org.apache.tomcat.util.net.JIoEndpoint$Worker.run(JIoEndpoint.java:489)
        at java.lang.Thread.run(Thread.java:619)

     

     

     

     

    2. The CanonicalizationMethod is not being passed down to the XMLSignatureUtil by means of the setCanonicalizationMethodType setter when using the picketlink-idfed.xml configuration:

     

    <PicketLinkSP
    xmlns="urn:picketlink:identity-federation:config:1.0"
    CanonicalizationMethod="http://www.w3.org/2001/10/xml-exc-c14n#"
    ServerEnvironment="tomcat">

     

     

     

    Only when I have changed the default in the source XMLSignatureUtil class to CanonicalizationMethod.EXCLUSIVE and together with using the SPPostFormAuthenticator valve SSO seem to work. So we are very close!

     

    Please let me know if I can be of any help. I promise to write a wiki page when we have a working version of picketlink.

     

    Kind regards,

     

    Edwin

  • 14. Re: Major security leak PicketLink and testing on ADFSv2
    Anil Saldhana Master

    https://jira.jboss.org/browse/PLFED-101

    has been closed.  I also establish the canonicalization method right where we get it from configuration.

     

    Try out trunk and tell me how it behaves.

1 2 3 4 Previous Next