ebourg / jsign

Java implementation of Microsoft Authenticode for signing Windows executables, installers & scripts
https://ebourg.github.io/jsign
Apache License 2.0
250 stars 107 forks source link

DigestAlgorithmIdentifiers now omits null parameters which causes issues on some systems #139

Closed netmackan closed 1 year ago

netmackan commented 1 year ago

It has been reported that the signatures changed between one version and an other and we have tracked this down to be a change introduced in Bouncy Castle version 1.69 (probably) where it stopped including the NULL parameters in SHA2 DigestAlgorithmIdentifier:s in CMSSignedData.

As far as I can tell this change follows RFC 5754 and should be the right thing for CMS in general but I am not sure if it is right for Authenticode (?). At least we have got reports that systems are having problem with this change and also this is a difference as compared to what signtool does which still generates the NULL parameters.

Diff between Jsign and SignTool shows that SignTool still includes this NULL parameters:

--- HelloPE-signtool.exe.peprint    2022-12-16 11:18:33.805000000 +0100
+++ HelloPE-dss.exe.peprint 2022-12-16 11:40:11.468000000 +0100 
 PE Header
   Machine:                    I386
@@ -30,8 +30,8 @@
   Subsystem version:          4.0
   Size of image:              32768
   Size of headers:            512
-  Checksum:                   0x114c6
-  Checksum (computed):        0x114c6
+  Checksum:                   0xe043
+  Checksum (computed):        0xe043
   Subsystem:                  WINDOWS_CUI
   DLL characteristics:        0x0
   Size of stack reserve:      1048576
@@ -43,7 +43,7 @@
 Data Directory
   IMPORT_TABLE                   0x00002018       79 bytes
   RESOURCE_TABLE                 0x00004000      736 bytes
-  CERTIFICATE_TABLE              0x00000c00     2760 bytes
+  CERTIFICATE_TABLE              0x00000c00     2800 bytes
   BASE_RELOCATION_TABLE          0x00006000       12 bytes
   IMPORT_ADDRESS_TABLE           0x00002010        8 bytes
   CLR_RUNTIME_HEADER             0x00002064       72 bytes
@@ -66,7 +66,6 @@
             DER Set
                 DER Sequence
                     ObjectIdentifier(2.16.840.1.101.3.4.2.1)
-                    NULL
             DER Sequence
                 ObjectIdentifier(1.3.6.1.4.1.311.2.1.4)
                 Tagged [0]
@@ -305,7 +304,6 @@
                         Integer(7516069195698284833)
                     DER Sequence
                         ObjectIdentifier(2.16.840.1.101.3.4.2.1)
-                        NULL
                     Tagged [0] IMPLICIT 
                         Sequence
                             DER Sequence
...

What is your opinion on what the right thing would be here, should we switch back to encode the NULL params as that should be safe given it is what signtool produces? If you agree I can submit a PR for that?

Cheers,

ebourg commented 1 year ago

What kind of problems were reported? Invalid signatures?

I'm fine with aligning Jsign with signtool, your PR will be welcome.

netmackan commented 1 year ago

I do not have the exact details yet but the user saw that the new signatures made some systems fail to run the signed binaries. I guess that could be due to signature verification failure or even failure to parse the signature when the expected parameters where absent, not sure.

I have created PR #140.