dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.04k stars 4.68k forks source link

new X509Certificate2(byte[]) should return the signing cert for PKCS#7 on Unix #15073

Closed bartonjs closed 1 week ago

bartonjs commented 9 years ago

The Windows code supports reading a PKCS#7 signed or PKCS#7 signed-and-enveloped structure; but in single certificate mode it doesn't return certs[0], it returns the certificate which signed the structure.

My test files (produced by Windows (certmgr.msc and X509Certificate2Collection::Export)) aren't signed, so Windows emits an exception (new CryptographicException(ErrorCode.CRYPT_E_SIGNER_NOT_FOUND)).

Without a sample to see what's going on here, it's hard to make compatible behavior. So, for now, the Unix implementation will just throw, even if it could have worked.

danmoseley commented 7 years ago

Marking Future explicitly because we have decided thisi s not necessary for 2.0.

bartonjs commented 6 years ago

Moving to Future, it's just getting a bit late to plumb this down (and a lot of tests need to be defined to ensure that we're behaving the same way as in our Windows black-box implementation... zero signers, multiple signers, igner-cert-not-included, hybridized values, etc)

The workaround for caller code which knows it's getting a PKCS7 is to just load it into the SignedCms class. This is the naive approach (only works for one signer, when the signer cert is present)

SignedCms cms = new SignedCms();
cms.Decode(bytes);
SignerInfoCollection signers = cms.SignerInfos;

if (signers.Count != 1)
{
    throw new CryptographicException("Wrong number of signers.");
}

return signers[0].Certificate ?? throw new CryptograpicException("Signer cert was not present");
filipnavara commented 6 years ago

I was playing with decoding Authenticode, which is also supposed to be supported. Turns out most of the plumbing code is already present in CoreFX (no error handling, just PoC):

        var peReader = new PEReader(new MemoryStream(smallspcexe));
        // TODO: Check for non-PE files
        var certDirectory = peReader.PEHeaders.PEHeader.CertificateTableDirectory;
        // TODO: Check for no certificate table
        var certData = peReader.GetEntireImage().GetContent(certDirectory.RelativeVirtualAddress, certDirectory.Size);
        // TODO: Handle different certificate types (X.509, PKCS#7) and read the length (ie. multiple certificates)
        var certData2 = new byte[certData.Length - 8];
        certData.CopyTo(8, certData2, 0, certData2.Length);
        var certType = X509Certificate2.GetCertContentType(certData2);
        var x509 = new X509Certificate2(certData2);

and it boils down to reading PKCS#7 certificates. The last constructor fails with an exception.

kygagner commented 6 years ago

We need to be able to verify Authenticode signatures on managed DLLs in Linux for licensing enforcement.

Kalyxt commented 5 years ago

Hello, my project based on linux relying on reading cert from PKCS#7 signed file, which is not working atm. Is this going to be fixed in upcoming .NET Core versions (3.0) or what is the situation around this? I would greatly appreciate any info about this issue and its future.

secana commented 5 years ago

@Kalyxt Maybe https://github.com/secana/PeNet can do what you want (I'm the author). The PeNet library is able to extract the cert from signed files on Windows, Linux and Mac.

Kalyxt commented 5 years ago

@Kalyxt Maybe https://github.com/secana/PeNet can do what you want (I'm the author). The PeNet library is able to extract the cert from signed files on Windows, Linux and Mac.

Thanks! This is exactly what I was looking for. I've tested it on win and linux-arm.

bartonjs commented 5 years ago

Is this going to be fixed in upcoming .NET Core versions (3.0)

No, there's not a lot of time left in the 3.0 release and I'm already over committed. (Though PRs are welcome if someone wants to contribute a fix...)

secana commented 4 years ago

Any updates on the issue regarding the version it will most likely be fixed in? It's not fixed in 3.1 and was already an issue in 2.0. If someone points me to a starting point, I could try to implement it myself and do PR. Never worked with the dotnet repo, so any hints are welcome.

bartonjs commented 4 years ago

I'll go ahead and mark it as 5 (aka "next version"). If you want to give it a try, https://github.com/dotnet/runtime/blob/4f9ae42d861fcb4be2fcd5d3d55d5f227d30e723/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Unix/PkcsFormatReader.cs#L226-L236 is where things need to get smarter. Largely what's needed is a test suite. e.g. what happens on Windows with

If you just want the functionality but aren't too keen on playing with pointers, that's fine... it'll still get taken care of this release.

secana commented 4 years ago

TODO(2910): Figure out how to extract the signing certificate, when it's present. Getting the signing certificate on Linux from a PE file (taken from PeNet) is pretty straight forward.

Get the serial number of the signing certificate:

private string? GetSigningSerialNumber()
        {
            var asn1 = _contentInfo?.Content;
            if (asn1 is null) return null;
            var x = (Asn1Integer)asn1.Nodes[0].Nodes[4].Nodes[0].Nodes[1].Nodes[1]; // ASN.1 Path to signer serial number: /1/0/4/0/1/1
            return x.Value.ToHexString().Substring(2).ToUpper();
        }

Get the signing certificate from the PE file with the SignerSerialNumber from above.

private X509Certificate2 GetSigningCertificateNonWindows(PeFile peFile)
        {
            var collection = new X509Certificate2Collection();
            collection.Import(peFile.WinCertificate?.BCertificate.ToArray());
            return collection.Cast<X509Certificate2>().FirstOrDefault(cert =>
                string.Equals(cert.SerialNumber, SignerSerialNumber, StringComparison.CurrentCultureIgnoreCase));
        }

But I'm not sure on how to get access to the Asn1 of the PE file in the PkcsFormarReader.cs @bartonjs linked, so I'm stuck with any implementation attempt before I even get started :(

bartonjs commented 4 years ago

@secana There are two different issues.

vcsjones commented 2 years ago

I think I can pick this one up to get X509Certificate(byte[] contents) handling PKCS#7/CMS data.

but in single certificate mode it doesn't return certs[0], it returns the certificate which signed the structure.

@bartonjs Do you know off the top of your head if it is actually validating anything? Maybe it isn't going so far as to validate a certificate chain or anything, but I wonder what happens if the EncapsulatedContentInfo has been tampered with. The testing scenarios for this are going to be complex regardless, so I suppose I will find out.

If we actually need to validate something, we can't add a reference to S.S.C.Pkcs due to circular references.

bartonjs commented 2 years ago

Do you know off the top of your head if it is actually validating anything?

I'm like 98% sure that it's just a structural decode and checking if it's a signed CMS for Windows to report that it's CERT_QUERY_CONTENT_PKCS7_SIGNED. Once it's one of those, we just ask for the signerinfo, don't see any calls to verification: https://github.com/dotnet/runtime/blob/main/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/X509Certificates/CertificatePal.Windows.Import.cs#L109-L141 (The hCertStore there represents the optional embedded certificates, nothing magical and dynamic with CU\My lookup or anything like that)

NGame1 commented 2 years ago

@jeffhandley OMG! After about 7 years of opening this issue. Again is it postponed? No way!

et1975 commented 1 year ago

@jeffhandley This issue was in the 7.0.0 bucket that's now marked as completed. Is the fix in .NET 7?

vcsjones commented 1 year ago

This issue was in the 7.0.0 bucket that's now marked as completed. Is the fix in .NET 7?

No. This is currently marked "Future" and still open. This did not get complete for 7.0.

bryandam commented 1 year ago

I've been hitting this issue (I think), reading up on it, and am wondering if someone can confirm that I'm understanding the implications correctly. Does this imply that there is currently no cross-platform way in .NET to sign files or verify file signatures? In this case, files signed in Windows can't be verified on Unix/Linux?

vcsjones commented 1 year ago

Does this imply that there is currently no cross-platform way in .NET to sign files or verify file signatures? In this case, files signed in Windows can't be verified on Unix/Linux?

If you mean a file signed with Windows Authenticode, then correct - .NET does not provide an API to verify Authenticode signatures.

But new X509Certificate2(byte[]) on Windows does not do that either. It returns the signing cert, or the first cert-looking thing it can find in the file, but it does not actually check the signature.

bryandam commented 1 year ago

Thanks. Part of what I was struggling to determine was if all code signing in Windows was Authenticode or not as a lot of info just refers to 'digital signature'. In any case, I think it's safe to say that Window's signtool.exe is Authenticode.

Quite right, getting the cert from the file isn't validating the cert, that was a subsequent step I was taking. I just wanted to be sure that my actual goal of validating the file wasn't possible some other way that didn't necessitate actually having the cert object itself.

wstaelens commented 10 months ago

what is the status of this? as we think we experience invalid exports of PKCS#7 in .net 7 and seeing this makes me believe things are not fully implemented yet?

vcsjones commented 10 months ago

we think we experience invalid exports of PKCS#7 in .net 7

This issue is specifically about importing a PKCS#7 blob from X509Certificate{2} and it has not been implemented.

@wstaelens do you have a code snippet of what is not working for you?

wstaelens commented 10 months ago

@vcsjones We tried several things. Maybe missing something, not sure how to get it working.

  static void ExportTo(X509Certificate2 cert, string location, string baseFileName, string pwd)
    {
      // Create PFX (PKCS) with private key
      File.WriteAllBytes(Path.Combine(location, baseFileName + ".pfx"), cert.Export(X509ContentType.Pfx, pwd));
      var collection = new X509Certificate2Collection(cert);
      File.WriteAllBytes(Path.Combine(location, baseFileName + ".pk7"), collection.Export(X509ContentType.Pkcs7));

      // Create Base 64 encoded CER (public key only)
      File.WriteAllText(Path.Combine(location, baseFileName + ".cer"),
          "-----BEGIN CERTIFICATE-----\r\n"
          + Convert.ToBase64String(cert.Export(X509ContentType.Cert), Base64FormattingOptions.InsertLineBreaks)
          + "\r\n-----END CERTIFICATE-----");
    }

If we do cert.export(pkcs7 this does not work, doesn't seem to be supported?

If we put it in a collection and export it the file is about 30% smaller and when importing it it isn't valid (can't debug it is being loaded into an external device).

vcsjones commented 1 week ago

@bartonjs I think we can close this as "won't fix". I don't think we are going to add any new functionality to the X509Certificate constructors, and the "right" way to get certs out of a PKCS#7/CMS is to use SignedCms.

bartonjs commented 1 week ago

Yeah, we probably shouldn't be adding features to [Obsolete] members (unless we get a compelling compat/square-the-circle scenario).