HinTak / Font-Validator

Font Validator is a tool for testing fonts prior to release. This testing ensures that fonts meet Microsoft's high quality standards and perform exceptionally well on Microsoft's platform.
Other
146 stars 12 forks source link

DSIGInfo crash #23

Closed jenskutilek closed 7 years ago

jenskutilek commented 7 years ago

I’m trying to add a DSIG table to fonts using the OpenSSL S/MIME signature functions and thought that DSIGInfo could help me debug the signing process.

So far I managed to crash DSIGInfo when trying to display the info from the signed font (attached). It crashes directly after listing the certificates’ (2 in this example) common names.

The message is:

Unhandled Exception: System.IndexOutOfRangeException: Index was outside the bounds of the array.
    at Mono.Security.ASN1..ctor(Byte[] data)
    at Compat.DSIGInfo.Main(String[] args)

My DSIG table may well be faulty, but maybe you want to look into the crash and fix it :)

DSIGTestFont.zip

anthrotype commented 7 years ago

S/MIME if for signing emails, not fonts. It still is a variant of a PKCS#7 SignedData structure, but it is not exactly the same as the ones Microsoft uses for signing OpenType fonts. The latter are very much the same as "Authenticode" signatures., i.e. the ones used for code signing Windows executables or installers.

jenskutilek commented 7 years ago

Thanks for your comments!

I thought maybe it was possible to generate the PKCS7 blob that goes into the DSIG table with OpenSSL. openssl smime can be used to sign a binary file (the font) and output a detached signature into a separate file.

So you are saying it’s not possible to generate a valid signature with just OpenSSL?

anthrotype commented 7 years ago

yeah, sure, if you write a C program that links to the OpenSSL crypto library, but not I'm afraid with the openssl console executable...

anthrotype commented 7 years ago

I mean, something like this, but for fonts: https://sourceforge.net/projects/osslsigncode/

At DaMa we have an internal tool, but I still haven't found the time to clean it up and open source it. :-/

Mostly because I think this is so low priority nowadays.. I just wish the DSIG was just formally deprecated for good. Because, as you know, it already is deprecated in practice, as no one (except maybe fontvalidator) does validate font signatures.

HinTak commented 7 years ago

My notes, etc for a possible signing tool based on mono's authenticode and FontVal. https://github.com/Microsoft/Font-Validator/issues/44

The ASN1 exception means your signature is broken at the very basic level - you can probably use openssl asn1parse to debug it :-).

jenskutilek commented 7 years ago

The ASN1 exception means your signature is broken at the very basic level - you can probably use openssl asn1parse to debug it :-).

Well, openssl asn1parse doesn’t give me any error messages, but if what I see is what a code signing signature should look like, I can’t say, it’s a bit over my head ;)

I think I will take @anthrotype’s advice and not put too much effort into this. Signing on Windows with signtool.exe is working here, that should be good enough.

jenskutilek commented 7 years ago

Here is a detached signature generated by OpenSSL, in case you want to take a look, but it’s not really too important right now.

signature.p7b.zip

anthrotype commented 7 years ago

Signing on Windows with signtool.exe is working here

you mean the old signcode.exe with the 32-bit only mssipotf.dll? https://www.microsoft.com/typography/developers/dsig/dsig.htm

As far as I know, the new signtool.exe that comes with the latest Win SDKs does not accept a -j option any more. That option was used in the old signcode.exe to take the external DLL which runs the checks and defines the authenticated attributes.

Or am I missing something?

jenskutilek commented 7 years ago

I’m using the signtool.exe from the Windows 10 SDK with the 64 bit mssipotf.dll (which is not for download, but you can ask the font people at MS for it) registered in the system. That seems to work, the signatures verify.

anthrotype commented 7 years ago

ah.. I guess they want a fax? 😬

jenskutilek commented 7 years ago

Weirdly enough, no :)

HinTak commented 7 years ago

Did you delete anything between Mono.Security.ASN1..ctor(Byte[] data) and Compat.DSIGInfo.Main(String[] args) ? I don't get anything about ASN1. Under either mono or MS .net, it fails at System.Security.Cryptography.Pkcs.SignedCms.Decode, which is part of the standard libraries. Nothing much I could do or poke except to catch the exception.

The exception under MS .Net (in wine) is

Unhandled Exception: System.Security.Cryptography.CryptographicException: Unknown error "-2146881277".
   at System.Security.Cryptography.Pkcs.PkcsUtils.GetParam(SafeCryptMsgHandle safeCryptMsgHandle, UInt32 paramType, UInt32 index, Byte[]& pvData, UInt32& cbData)
   at System.Security.Cryptography.Pkcs.PkcsUtils.GetContent(SafeCryptMsgHandle safeCryptMsgHandle)
   at System.Security.Cryptography.Pkcs.SignedCms.Decode(Byte[] encodedMessage)
   at Compat.DSIGInfo.Main(String[] args)

while under mono (4.4.2) it is:

System.NullReferenceException: Object reference not set to an instance of an object
  at System.Security.Cryptography.Pkcs.SignedCms.Decode (System.Byte[] encodedMessage) <0x404808a0 + 0x009a0> in <filename unknown>:0 
  at Compat.DSIGInfo.Main (System.String[] args) <0x404626d0 + 0x00773> in <filename unknown>:0 
[ERROR] FATAL UNHANDLED EXCEPTION: System.NullReferenceException: Object reference not set to an instance of an object
  at System.Security.Cryptography.Pkcs.SignedCms.Decode (System.Byte[] encodedMessage) <0x404808a0 + 0x009a0> in <filename unknown>:0 
  at Compat.DSIGInfo.Main (System.String[] args) <0x404626d0 + 0x00773> in <filename unknown>:0 

I built the mac binary with an earlier (4.2.3?) version of mono so it is possible you see a different error, but in general if mono behaves differently from .net, mono is wrong; so it is possible a small bug fix happened after 4,2.3. Anyway, the current behaviour still don't match so I'd probably go and file at Xamarin, but as far as this goes, it is done.

HinTak commented 7 years ago

@anthrotype : actually the 64-bit dll was posted as an unofficial dropbox url by one of the MS guys on the opentype list a month or two ago. The dll itself has a built-date of is 2009 - which seems to be when MS stopped doing a lot of things. :-)

Some people reported difficulty using the 64-bit dll with win 10 - guess it works? @jenskutilek what built-date does yours have?

HinTak commented 7 years ago

Thanks for the detached signature file - I'll take a look nex time when I do something major around there. (some of the TODOs - mostly very old signed ttc's - https://github.com/HinTak/Font-Validator/issues/4 )

anthrotype commented 7 years ago

Thanks for the detached signature file

the S/MIME detached signature is not a valid Authenticode signature, and will never pass chktrust.exe validation. I think FontVal should just reject those.

HinTak commented 7 years ago

It is rejected - the problem was that it choked :-). After the change I just added, it does 'Error: Malformed Signature'. As I said, the net standard crypto library does not like the signature, nothing much I can do about it other than catching the exception, and output a suitable message about not liking the signature.

jenskutilek commented 7 years ago

Sorry for the long response time, I was offline over the holidays ;)

Did you delete anything between Mono.Security.ASN1..ctor(Byte[] data) and Compat.DSIGInfo.Main(String[] args)?

No, it was displayed this way in the command line prompt.

Some people reported difficulty using the 64-bit dll with win 10 - guess it works? @jenskutilek what built-date does yours have?

It seems to be the same version as yours from May 22, 2009. The difficulty may come from that you really have to take care which version of regsrv32 you use to register the dll mssipotf.dll. You need to use the 64 bit version which (counter-intuitively) is located in c:\Windows\System32 and is called regsrv32.exe.

I also imported the certificates into the store (is that what it’s called? You double-click the pfx file and just go with the selected options), and use its hash to select it for signing:

signtool sign /sha1 e419[...]861 /t http://timestamp.verisign.com/scripts/timstamp.dll font.ttf

HinTak commented 7 years ago

I am a little worried that the .net runtime (4.6 or something) on win 10 behaves differently from .net2 and mono and dies at a different place - so let's keep this open until re-testing.

HinTak commented 7 years ago

@jenskutilek : can you please give this a try on your DSIGTestFont.zip above please?

https://sourceforge.net/projects/hp-pxl-jetready/files/Microsoft%20Font%20Validator/misc/FontVal-2.0.0-py-56-gfecd318.zip

For mono and net2 it should do:

SourceSerifPro-Regular_dsig.otf DSIG table: Present
Error: Malformed Signature

If it says instead, Error: Malformed Signature (Unexpected Case 1) , Error: Malformed Signature (Win10), or Error: Malformed Signature (Unexpected Case 2), I'd like to know which.

jenskutilek commented 7 years ago

It prints out information about the signature (Subject, Issuer, Serial Number, Not Before, Not After, Thumbprint), and then at the end:

#Certificates: 2
CN=LucasFonts, O=LucasFonts, L=Berlin, S=Berlin, C=DE
CN=Symantec Class 3 SHA256 Code Signing CA, OU=Symantec Trust Network, O=Symantec Corporation, C=US
Error: Malformed Signature (Win10)
HinTak commented 7 years ago

@jenskutilek : thanks for testing. Your test result confirms my fear: mono and MS .net v2 treats the signature as wholly invalid (in two different ways) quite early, while current .net (4.6.x) thinks the signature is partly valid and managed to read a bit of it, and fails later...

In any case, that justify pushing the little extra code change that gives the "Malformed Signature (Win10)" part out.

HinTak commented 7 years ago

I made a small cosmetic change "(Win10)" -> "(Win10/.net 4.6.x)" as well as some of the surrounding comments before pushing; otherwise the FontVal-2.0.0-py-56-gfecd318.zip posted binary is as if it were built from FontVal-2.0.0-py-59-ga13d20e (the current head of the public embedding-ironpython branch). Closing the issue now.

jenskutilek commented 7 years ago

Thanks!

HinTak commented 7 years ago

Thanks for the re-test and the sample file!