You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@santuario.apache.org by "Peter De Maeyer (Jira)" <ji...@apache.org> on 2020/04/15 20:42:00 UTC

[jira] [Updated] (SANTUARIO-536) SecurePart.getIdToSign is misleading since it's used for both signature and encryption

     [ https://issues.apache.org/jira/browse/SANTUARIO-536?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Peter De Maeyer updated SANTUARIO-536:
--------------------------------------
    Summary: SecurePart.getIdToSign is misleading since it's used for both signature and encryption  (was: SecurePart.getIdToSign has misleading name since it's used for both signature and encryption)

> SecurePart.getIdToSign is misleading since it's used for both signature and encryption
> --------------------------------------------------------------------------------------
>
>                 Key: SANTUARIO-536
>                 URL: https://issues.apache.org/jira/browse/SANTUARIO-536
>             Project: Santuario
>          Issue Type: Bug
>          Components: Java
>    Affects Versions: Java 2.1.5
>            Reporter: Peter De Maeyer
>            Assignee: Colm O hEigeartaigh
>            Priority: Major
>         Attachments: SANTUARIO-536.patch
>
>
> {{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...



--
This message was sent by Atlassian Jira
(v8.3.4#803005)