New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Different behavior on .Net Framework and Core in SignedCms.CheckSignature #34202

Open
secana opened this Issue Dec 21, 2018 · 14 comments

Comments

Projects
None yet
6 participants
@secana
Copy link

secana commented Dec 21, 2018

I want to verify a signature contained in a CMS. I used the code below, which works fine on .Net Framework but not in .Net Core.

I've verified that this is the case with the following code:

using System;
using System.IO;
using System.Security.Cryptography;
using System.Security.Cryptography.Pkcs;

namespace Framework
{
    class Program
    {
        static void Main(string[] args)
        {
            var rawCerts = File.ReadAllBytes("cert.dump");
            var signedCms = new SignedCms();
            signedCms.Decode(rawCerts);

            try
            {
                signedCms.CheckSignature(true);
                Console.WriteLine("Valid siganture.");
            }
            catch(CryptographicException e)
            {
                Console.Error.WriteLine(e.Message);
            }

            Console.ReadKey();
        }
    }
}

On .Net Core: "Invalid Siganture."
On .Net Framework: "Valid Signature."

I've tried to update System.Security.Cryptography.Pkcs to a preview version (4.6.0-preview.18571.3), but now the signedCms.Decode(rawCerts) fails with an CryptographicException: ASN1 corrupted data. exception.

I've attached the CMS I had the issues with. It's from a Firefox executable from 4 month ago.

cert.zip

@vcsjones

This comment has been minimized.

Copy link
Collaborator

vcsjones commented Dec 21, 2018

A quick look at this, it looks like netcoreapp doesn't like the timestamp. To reproduce it a little bit differently..

    try
    {
        foreach(var info in signedCms.SignerInfos)
        {
            Console.WriteLine("Checking signature.");
            info.CheckSignature(true);

            Console.WriteLine("Checking counter signers");
            foreach(var counterSigner in info.CounterSignerInfos)
            {
                counterSigner.CheckSignature(true);
            }
        }
    }
    catch (CryptographicException e)
    {
        Console.Error.WriteLine(e.Message);
    }

The Authenticode signature is OK - it bombs on the timestamp. Looking into why.

@vcsjones

This comment has been minimized.

Copy link
Collaborator

vcsjones commented Dec 21, 2018

/cc @bartonjs

@onovotny

This comment has been minimized.

Copy link
Member

onovotny commented Dec 27, 2018

/cc @dtivel

@vcsjones

This comment has been minimized.

Copy link
Collaborator

vcsjones commented Dec 30, 2018

This appears to be an interesting case of CAPI1 and CNG not agreeing on if a signature is valid. CNG believes the signature is invalid, while CAPI thinks it is. Doing a bit of reverse engineering of CAPI/CNG, the hash of the CMS is "aLT5JjQxJd0mUBNowZkmcRmi3oE=" which the .NET Core implementation agrees on. So this is not a canonicalization issue like I previously suspected.

SignedCms works on .NET Framework because SignedCms there uses Win32. Win32 seems to use CAPI1 when validating this signature (though it will use CNG for others). It's not clear to me right now how it decides to use CAPI vs. CNG, like the digest algorithm. That decision is made in crypt32!ICM_VerifySignature.

Aside: Interestingly when Win32 uses CAPI to perform the validation, it seems to delegate back to BCryptVerifySignature, but looks like it is passing something undocumented into the padding struct?

In .NET Core, there is no CAPI. Everything is CNG, so the signature check fails.

On .NET Framework 4.7.2, you can more easily reproduce this and take SignedCms out of the equation like this:

