WSS4J
  1. WSS4J
  2. WSS-300

SubjectKeyIidentifier (SKI) incorrectly calculated for 2048-bit RSA key

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.5.11, 1.6.1
    • Fix Version/s: 1.5.12, 1.6.2
    • Component/s: WSS4J Core
    • Labels:
      None
    • Environment:
      Tomcat 5, Solaris 10, Java 1.4 and Tomcat 6, Win XP, Java 6

      Description

      The crypto function to get the SubjectKeyIdentifier from an X509Certificate has incorrect hard-coded assumptions about the size of the encoded information, meaning the calculation of the SKI from a 2048-bit RSA key is incorrect.

      The method org.apache.ws.security.components.crypto.CryptoBase.getSKIBytesFromCert does not parse the DER encoding of information, but just tries to pick out the piece of the byte array that corresponds to the content of interest. However, that approach fails because the DER encoding is variable length, depending on the size of the data being encoded. e.g. a 1024-bit key in a DER BIT STRING takes 4 bytes header + 140 bytes data to encode, whereas a 2048-bit key takes 5 bytes header + 270 bytes data; the header is one byte longer for the larger key, so the data starts at a different point in the array.

      To fix this, the DER data structures should be properly processed, reading the DER header bytes to determine the length of each data element (that also allows the generalization of handling any X.509 encoded public key, rather than just RSA keys as currently coded).

      Attached is a suggested patch (against WSS4J 1.6.1) that implements this idea: it processes the SubjectPublicKeyInfo and SubjectKeyIdentifier DER-encoded byte arrays according to their ASN.1 definitions from RFC 3280/5280 to pick out the desired data bytes.

      1. broken.xml
        2 kB
        Linton Miller
      2. client.wsdd
        0.9 kB
        Linton Miller
      3. correct.xml
        2 kB
        Linton Miller
      4. CryptoBase.java.patch
        2 kB
        Linton Miller
      5. DERDecoder.java
        7 kB
        Linton Miller
      6. rsa2048.jks
        0.9 kB
        Linton Miller
      7. rsa2048.jks
        0.9 kB
        Linton Miller
      8. test.cypto.properties
        0.3 kB
        Linton Miller
      9. X509SubjectPublicKeyInfo.java
        3 kB
        Linton Miller
      10. X509SubjectPublicKeyInfo.java
        3 kB
        Linton Miller

        Activity

        Linton Miller created issue -
        Linton Miller made changes -
        Field Original Value New Value
        Attachment CryptoBase.java.patch [ 12486498 ]
        Attachment DERDecoder.java [ 12486499 ]
        Attachment X509SubjectPublicKeyInfo.java [ 12486500 ]
        Linton Miller made changes -
        Attachment client.wsdd [ 12487917 ]
        Attachment test.cypto.properties [ 12487918 ]
        Attachment rsa2048.jks [ 12487919 ]
        Attachment broken.xml [ 12487920 ]
        Attachment correct.xml [ 12487921 ]
        Linton Miller made changes -
        Attachment X509SubjectPublicKeyInfo.java [ 12487922 ]
        Linton Miller made changes -
        Comment [ Minor update to include "X509" as an acceptable identifier for public key format (used by Java 1.4). ]
        Colm O hEigeartaigh made changes -
        Fix Version/s 1.5.12 [ 12316045 ]
        Fix Version/s 1.6.2 [ 12316475 ]
        Colm O hEigeartaigh made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Resolution Fixed [ 1 ]
        Linton Miller made changes -
        Comment [ Unfortunately, the new SKITest test case doesn't test this fix. That's because the test certificate has the SKI extension, thus the getSKIBytesFromCert doesn't need to calculate it from the public key (using the new code), but simply reads the extension value. You need to generate test certificates that don't have the X509v3 Subject Key Identifier extension (like my attached example) to test this new code. ]
        Linton Miller made changes -
        Attachment rsa2048.jks [ 12488237 ]
        Colm O hEigeartaigh made changes -
        Status Resolved [ 5 ] Closed [ 6 ]

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development