Skip to content
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

IsSignatureValid doesn't actually verify the signature #59

Closed
FlorianHockmann opened this issue Dec 18, 2018 · 6 comments
Closed

IsSignatureValid doesn't actually verify the signature #59

FlorianHockmann opened this issue Dec 18, 2018 · 6 comments
Labels

Comments

@FlorianHockmann
Copy link

The check for PeFile.IsSignatureValid is implemented in the private method AuthenticodeInfo.CheckSignature which basically does this:

var hash = ComputeAuthenticodeHash();
return SignedHash.SequenceEqual(hash);

This shows that the check only compares whether the hash from the signature matches the computed hash of the PE file, but the signature itself isn't checked at all. It should be verified with the certificate.

FlorianHockmann added a commit to FlorianHockmann/PeNet that referenced this issue Dec 18, 2018
The signature is checked using the System.Security.Cryptography.Pkcs
package. This package unfortunately doesn't support .NET Framework 4.0
which is why I had to drop support for that target framework.

A test case ensures that the verification actually works.
It uses a manipulated binary where the signature shouldn't be
valid any more.
I simply changed 1 bit in the binary and then exchanged the included
hash so it matches the changed binary. The previous verification
reported the signature as valid as it only compared the hashes.

Unfortunately, two other test cases are failing now where the signature
is reported as invalid, but it should be valid.

Fixes secana#59
@secana
Copy link
Owner

secana commented Dec 19, 2018

Initial idea for a fix by @FlorianHockmann
FlorianHockmann@e73a1de

@secana
Copy link
Owner

secana commented Dec 21, 2018

It seems that we are running into a known problem: https://github.com/dotnet/corefx/issues/31073

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."

@secana
Copy link
Owner

secana commented Dec 21, 2018

Not sure how to handle this. Technically .Net Core is right, since DigiCert doesn't comply to the CMS standard. On the other hand, the "real world" standard would be to accept the signature, like .Net Framework does it.

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.

@secana
Copy link
Owner

secana commented Dec 21, 2018

I've requested help in the CoreFX issue: dotnet/corefx#31073

@secana
Copy link
Owner

secana commented Dec 21, 2018

Opened an issue in CoreFX: https://github.com/dotnet/corefx/issues/34202

secana added a commit that referenced this issue Jan 14, 2019
@secana secana added the bug label Jan 14, 2019
@secana
Copy link
Owner

secana commented Jan 14, 2019

Fixed in 0.8.2

@secana secana closed this as completed Jan 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants