search

mtrojnar / osslsigncode

OpenSSL based Authenticode signing for PE/MSI/Java CAB files
Other
806 stars 131 forks source link

Appx #277

Closed panekmaciej closed 1 year ago

panekmaciej commented 1 year ago

Added support of appx/msix. Still fresh and probably buggy.

Know issues / limitations: -Adds zlib dependency -uses BIO api to write files, which is 32 bit api, so 2GB file limit, as it uses BIO_tell to write LH offsets / CD offset in ZIP files -ZIP implementation is embedded, which limits external dependencies, but increases the code complexity and probably brings some bugs. It could also be separated from appx.c, but this is up to you. -There are grey areas in requirements for the ZIP headers, so the file will verify properly. We have already found few, but there might be more.

See the discussion for the details and more info about the APPX structure / signing: https://github.com/ebourg/jsign/issues/81

ebourg commented 1 year ago

Related to issue https://github.com/mtrojnar/osslsigncode/issues/51

ebourg commented 1 year ago

It looks like the digest algorithm isn't checked, only the one specified in AppxBlockMap.xml can be used. Jsign uses that by default and ignores the one specified on the command line.

Also if I may suggest, maybe a separate source file with all the zip specific code would make the code easier to browse.

mtrojnar commented 1 year ago

It looks like the digest algorithm isn't checked, only the one specified in AppxBlockMap.xml can be used. Jsign uses that by default and ignores the one specified on the command line.

At the very least, a discrepancy should be reported as an error.

Also if I may suggest, maybe a separate source file with all the zip specific code would make the code easier to browse.

Only when we need a compression layer for other file formats. Until then, I recommend leaving it as it is.

panekmaciej commented 1 year ago

It looks like the digest algorithm isn't checked, only the one specified in AppxBlockMap.xml can be used. Jsign uses that by default and ignores the one specified on the command line.

At the very least, a discrepancy should be reported as an error.

Added HashMethod parsing to determine the md type.

ebourg commented 1 year ago

My understanding is that the APPX hash always uses SHA-256, and the algorithm specified in AppxBlockMap.xml is only used in the PKCS#7 signature. That would be interesting to check with a package having a different algorithm in AppxBlockMap.xml.

olszomal commented 1 year ago

It's great that you contribute to the project. Could you please enable CMAKE_BUILD_TYPE=Debug and squash compiler warnings in appx.c We prefer C89/C90 coding style. ISO C90 forbids mixed declarations and code. Please correct it.

panekmaciej commented 1 year ago

Will, do, but will need few days, so probably after the 4th July weekend. Do you expect full C89 compliance (including getting rid of stdbool etc.) or just the variable declarations? Do you want me to do any other code changes?

:sweat_smile:

FBtvrotWQAgIjvT

olszomal commented 1 year ago

It's great if you can do it, but there's nothing urgent around here. It would be kind if you could use the consistent style in this project.

olszomal commented 1 year ago

MakeAppx.exe packager with /h option allows to specify the hash algorithm to use when creating the block map with one of the algorithms: SHA256 (default), SHA384 or SHA512.

AppxBlockMap.xml

+<BlockMap HashMethod="http://www.w3.org/2001/04/xmlenc#sha512" xmlns="http://schemas.microsoft.com/appx/2010/blockmap">

The hashes calculation looks good <3

Hash method is SHA512
Checking Block Map hashes:
Message digest algorithm  : SHA512
Current message digest    : A933C624A881D9C186C87E2C340455060EA53476865866437AB13852BAB653A0E29439F86A49EB78D9FC42E258C0F71DFEA723A9E75C0ECF38D7F586AF0664CB 
Calculated message digest : A933C624A881D9C186C87E2C340455060EA53476865866437AB13852BAB653A0E29439F86A49EB78D9FC42E258C0F71DFEA723A9E75C0ECF38D7F586AF0664CB 

Checking Content Types hashes:
Message digest algorithm  : SHA512
Current message digest    : 4966B087865353B32DA0F17F6A83ED99C32600313AAB27DD5C83AD936DFE320E2B3BD5E7451F6E921E0076EFB7E39C4D11230BBC678131C1AAB89F730CB3EEAA 
Calculated message digest : 4966B087865353B32DA0F17F6A83ED99C32600313AAB27DD5C83AD936DFE320E2B3BD5E7451F6E921E0076EFB7E39C4D11230BBC678131C1AAB89F730CB3EEAA 