static void Main(string[] args)
{
    // Timestamping certificate
    var certStr = "MIIEozCCA4ugAwIBAgIQDs/0OMj+vzVuBNhqmBsaUDANBgkqhkiG9w0BAQUFADBeMQswCQYDVQQGEwJVUzEdMBsGA1" +
        "UEChMUU3ltYW50ZWMgQ29ycG9yYXRpb24xMDAuBgNVBAMTJ1N5bWFudGVjIFRpbWUgU3RhbXBpbmcgU2VydmljZXMgQ0EgLSBHMj" +
        "AeFw0xMjEwMTgwMDAwMDBaFw0yMDEyMjkyMzU5NTlaMGIxCzAJBgNVBAYTAlVTMR0wGwYDVQQKExRTeW1hbnRlYyBDb3Jwb3JhdG" +
        "lvbjE0MDIGA1UEAxMrU3ltYW50ZWMgVGltZSBTdGFtcGluZyBTZXJ2aWNlcyBTaWduZXIgLSBHNDCCASIwDQYJKoZIhvcNAQEBBQ" +
        "ADggEPADCCAQoCggEBAKJjCzlEuLsjp0RJuw7/ofBhClOTsJjbrSwPSsVu/4Y8U1UPFc4EPyv9qZaW2b5heQtbyUyGduXgQ0sile" +
        "7CK0PBn9hotI5AT+6FOLkRxSPyZFjwFTJvTlehroikAtcqHs1L4d1j1ReJMluwXplaqJ0oUA4X7pbbYTtFUR3PElYLkkf8q672Zj" +
        "1HrHBy55LnX80QucSDZJQZvSWA4ejSIqXQugJ6oXeTW2XD7hd0vEGGKtwITIySjJEtnndEH2jWqHR32w5bMotWizO92WPISZ06xc" +
        "XqMwvS8aMb9Iu+2bNXizveBKd6IrIkri7HcMW+ToMmCPsLvalPmQjhEChyqs0CAwEAAaOCAVcwggFTMAwGA1UdEwEB/wQCMAAwFg" +
        "YDVR0lAQH/BAwwCgYIKwYBBQUHAwgwDgYDVR0PAQH/BAQDAgeAMHMGCCsGAQUFBwEBBGcwZTAqBggrBgEFBQcwAYYeaHR0cDovL3" +
        "RzLW9jc3Aud3Muc3ltYW50ZWMuY29tMDcGCCsGAQUFBzAChitodHRwOi8vdHMtYWlhLndzLnN5bWFudGVjLmNvbS90c3MtY2EtZz" +
        "IuY2VyMDwGA1UdHwQ1MDMwMaAvoC2GK2h0dHA6Ly90cy1jcmwud3Muc3ltYW50ZWMuY29tL3Rzcy1jYS1nMi5jcmwwKAYDVR0RBC" +
        "EwH6QdMBsxGTAXBgNVBAMTEFRpbWVTdGFtcC0yMDQ4LTIwHQYDVR0OBBYEFEbGaaMOShQe1UzaUmMXP142vA3mMB8GA1UdIwQYMB" +
        "aAFF+a9W5czMx0mtTdfe8/2+xMgC7dMA0GCSqGSIb3DQEBBQUAA4IBAQB4O7SRKgBM8I9iMDd4o4QnB28Yst4l3KDUlAOqhk4ln5" +
        "pAAxzdzuN5yyFoBtq2MrRtv/QsJmMz5ElkbQ3mw2cO9wWkNWx8iRbG6bLfsundIMZxD82VdNy2XN69Nx9DeOZ4tc0oBCCjqvFLxI" +
        "gpkQ6A0RH83Vx2bk9eDkVGQW4NsOo4mrE62glxEPwcebSAe6xp9P2ctgwWK/F/Wwk9m1viFsoTgW0ALjgNqCmPLOGy9FqpAa8VnC" +
        "wvSRvbIrvD/niUUcOGsYKIXfA9tFGheTMrLnu53CAJE3Hrahlbz+ilMFcsiUk/uc9/yb8+ImhjU5q9aXSsxR08f5Lgw7wc2AR1";
    var certBytes = Convert.FromBase64String(certStr);
    var cert = new X509Certificate2(certBytes);

    // Hash of the CMS's content.
    var hashStr = "aLT5JjQxJd0mUBNowZkmcRmi3oE=";
    var hashBytes = Convert.FromBase64String(hashStr);

    // Signature in the CMS.
    var sigStr = "ats55WOzJd5YysPxNpwLNrfWafm6pmgUjCRS0yWl863JR0TeBtgPVsot+w/pmeKdiuh/+5qZlvEsSuTArk0pRziWUS9t" +
        "jriIvRoKcLwjOGdiIgEjceW7leprjTFiv/DEuUbWZ/xM5h/WXfeprTrxv6L5Zt62juyPgY0eOhInavyukp/Dh8O6jQS4jw9haJqWL" +
        "IAsMkDenbmb4uRFLpFHXEeknQJXWfd1XV8ygnVd5XjJGWFGBp2lHdYySJrbCSmBFC7wJ+k3E3Tspc1naxn2iPDCi6h/L3ZaPgxHXe" +
        "iCUCdAzidBRaDPqi/TrTy/c/+T43hJ2al4IoGa5eKU6UCr8Q==";
    var sigBytes = Convert.FromBase64String(sigStr);
    RSA rsaCapi = (RSACryptoServiceProvider)cert.PublicKey.Key;
    RSA rsaCng = (RSACng)cert.GetRSAPublicKey();
    var resultCapi = rsaCapi.VerifyHash(hashBytes, sigBytes, HashAlgorithmName.SHA1, RSASignaturePadding.Pkcs1);
    var resultCng = rsaCng.VerifyHash(hashBytes, sigBytes, HashAlgorithmName.SHA1, RSASignaturePadding.Pkcs1);
    Console.WriteLine($"CAPI: {resultCapi}.");
    Console.WriteLine($"CNG: {resultCng}.");
}

That gives back:

CAPI: True.
CNG: False.


@bartonjs does anything jump out at you that might cause this?

Edit: macOS's CommonCrypto also thinks the signature is invalid.

@bartonjs

This comment has been minimized.

Copy link
Member

bartonjs commented Dec 30, 2018

Ah, CAPI is wrong. That signature is invalid.

Taking the ModPow(signature, e, n) =>

0001FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFF0068B4F926343125DD26501368C199267119A2DE81

with a hash of 68B4F926343125DD26501368C199267119A2DE81.

