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

    Major security leak PicketLink and testing on ADFSv2

    pipo1000

      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
          pipo1000

          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

            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

              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
                pipo1000

                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

                  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
                    pipo1000

                    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

                      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
                        raycardillo

                        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
                          pipo1000

                          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

                            Not yet. In few days.

                            • 11. Re: Major security leak PicketLink and testing on ADFSv2
                              anil.saldhana

                              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

                                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
                                  pipo1000

                                  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

                                    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