Checking Data hashes:
Message digest algorithm  : SHA512
Current message digest    : 180C823FD655263105768B43D5DBC73534C93A146BBF901AACAC7D9831D0ACDCFD4CD3E4F2B0750E3A9CC4D70897FC8944FA851EF0E6C68431484BA07E75D570 
Calculated message digest : 180C823FD655263105768B43D5DBC73534C93A146BBF901AACAC7D9831D0ACDCFD4CD3E4F2B0750E3A9CC4D70897FC8944FA851EF0E6C68431484BA07E75D570 

Checking Central Directory hashes:
Message digest algorithm  : SHA512
Current message digest    : 07023A8AFC390C3BDCD2397F0150797010945CEAD845658628FD5ABC2C2FA8B21A4453EA099CE8CA5275AE065670F474E2E1DAEAE6E8C34D6E4829007B9A6E3B 
Calculated message digest : 07023A8AFC390C3BDCD2397F0150797010945CEAD845658628FD5ABC2C2FA8B21A4453EA099CE8CA5275AE065670F474E2E1DAEAE6E8C34D6E4829007B9A6E3B
panekmaciej commented 1 year ago

Yeah, I wrote the ZIP layer first, then starting adding it to osslsigncode which has totally different code style, so it does not look well.

I have been using this branch in production for the past week, and so far so good, so I will work on styling changes and hope it can be merged so more people can use it :)

ebourg commented 1 year ago

MakeAppx.exe packager with /h option allows to specify the hash algorithm to use when creating the block map with one of the algorithms: SHA256 (default), SHA384 or SHA512.

Thank you for the hint, I used the MSIX Packaging Tool to generate the test file and there was no option to change the algorithm.

I can confirm the APPX hash algorithm is aligned on the block map hash.

olszomal commented 1 year ago

It looks like the digest algorithm isn't checked, only the one specified in AppxBlockMap.xml can be used. Jsign uses that by default and ignores the one specified on the command line.

At the very least, a discrepancy should be reported as an error.

Mixing of different digest algorithms is accepted.

AppxBlockMap.xml

+<BlockMap HashMethod="http://www.w3.org/2001/04/xmlenc#sha512" xmlns="http://schemas.microsoft.com/appx/2010/blockmap">

Let's sign this SHA512 appx file with -h sha256 option and then extract the newly created signature:

$ osslsigncode sign -h sha256 -in unsigned_sha512.appx -out signed_sha512.appx -certs (...)
Hash method is SHA512
Signing as a package
Succeeded

$ ./osslsigncode extract-signature -in signed_sha512.appx -out signed_sha512.der
Hash method is SHA512
Succeeded

Parsed PKCS#7 signature:

$ openssl asn1parse -i -inform DER -in signed_sha512.der
    0:d=0  hl=4 l=3982 cons: SEQUENCE          
    4:d=1  hl=2 l=   9 prim:  OBJECT            :pkcs7-signedData
   15:d=1  hl=4 l=3967 cons:  cont [ 0 ]        
   19:d=2  hl=4 l=3963 cons:   SEQUENCE          
   23:d=3  hl=2 l=   1 prim:    INTEGER           :01
   26:d=3  hl=2 l=  15 cons:    SET               
   28:d=4  hl=2 l=  13 cons:     SEQUENCE          
   30:d=5  hl=2 l=   9 prim:      OBJECT            :sha256
   41:d=5  hl=2 l=   0 prim:      NULL              
   43:d=3  hl=4 l= 374 cons:    SEQUENCE          
   47:d=4  hl=2 l=  10 prim:     OBJECT            :1.3.6.1.4.1.311.2.1.4
   59:d=4  hl=4 l= 358 cons:     cont [ 0 ]        
   63:d=5  hl=4 l= 354 cons:      SEQUENCE          
   67:d=6  hl=2 l=  53 cons:       SEQUENCE          
   69:d=7  hl=2 l=  10 prim:        OBJECT            :1.3.6.1.4.1.311.2.1.30
   81:d=7  hl=2 l=  39 cons:        SEQUENCE          
   83:d=8  hl=2 l=   4 prim:         INTEGER           :01010000
   89:d=8  hl=2 l=  16 prim:         OCTET STRING      [HEX DUMP]:4BDFC50A07CEE24DB76E23C839A09FD1
  107:d=8  hl=2 l=   1 prim:         INTEGER           :00
  110:d=8  hl=2 l=   1 prim:         INTEGER           :00
  113:d=8  hl=2 l=   1 prim:         INTEGER           :00
  116:d=8  hl=2 l=   1 prim:         INTEGER           :00
  119:d=8  hl=2 l=   1 prim:         INTEGER           :00
  122:d=6  hl=4 l= 295 cons:       SEQUENCE          
  126:d=7  hl=2 l=  13 cons:        SEQUENCE          
  128:d=8  hl=2 l=   9 prim:         OBJECT            :sha512
  139:d=8  hl=2 l=   0 prim:         NULL              
  141:d=7  hl=4 l= 276 prim:        OCTET STRING      [HEX DUMP]:415050584158504343D994AC5B76542429C63699D6B827ABEFDDEB457A8A7315501963C9342C67CD99B3AC01966D08248BA53ECF4DB6F3FC9DEBBA6BA2A3F581A03B42A0E996ACE4415843447A98B78EA9FD335B6ED3CB8ACF57323F2E2EE8F27D787F03730604F0FBECAC8851071B96B3395A1F3260299E3E9F8846AF22D8A3683CB85DEDB55A33893A7AE6415843544BBB511EC72C65BC656F668F3E461CD760D6413606A649D09D6C303C372EC709C40E16ECFF0EEF7933D67F8A09C808577F62D206E07034F91B0CAC1F63B5ECF44158424D553C7617C9C9972302406EDCF4A1A43A947CD64DC7C259D92AD6B7FD7DEB4C21350D438301C09B54E75256F197E7C65955C62A70A07013652F3647FB39C1A08D
(...)
 3490:d=5  hl=2 l=  13 cons:      SEQUENCE          
 3492:d=6  hl=2 l=   9 prim:       OBJECT            :sha256
 3503:d=6  hl=2 l=   0 prim:       NULL              
 3505:d=5  hl=3 l= 203 cons:      cont [ 0 ]        
 3508:d=6  hl=2 l=  25 cons:       SEQUENCE          
 3510:d=7  hl=2 l=   9 prim:        OBJECT            :contentType
 3521:d=7  hl=2 l=  12 cons:        SET               
 3523:d=8  hl=2 l=  10 prim:         OBJECT            :1.3.6.1.4.1.311.2.1.4
 3535:d=6  hl=2 l=  28 cons:       SEQUENCE          
 3537:d=7  hl=2 l=   9 prim:        OBJECT            :signingTime
 3548:d=7  hl=2 l=  15 cons:        SET               
 3550:d=8  hl=2 l=  13 prim:         UTCTIME           :190501000000Z
 3565:d=6  hl=2 l=  28 cons:       SEQUENCE          
 3567:d=7  hl=2 l=  10 prim:        OBJECT            :1.3.6.1.4.1.311.2.1.11
 3579:d=7  hl=2 l=  14 cons:        SET               
 3581:d=8  hl=2 l=  12 cons:         SEQUENCE          
 3583:d=9  hl=2 l=  10 prim:          OBJECT            :Microsoft Commercial Code Signing
 3595:d=6  hl=2 l=  47 cons:       SEQUENCE          
 3597:d=7  hl=2 l=   9 prim:        OBJECT            :messageDigest
 3608:d=7  hl=2 l=  34 cons:        SET               
 3610:d=8  hl=2 l=  32 prim:         OCTET STRING      [HEX DUMP]:4139B08892D75702296943EA02B01C927771F85230E811D669064AC7C86E8A5D
 3644:d=6  hl=2 l=  65 cons:       SEQUENCE          
 3646:d=7  hl=2 l=  10 prim:        OBJECT            :1.3.6.1.4.1.311.2.1.12
 3658:d=7  hl=2 l=  51 cons:        SET               
 3660:d=8  hl=2 l=  49 cons:         SEQUENCE          
 3662:d=9  hl=2 l=  14 cons:          cont [ 0 ]        
 3664:d=10 hl=2 l=  12 prim:           cont [ 1 ]        
 3678:d=9  hl=2 l=  31 cons:          cont [ 1 ]        
 3680:d=10 hl=2 l=  29 prim:           cont [ 0 ]        
 3711:d=5  hl=2 l=  13 cons:      SEQUENCE          
 3713:d=6  hl=2 l=   9 prim:       OBJECT            :rsaEncryption
 3724:d=6  hl=2 l=   0 prim:       NULL              
 3726:d=5  hl=4 l= 256 prim:      OCTET STRING      [HEX DUMP]:6F397EB9982ECECFC17FD2DF20CFE659AC957B1C9CF201F9FE523FB8360DCCCDDA4F3796CD0A4A425113F8A97A117C72DF605FC281316A89BADF6719D1C3D581CDC5FD886B8A9A78F82AF70B4FE7B52D697D42A7776BDAABED2C8FBC78BF3AFF1F11874B0067432B4C6849C5BA916202F089735C1417DCB1EBEE1A302572AF56397CA5A7805B8858CA55F5421C71A272F5452E24D71BFA3586C3D705FDA86D856E48F8B9DA9FAE8E6C0E21F79FD5691DEC8BAA53655F951862E3F1231FD58CB4F018D74756604DBC3BBA47DF4ABAFD52BA176BE3A4BB8563B91D35AA7F333C7931A226E2B5D2B78614E160A48E7D56B3372B09A1413621F1BEA0619F2F662B8D