The correct PKCS1 signature block would be

0001FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF00302130
0906052B0E03021A0500041468B4F926343125DD26501368C199267119A2DE81

That is, the signature is missing the SEQUENCE(OID(SHA-1), OCTETSTRING) structure.

It's possible that CAPI is allowing for an older form of PKCS signature that neither CNG nor CommonCrypto permit; or CAPI historically just checked 00 01 FF and "the last n bytes". But the signature's definitely not valid under RSASSA-PKCS1-v1_5.

@onovotny

This comment has been minimized.

Copy link
Member

onovotny commented Dec 30, 2018

What created the original signature (incorrectly)?

@jariq

This comment has been minimized.

Copy link

jariq commented Dec 30, 2018

That is, the signature is missing the SEQUENCE(OID(SHA-1), OCTETSTRING) structure.

Just for the sake of completeness, that structure is called DigestInfo and is defined in RFC3447 as follows:

DigestInfo ::= SEQUENCE {
	digestAlgorithm AlgorithmIdentifier,
	digest OCTET STRING
}
@vcsjones

This comment has been minimized.

Copy link
Collaborator

vcsjones commented Dec 30, 2018

OK, so the docs for BCRYPT_PKCS1_PADDING_INFO say a little more about this.

pszAlgId: When verifying a signature, the verification fails if the OID that corresponds to this member is not the same as the OID in the signature. If there is no OID in the signature, then verification fails unless this member is NULL.

So it appears that CNG does allow for this, but you need to explicitly pass in null as the pszAlgId if I understand correctly. .NET Core appears to always require passing in a hash algorithm to BCRYPT_PKCS1_PADDING_INFO.

@vcsjones

This comment has been minimized.

Copy link
Collaborator

vcsjones commented Dec 31, 2018

@onovotny

What created the original signature (incorrectly)?

I believe this came from Symantec's Legacy Timestamp Authority, http://timestamp.verisign.com/scripts/timstamp.dll. I just reproduced this again by submitting a timestamp to there.

This timestamp uses "legacy" Authenticode timestamps, not RFC3161. To answer a question you may be thinking, I don't think this impact Nuget Packages because Nuget packages do not support legacy timestamps, only RFC3161 timestamps. Symantec/DigiCert's timestamps appear to return correctly encoded signatures when using their RFC3161 TSAs.

@vcsjones

This comment has been minimized.

Copy link
Collaborator

vcsjones commented Dec 31, 2018

For what it's worth, I'm pretty sure this Symantec TSA is ancient. I don't know how accurate Google's date mechanism is, but the earliest reference to this TSA I could find is 1996. That predates the standardization of RSASSA-PKCS1-v1_5 into an RFC by a few years.

@bartonjs

This comment has been minimized.

Copy link
Member

bartonjs commented Jan 3, 2019

FWIW: PKCS#1 v1.5 was published in 1993 (ftp://ftp.rsasecurity.com/pub/pkcs/ascii/pkcs-1.asc).

If the only case where there's a behavioral difference is when using a signature generated using a pre-1993 format, I'm inclined to say Won't Fix.

pre-1993 format? My first 'network' was a serial cable to play heads-up Rise of the Triad, published 1994... wow. I don't know why these facts make me feel old at the moment.

@vcsjones

This comment has been minimized.

Copy link
Collaborator

vcsjones commented Jan 3, 2019

I'm inclined to say Won't Fix.

I don't think it is fixable without a significant amount of work. I looked at CommonCrypto and OpenSSL and I don't see a supported or easy way to get them to recognize the signature. It's unfortunate that this is a rather popular TSA though.

@secana

This comment has been minimized.

Copy link

secana commented Jan 3, 2019

@bartonjs I understand the "won't fix" but the signature I attached is from an up-to-date Firefox. So the problem exists with widely used and current software . Checking if there signature is valid, is not possible with the current state.

If a fix is not feasible in a short time, is there any way to code around it to get the same behavior on .Net Framework and Core?

@vcsjones

This comment has been minimized.

Copy link
Collaborator

vcsjones commented Jan 6, 2019

If a fix is not feasible in a short time, is there any way to code around it to get the same behavior on .Net Framework and Core?

Nothing trivial comes to my mind right now. CNG (and CommonCrypto and OpenSSL) will treat the RSA signature as invalid. You would have to use an API that uses CAPI to validate the signature, or find an RSA library that is willing to treat this signature is valid. Even then you'd have to get SignedCms to use it, which probably means having a local copy of the class as of .NET Core 2.2.

For .NET Core 3.0, you can replace the RSA sign/verify functions in SignedCms as of #26406.

With that, you could write a wrapper around CAPI, or use another RSA library that tolerates the malformed signature.

It looks like we only implemented key extensibility for signing, not verifying.


If all you are trying to do is verify an Authenticode signature for a file, perhaps WinVerifyTrustEx is a better solution, albeit much more high level and not xplat. It handles a lot of the nuances of validating an Authenticode signature, plus it works on all Authenticode-signable files, like MSI, APPX, PEs, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment