SafeCertContextHandle is missing a default constructor

[MichalStrehovsky]

This one:

runtime/src/libraries/System.Security.Cryptography.Pkcs/src/Microsoft/Win32/SafeHandles/SafeCertContextHandle.cs

Line 12 in 6072e4d

internal sealed class SafeCertContextHandle : SafeHandle

Without that, it cannot be used in return value marshalling because the return value marshaller needs the default constructor to make an instance of one.

Here's an example of a p/invoke that doesn't actually work because of this:

runtime/src/libraries/Common/src/Interop/Windows/Crypt32/Interop.CertCreateCertificateContext.cs

Lines 11 to 16 in 6072e4d

	[DllImport(Libraries.Crypt32, CharSet = CharSet.Unicode, SetLastError = true)]
	internal static extern unsafe SafeCertContextHandle CertCreateCertificateContext(
	MsgEncodingType dwCertEncodingType,
	void* pbCertEncoded,
	int cbCertEncoded);
	}

Interop.CertCreateCertificateContext.cs is probably dead code. But I can see that SafeCertContextHandle is actually used as a return value in a number of p/invokes. It should probably be investigated why they're not failing (or why we have no coverage).

Here's a sample standalone example showing the CertCreateCertificateContext p/invoke not actually working (feel free to paste into an empty console app):

using System;
using System.Runtime.InteropServices;

unsafe
{
    Crypt32.CertCreateCertificateContext(0, null, 0);
}

internal static partial class Crypt32
{
    [DllImport("Crypt32", CharSet = CharSet.Unicode, SetLastError = true)]
    internal static extern unsafe SafeCertContextHandle CertCreateCertificateContext(
        int dwCertEncodingType,
        void* pbCertEncoded,
        int cbCertEncoded);
}

internal sealed class SafeCertContextHandle : SafeHandle
{
    internal SafeCertContextHandle(IntPtr handle) :
        base(handle, ownsHandle: true)
    {
    }

    public sealed override bool IsInvalid
    {
        get { return handle == IntPtr.Zero; }
    }

    protected sealed override bool ReleaseHandle()
    {
        //Interop.Crypt32.CertFreeCertificateContext(handle); // CertFreeCertificateContext always returns TRUE so no point in checking.
        SetHandle(IntPtr.Zero);
        return true;
    }
}

The above example will fail with a useless System.MissingMethodException: '.ctor', but that's what interop is complaining about.

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @jeffhandley
See info in area-owners.md if you want to be subscribed.

[vcsjones]

Interesting. I'll take a look.

[vcsjones]

Here's what I found.

1. There are two SafeCertContextHandles because... well I don't know why. There is one in X509 and one in Pkcs. We're looking at the one in Pkcs. The one in X509 does have a default constructor.

   But I can see that SafeCertContextHandle is actually used as a return value in a number of p/invokes

   You're likely seeing usages of the X509 version of SafeCertContextHandle:

   runtime/src/libraries/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/Native/SafeHandles.cs

   Line 44 in 54623ba

   internal class SafeCertContextHandle : SafePointerHandle<SafeCertContextHandle>

2. The p/invoke CertCreateCertificateContext in Pkcs is indeed unused code an can be removed.

3. The rest of the usages of the Pkcs SafeCertContextHandle are explicitly constructing it with the IntPtr constructor. It's never used as a return or [Out] from p/invoke.

   runtime/src/libraries/System.Security.Cryptography.Pkcs/src/Internal/Cryptography/Pal/Windows/HelpersWindows.cs

   Lines 144 to 146 in 54623ba

   	IntPtr pCertContext = cert.Handle;
   	pCertContext = Interop.Crypt32.CertDuplicateCertificateContext(pCertContext);
   	SafeCertContextHandle hCertContext = new SafeCertContextHandle(pCertContext);

[vcsjones]

There are two SafeCertContextHandles

"Can we consolidate them and clean this up? 🤔"

Yeah I think that work is being tracked in #26541. It's a bit tangled up though, I'll see if I can keep chipping away at that cleanup effort.

[MichalStrehovsky]

I don't have a vested interest in the resolution of this - if this is essentially a dup of #26541, we can close this. I hit this when running a tool that eagerly pregenerates p/invoke interop and this hit a bug in the tool where it didn't handle junk input well. I fixed the bug in the tool and filed this because I was already in the middle of yak shaving and didn't want to deal with another yak.

[dotnet-policy-service]

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.

[dotnet-policy-service]

This issue will now be closed since it had been marked no-recent-activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.