Signtool accepts this signature:

>signtool verify /pa /v signed_sha512.appx
(...)
Signature Index: 0 (Primary Signature)
Hash of file (sha512): 415050584158504343D994AC5B76542429C63699D6B827ABEFDDEB457A8A7315501963C9342C67CD99B3AC01966D08248BA53ECF4DB6F3FC9DEBBA6BA2A3F581A03B42A0E996ACE4415843447A98B78EA9FD335B6ED3CB8ACF57323F2E2EE8F27D787F03730604F0FBECAC8851071B96B3395A1F3260299E3E9F8846AF22D8A3683CB85DEDB55A33893A7AE6415843544BBB511EC72C65BC656F668F3E461CD760D6413606A649D09D6C303C372EC709C40E16ECFF0EEF7933D67F8A09C808577F62D206E07034F91B0CAC1F63B5ECF44158424D553C7617C9C9972302406EDCF4A1A43A947CD64DC7C259D92AD6B7FD7DEB4C21350D438301C09B54E75256F197E7C65955C62A70A07013652F3647FB39C1A08D

Signing Certificate Chain:
    Issued to: Root CA
    Issued by: Root CA
    Expires:   Tue Nov 10 02:00:00 2026
    SHA1 hash: 7DE2629C73A86B21114C234B2DFEBC29A940DC41
(...)

Number of files successfully Verified: 1
Number of warnings: 0
Number of errors: 0
ebourg commented 1 year ago

Mixing of different digest algorithms is accepted

Just checked and that's absolutely correct. Curiously signtool refuses to use a different algorithm when signing (an error code 0x8007000B is returned ), but this isn't enforced when verifying the signature.

mtrojnar commented 1 year ago

@panekmaciej Would you please resolve the received review comments, so that I can merge your PR?

mtrojnar commented 1 year ago

My initial fuzzing results against this PR:

mtrojnar commented 1 year ago

@panekmaciej is no longer responsive, so I took the liberty of creating https://github.com/mtrojnar/osslsigncode/tree/appx to continue working on the appx branch before it can be merged to the master branch.

panekmaciej commented 1 year ago

I am on vacation until the end of this week and had no access to hw token to verify the code functionality. You are welcome to close the PR and finish the work yourself, all what's left is cosmetics.

mtrojnar commented 1 year ago

You are welcome to close the PR and finish the work yourself,

Thank you for your work.

all what's left is cosmetics.

I'm not so sure about it. At least some of the memory corruption issues seem to be exploitable. One of the osslsigncode applications is signature verification of untrusted files, so an exploitable memory corruption results in remote code execution (RCE). This is definitely not cosmetics.

panekmaciej commented 1 year ago

I'm not so sure about it. At least some of the memory corruption issues seem to be exploitable. One of the osslsigncode applications is signature verification of untrusted files, so an exploitable memory corruption results in remote code execution (RCE). This is definitely not cosmetics.

Agreed, but I do not have enough experience in the cybersecurity, so thank you for taking over and working towards merging it into the master branch.

BTW at the time it was the 1st working 3rd party appx signing code (even before JSign :P )