Uploaded image for project: 'Santuario'
  1. Santuario
  2. SANTUARIO-536

SecurePart.getIdToSign is misleading since it's used for both signature and encryption

    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Fixed
    • Java 2.1.5
    • Java 2.2.0
    • Java
    • None

    Description

      OutboundXMLSec.processOutMessage incorrectly uses SecurePart.getIdToSign for encryption.
      There are two "if/else if" branches, one for signature and one for encryption.
      Both branches however use SecurePart.getIdToSign, which is correct for the signature branch, but not for the encryption branch.
      I believe that the encryption branch must use getIdToReference instead.
      "I believe" because the lack of Javadoc and unit tests makes it difficult to "know" the intent, so I have to make an educated guess based on the following observations:

      • SecurePart.getIdToReference is called nowhere, which smells - I suppose it's intended for something.
      • Neither setIdToSign nor setIdToReference are called anywhere, indicating that there are no tests for it, which explains why this feature is broken.

      The impact is that encryption based on matching XML attribute ID does not work as intended.

      The workaround is to use the equivalent for signing, but that means you can't choose a different ID for elements that need both signing and encryption.

      That being said, I'm thinking that either I don't understand the feature "secure elements based on attribute ID" very well, or else it is half-baked, like it was done in a hurry:

      • No unit tests.
      • This bug (assuming it is even a bug).
      • Splitting of encryption ID and sign ID on the SecurePart API is incomplete: the API is split on SecurePart in get/setIdToSign and get/setIdToReference, but not on XMLSecurityProperties: there is only a single XMLSecurityProperties.get/setIdAttributeNS.

      I wonder what is the vision is here? Unify encrypt/sign into a single set of "secure" methods that apply to both, or split encrypt and sign APIs completely? If you ask me, unifying makes most sense, but that's not what most of the code looks like...

      Attachments

        1. SANTUARIO-536.patch
          12 kB
          Peter De Maeyer

        Issue Links

          Activity

            People

              coheigea Colm O hEigeartaigh
              peterdm Peter De Maeyer
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: