Uploaded image for project: 'WSS4J'
  1. WSS4J
  2. WSS-40

WSSecurityEngine does not support chained certificates

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • 1.5.6
    • 1.5.10, 1.6
    • None
    • None
    • WSS4J 1.0.0, Axis 1.2.1, Sun JDK 1.4.2

    Description

      My project, which is associated with the Grid, uses limited proxy certificates for digital signature. I.e., the signing application holds a user's permanent certificate, signed by a CA and a proxy certificate signed with the permanent certificate. The application signs a message using the proxy certificate and includes both the proxy and permanent certificates in the message header as a WS-Security direct reference to a BinarySecurityToken. The service has the CA certificate with which the user's permanent certficate was signed. Therefore, to establish trust, the service has to chain back from the proxy to the permanent certificate and then to the CA certificate.

      WSSignEnvelope includes both certificates correctly but WSSecurityEngine fails when checking the chain of trust. WSSecurityEngine..processSecurityHeader() only adds one certificate to the results passed back to WSDoAllReceiver; it ignores the intermediate certificate in the chain.

      Attachments

        1. client_keystore.jks
          3 kB
          Seumas Soltysik
        2. server_keystore.jks
          1 kB
          Seumas Soltysik
        3. wss40.patch
          7 kB
          Seumas Soltysik
        4. wss40.patch.11.09.2010
          12 kB
          Seumas Soltysik
        5. wss-40-test.patch
          4 kB
          Seumas Soltysik
        6. wss40-trunk-revised.patch
          8 kB
          Colm O hEigeartaigh

        Issue Links

          Activity

            WSSecurityEngine.processSecurityHeader() returns a Vector of WSSecurityEngineResult objects. One of those WSSecurityEngineResult objects does indeed only contain the root certificate of the CertificateChain which is the Certificate used to sign the message. However, there is another WSSecurityEngineResult object in the Vector that contains the CertificateChain from the BST. While this chain is not validated against the local keystore it seems to me that this can be easily done in your code by first getting the CertificateChain from the WSSecurityEngineResult and then calling Crypto.validateCertPath(certs). It would look something like this:

            Vector certsResults = new Vector();
            certsResults =
            WSSecurityUtil.fetchAllActionResults(wsResult, WSConstants.BST, certsResults);

            if (!certsResults.isEmpty()) {
            for (int i = 0; i < certsResults.size(); i++)

            { WSSecurityEngineResult result = (WSSecurityEngineResult) certsResults.get(i); X509Certificate[] certs = (X509Certificate[])result.get(WSSecurityEngineResult.TAG_X509_CERTIFICATES); reqData.getSigCrypto().validateCertPath(certs); }

            }

            Would this provide the type of validation you require?

            seumassoltysik Seumas Soltysik added a comment - WSSecurityEngine.processSecurityHeader() returns a Vector of WSSecurityEngineResult objects. One of those WSSecurityEngineResult objects does indeed only contain the root certificate of the CertificateChain which is the Certificate used to sign the message. However, there is another WSSecurityEngineResult object in the Vector that contains the CertificateChain from the BST. While this chain is not validated against the local keystore it seems to me that this can be easily done in your code by first getting the CertificateChain from the WSSecurityEngineResult and then calling Crypto.validateCertPath(certs). It would look something like this: Vector certsResults = new Vector(); certsResults = WSSecurityUtil.fetchAllActionResults(wsResult, WSConstants.BST, certsResults); if (!certsResults.isEmpty()) { for (int i = 0; i < certsResults.size(); i++) { WSSecurityEngineResult result = (WSSecurityEngineResult) certsResults.get(i); X509Certificate[] certs = (X509Certificate[])result.get(WSSecurityEngineResult.TAG_X509_CERTIFICATES); reqData.getSigCrypto().validateCertPath(certs); } } Would this provide the type of validation you require?

            Hi Seumas,

            That looks like it could work. Could you try writing a test to see if your approach would work? For example, see the test "testSignatureDirectReferenceCACert" in this file (only available on trunk):

            http://svn.apache.org/viewvc/webservices/wss4j/trunk/test/wssec/TestWSSecurityWSS40.java?view=markup

            This test sends a certificate chain in the header. You could adapt this test for the 1_5_x-fixes branch.

            Colm.

            coheigea Colm O hEigeartaigh added a comment - Hi Seumas, That looks like it could work. Could you try writing a test to see if your approach would work? For example, see the test "testSignatureDirectReferenceCACert" in this file (only available on trunk): http://svn.apache.org/viewvc/webservices/wss4j/trunk/test/wssec/TestWSSecurityWSS40.java?view=markup This test sends a certificate chain in the header. You could adapt this test for the 1_5_x-fixes branch. Colm.

            Hi Colm,
            Thanks for the response. Here is what I am thinking for a testcase and for wss4j modifications.

            1)Create a Certificate chain where A is signed by B which is signed by a CA.

            2)Create a keystore containing only the CA

            3)Create a message that is signed by Cert A and includes Cert A and Cert B within the BST using the PKIPath ValueType.

            4)Call WSSecEngine.processSecurityHeader() on the signed message.

            This should test the above scenario where the server keystore only has the CA and WSS4J is able to validate the CertChain against the CA in the keystore.

            What do you think?

            I am attaching what I think are the necessary changes to WSS4J which will validate the Cert chain when the ValueType for the BST is set to PKIPath.

            I will put together a test following the above outline on the trunk as a start.

            seumassoltysik Seumas Soltysik added a comment - Hi Colm, Thanks for the response. Here is what I am thinking for a testcase and for wss4j modifications. 1)Create a Certificate chain where A is signed by B which is signed by a CA. 2)Create a keystore containing only the CA 3)Create a message that is signed by Cert A and includes Cert A and Cert B within the BST using the PKIPath ValueType. 4)Call WSSecEngine.processSecurityHeader() on the signed message. This should test the above scenario where the server keystore only has the CA and WSS4J is able to validate the CertChain against the CA in the keystore. What do you think? I am attaching what I think are the necessary changes to WSS4J which will validate the Cert chain when the ValueType for the BST is set to PKIPath. I will put together a test following the above outline on the trunk as a start.

            This patch is an official patch for wss40. To test this patch, I created a certificate chain as follows:

            Root CA -> Intermediary CA -> User Cert

            client_keystore.jks contains a chain consisting of Intermediary CA -> User Cert.

            server_keystore contains the Root CA.

            The test I have included uses the User Cert to sign a message and includes the chain from the client keystore as part of the BST. This message is then validated using the server keystore.

            seumassoltysik Seumas Soltysik added a comment - This patch is an official patch for wss40. To test this patch, I created a certificate chain as follows: Root CA -> Intermediary CA -> User Cert client_keystore.jks contains a chain consisting of Intermediary CA -> User Cert. server_keystore contains the Root CA. The test I have included uses the User Cert to sign a message and includes the chain from the client keystore as part of the BST. This message is then validated using the server keystore.

            Hi Colm,
            Any thoughts on the latest patch that I provided?
            Seumas

            seumassoltysik Seumas Soltysik added a comment - Hi Colm, Any thoughts on the latest patch that I provided? Seumas

            Hi Seumas,

            Sorry I haven't had time yet to look at it, as I wanted to finish the JSR105 port on trunk. This is almost done, so I will take a look at it next week. I want to review trust verification in general, and incorporate your patch as part of this review.

            Just to clarify, are you after a fix for the 1.5.x branch, or only trunk?

            Colm.

            coheigea Colm O hEigeartaigh added a comment - Hi Seumas, Sorry I haven't had time yet to look at it, as I wanted to finish the JSR105 port on trunk. This is almost done, so I will take a look at it next week. I want to review trust verification in general, and incorporate your patch as part of this review. Just to clarify, are you after a fix for the 1.5.x branch, or only trunk? Colm.

            No problem. Let me know your thoughts on the fix when you get a chance. It was a bit of a pain creating the Cert chain and I have only included the keystores containing the certs and not the certs themselves. Let me know if you want me to include the certs as well as part of the patch.

            The fix is primarily for the 1.5.x branch. I was just taking advantage of the existing tests on the trunk to validate my solution. If my solution is acceptable then I can retrofit for 1.5.x branch. Is there a chance of getting it into the next 1.5.x release? If so, when would that be?

            seumassoltysik Seumas Soltysik added a comment - No problem. Let me know your thoughts on the fix when you get a chance. It was a bit of a pain creating the Cert chain and I have only included the keystores containing the certs and not the certs themselves. Let me know if you want me to include the certs as well as part of the patch. The fix is primarily for the 1.5.x branch. I was just taking advantage of the existing tests on the trunk to validate my solution. If my solution is acceptable then I can retrofit for 1.5.x branch. Is there a chance of getting it into the next 1.5.x release? If so, when would that be?

            Hi Colm,
            Any progress on this issue?
            Seumas

            seumassoltysik Seumas Soltysik added a comment - Hi Colm, Any progress on this issue? Seumas

            Not yet sorry....I've been working on the CXF port to use WSS4J 1.6. I'll review it in the next few days and get back to you asap.

            Colm.

            coheigea Colm O hEigeartaigh added a comment - Not yet sorry....I've been working on the CXF port to use WSS4J 1.6. I'll review it in the next few days and get back to you asap. Colm.

            Hi Seumas,

            Please take a look at a revised patch for this issue (for trunk). If you could ok it, and test it against your certs etc., that would be great.

            I did some refactoring of trust verification in SignatureProcessor. Basically, if there is only one certificate it validates it using the old logic, and if there is more than one then it just validates the certificate path directly. You don't need to check whether the type was a PKI chain or not, as the BinarySecurityTokenProcessor takes care of that already.

            If you agree with the basic approach, I'll retrofit it to 1_5_x-fixes. You asked before when this could be released on that branch...I'm thinking of getting 1.5.10 out at the end of this month.

            Colm.

            coheigea Colm O hEigeartaigh added a comment - Hi Seumas, Please take a look at a revised patch for this issue (for trunk). If you could ok it, and test it against your certs etc., that would be great. I did some refactoring of trust verification in SignatureProcessor. Basically, if there is only one certificate it validates it using the old logic, and if there is more than one then it just validates the certificate path directly. You don't need to check whether the type was a PKI chain or not, as the BinarySecurityTokenProcessor takes care of that already. If you agree with the basic approach, I'll retrofit it to 1_5_x-fixes. You asked before when this could be released on that branch...I'm thinking of getting 1.5.10 out at the end of this month. Colm.

            Colm,
            Thanks for the patch. I have taken your patch and added additional files to test this feature and created a new patch wss40.patch.11.09.2010. I originally created a patch containing the binary keystore files but when I tried applying the patch to test it, it balked at applying the binary data. Rather than fight it I have simply attached the binary keystores separately. These files go in the trunk/keys directory.
            The patch looks fine and my test passes.

            Also getting 1.5.10 by the end of the month sounds fine.

            seumassoltysik Seumas Soltysik added a comment - Colm, Thanks for the patch. I have taken your patch and added additional files to test this feature and created a new patch wss40.patch.11.09.2010. I originally created a patch containing the binary keystore files but when I tried applying the patch to test it, it balked at applying the binary data. Rather than fight it I have simply attached the binary keystores separately. These files go in the trunk/keys directory. The patch looks fine and my test passes. Also getting 1.5.10 by the end of the month sounds fine.
            coheigea Colm O hEigeartaigh added a comment - - edited

            Hi Seumus,

            I've committed your test-case and a patch for this issue. I changed a few things around - namely I added back in the part about only checking the cert chain if it was a PKIPath. (edit: I also added in functionality to send a cert chain via configuration).

            I have a port of this almost done on 1_5_x-fixes. Things are done slightly differently here, as trust verification does not take place in the SignatureProcessor, as it does on trunk, but is done in WSHandler.verifyTrust. I added an overloaded version of this method to WSHandler. The client code (i.e. WSS4JInInterceptor in CXF) must call the right overloaded method depending on whether there are multiple certificates or not. The SignatureProcessor only saves multiple certificates if the inbound BinarySecurityToken uses PKIPath.

            One I get this done I'll add a patch to CXF. I'm not going to touch WSS4JHandler (which CXF doesn't use), or the inbound Axis handler. Other third party code which uses WSS4JHandler, or WSHandler in general, must implement their own logic to call the right verifyTrust method, if they want to take advantage of this functionality.

            Colm.

            coheigea Colm O hEigeartaigh added a comment - - edited Hi Seumus, I've committed your test-case and a patch for this issue. I changed a few things around - namely I added back in the part about only checking the cert chain if it was a PKIPath. (edit: I also added in functionality to send a cert chain via configuration). I have a port of this almost done on 1_5_x-fixes. Things are done slightly differently here, as trust verification does not take place in the SignatureProcessor, as it does on trunk, but is done in WSHandler.verifyTrust. I added an overloaded version of this method to WSHandler. The client code (i.e. WSS4JInInterceptor in CXF) must call the right overloaded method depending on whether there are multiple certificates or not. The SignatureProcessor only saves multiple certificates if the inbound BinarySecurityToken uses PKIPath. One I get this done I'll add a patch to CXF. I'm not going to touch WSS4JHandler (which CXF doesn't use), or the inbound Axis handler. Other third party code which uses WSS4JHandler, or WSHandler in general, must implement their own logic to call the right verifyTrust method, if they want to take advantage of this functionality. Colm.

            I committed a port to 1_5_x-fixes. I'm going to mark this issue as fixed. I'll commit a fix to CXF next week.

            Colm.

            coheigea Colm O hEigeartaigh added a comment - I committed a port to 1_5_x-fixes. I'm going to mark this issue as fixed. I'll commit a fix to CXF next week. Colm.

            People

              coheigea Colm O hEigeartaigh
              guyrixon Guy Rixon
              Votes:
              1 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: