Santuario
  1. Santuario
  2. SANTUARIO-305

No way to register internal key resolvers in DECRYPT_MODE

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: Java 1.5.1
    • Fix Version/s: Java 1.5.2
    • Component/s: None
    • Security Level: Public (Public issues, viewable by everyone)
    • Labels:
      None

      Description

      There is no way to register internal key resolvers in DECRYPT_MODE. The internal resolvers are usually registered on a KeyInfo. When we call XMLCipher.doFinal(Document, Element) to decrypt, it creates a new EncryptedData object on the fly and uses it immediately (See XMLCipher.decryptToByteArray). There is no chance to modify the KeyInfo inside that EncryptedData before it is used. It is possible to call XMLCipher.loadEncryptedData() separately, but there is little we can do with that EncryptedData afterwards. Using the static resolvers is not thread-safe in general. By that I mean, you cannot configure the static resolver per thread unless you use thread local storage.

      Possible solutions:
      1. Let the XMLCipher maintain a list of internal key resolvers directly.
      2. Pass internal resolvers when calling doFinal()
      3. Add a method XMLCipher.decryptData(EncryptedData) similar to decryptKey(EncryptedKey),
      So we could call XMLCipher.loadEncryptedData(Element), modify the KeyInfo inside the EncryptedData, and call XMLCipher.decryptData().

      1. santuario-305.zip
        23 kB
        Clement Pellerin
      2. santuario-305.diff
        13 kB
        Clement Pellerin

        Issue Links

          Activity

          Clement Pellerin created issue -
          Colm O hEigeartaigh made changes -
          Field Original Value New Value
          Affects Version/s Java 1.5.1 [ 12319507 ]
          Affects Version/s C++ 1.5.1 [ 12315940 ]
          Hide
          Colm O hEigeartaigh added a comment -

          Hi Clement,

          Could you submit a patch for whatever solution you think is best and I'll review it? It seems like a reasonable thing to be able to do - so long as the solution doesn't break backwards compatibility.

          Colm.

          Show
          Colm O hEigeartaigh added a comment - Hi Clement, Could you submit a patch for whatever solution you think is best and I'll review it? It seems like a reasonable thing to be able to do - so long as the solution doesn't break backwards compatibility. Colm.
          Hide
          Clement Pellerin added a comment -

          I wish there would be more interest in design discussions. Anything to avoid repeating my previous disaster (SANTUARIO-227).
          For SANTUARIO-305, I like option 3 and this is what I plan to submit as a patch within a couple days.

          Show
          Clement Pellerin added a comment - I wish there would be more interest in design discussions. Anything to avoid repeating my previous disaster ( SANTUARIO-227 ). For SANTUARIO-305 , I like option 3 and this is what I plan to submit as a patch within a couple days.
          Hide
          Clement Pellerin added a comment -

          I investigated option 3 but the result did not really please me.

          I considered the method Node decryptData(Document context, EncryptedData ed). This leaves the working document intact a la decryptKey() but leaves the caller with the problem of parenting the result. Reparenting requires more code than replaceChild() which might surprise some application developers and thus encourage bugs.

          I considered the method Document decryptData(Element parent, EncryptedData ed). This modifies the working document which breaks symmetry with decryptKey(). This behavior is usually associated with doFinal() which breaks another symmetry.

          I considered allowing null as the Element in doFinal(Document, Element). The idea was to use the EncryptedData member of XMLCipher in DECRYPT mode if the Element is null. That member is already set by loadEncryptedData(). This is workable but the feature is hidden by the choice of API.

          Since I could not find an API that was clearly better than the others, I decided to implement option 1 (Let the XMLCipher maintain a list of internal key resolvers directly). The resolvers are only used in DECRYPT and UNWRAP mode. In other modes, the KeyInfo is created explicitly by the caller and therefore we let the caller populate it. The KeyResolvers are passed to the EncryptedKeyResolver when the KEK is still unknown. The EncryptedKeyResolver passes the KeyResolvers to the inner XMLCipher to help resolve the KEK.

          The API works with one KeyResolver at a time as before. This avoids the problem of list ownership and whether the list is live in the object or just a copy.

          Show
          Clement Pellerin added a comment - I investigated option 3 but the result did not really please me. I considered the method Node decryptData(Document context, EncryptedData ed). This leaves the working document intact a la decryptKey() but leaves the caller with the problem of parenting the result. Reparenting requires more code than replaceChild() which might surprise some application developers and thus encourage bugs. I considered the method Document decryptData(Element parent, EncryptedData ed). This modifies the working document which breaks symmetry with decryptKey(). This behavior is usually associated with doFinal() which breaks another symmetry. I considered allowing null as the Element in doFinal(Document, Element). The idea was to use the EncryptedData member of XMLCipher in DECRYPT mode if the Element is null. That member is already set by loadEncryptedData(). This is workable but the feature is hidden by the choice of API. Since I could not find an API that was clearly better than the others, I decided to implement option 1 (Let the XMLCipher maintain a list of internal key resolvers directly). The resolvers are only used in DECRYPT and UNWRAP mode. In other modes, the KeyInfo is created explicitly by the caller and therefore we let the caller populate it. The KeyResolvers are passed to the EncryptedKeyResolver when the KEK is still unknown. The EncryptedKeyResolver passes the KeyResolvers to the inner XMLCipher to help resolve the KEK. The API works with one KeyResolver at a time as before. This avoids the problem of list ownership and whether the list is live in the object or just a copy.
          Hide
          Clement Pellerin added a comment -

          Patch submitted.

          Show
          Clement Pellerin added a comment - Patch submitted.
          Clement Pellerin made changes -
          Attachment santuario-305.zip [ 12519971 ]
          Colm O hEigeartaigh made changes -
          Link This issue supercedes SANTUARIO-227 [ SANTUARIO-227 ]
          Hide
          Colm O hEigeartaigh added a comment -

          Hi Clement,

          Could you format your patches using "diff" so that I can apply them via "patch"?

          Colm.

          Show
          Colm O hEigeartaigh added a comment - Hi Clement, Could you format your patches using "diff" so that I can apply them via "patch"? Colm.
          Hide
          Clement Pellerin added a comment -

          The files were already merged for the trunk. From now on, I'll submit a diff.

          Show
          Clement Pellerin added a comment - The files were already merged for the trunk. From now on, I'll submit a diff.
          Clement Pellerin made changes -
          Attachment santuario-305.diff [ 12520265 ]
          Hide
          Clement Pellerin added a comment -

          Patch in diff format submitted.

          Show
          Clement Pellerin added a comment - Patch in diff format submitted.
          Colm O hEigeartaigh made changes -
          Fix Version/s Java 1.5.2 [ 12320048 ]
          Colm O hEigeartaigh made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Colm O hEigeartaigh made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Colm O hEigeartaigh
              Reporter:
              Clement Pellerin
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development