Runtime: Returning an object initialized with a using {} block results in null on Linux but works on Windows

[CameronGoodwin]

Debugging my server sample I found that on Linux if you return the object defined with a using {} block of code it will result in null from the caller. However this works on Windows. Not sure which resulting behavior is correct from a language / platform point of view but wanted to call out the discrepancy between the runtime on Linux vs Windows.

Example code that works on Windows, but results in a null object on Linux:

        /// <summary>
        /// Creates a ECDsa instance with the specified curve
        /// </summary>
        /// <param name="Curve">ECCurve value</param>
        /// <returns>ECDsa instance</returns>
        public static ECDsa Create(ECCurve Curve)
        {
            using (var ecdsa = ECDsa.Create(Curve))
            {
                return ecdsa;
            }
        }

To fix this on Linux you need to remove the using variable to avoid the null.

        /// <summary>
        /// Creates a ECDsa instance with the specified curve
        /// </summary>
        /// <param name="Curve">ECCurve value</param>
        /// <returns>ECDsa instance</returns>
        public static ECDsa Create(ECCurve Curve)
        {
                return ECDsa.Create(Curve);
        }

[karelz]

@CameronGoodwin can you please provide repro we can run locally?

[CameronGoodwin]

Sure! Here is some code direct from my sample (which I can send you the entire sample sln over email if you want).

```c#
using System.Runtime.Serialization;
using System.Security.Cryptography;

public static class ProofKeyUtility
{
///

/// Creates a ECDsaP256 ProofKey
///

/// ECDsa instance
public static ECDsa Create()
{
// On Windows you can directly pass the ECParameters into the ECDsa.Create()
// Function like so: Create(ECCurve.NamedCurves.nistP256);
//
// HOWEVER, on Linux that will result in the following exception:
// The specified curve 'ecdsa_p256' or its parameters are not valid for
// this platform.
// For more information on this issue see the following:
// https://github.com/dotnet/corefx/issues/32323
//
// To work around this on Linux we create the ECDsa from an Oid created from
// the friendly name rahter than passing in the friendly name to ECDsa.Create()
// directly. This code also works on Windows.
var oid = Oid.FromFriendlyName(XstsConstants.ECDSAP256OidFriendlyName, OidGroup.All);
ECCurve curve = ECCurve.CreateFromOid(oid);
return Create(curve);
}

    /// <summary>
    /// Creates a ECDsa instance with the specified curve
    /// </summary>
    /// <param name="Curve">ECCurve value</param>
    /// <returns>ECDsa instance</returns>
    public static ECDsa Create(ECCurve Curve)
    {
        using (var ecdsa = ECDsa.Create(Curve))
        {
            return ecdsa;
        }
    }

}

Then with the above class do the following.  It runs fine on Windows, but when you run on Linux ecdsa will be null and therefore throw an exception when you try to call ExportParameters on it:
```c#
var ecdsa = ProofKeyUtility.Create();
var eParameters = ecdsa.ExportParameters(true);

[karelz]

The difference seems to be if you call Dispose on ECDsa vs. not. Is that correct?
cc @bartonjs

[CameronGoodwin]

I'm not calling Dispose directly, but it should be called after the using {} block by the system if I am correct.

[karelz]

Yep, that's what using does.

[CameronGoodwin]

Any idea why there is a functional difference of that code on Windows vs Linux? Which one is the correct behavior? I've updated my code so this isn't blocking me but just wanted to call it out in case there was something deeper.

[jnm2]

c# using (var ecdsa = ECDsa.Create(Curve)) { return ecdsa; }

Why are you returning a disposed object?

[svick]

As far as I can tell, the core issue was correctly identified by @jnm2: You're using a disposed object. When you do that, there is no guarantee that it will work, which is why you're getting an exception on Linux. Though you should not get NullReferenceException, the right exception kind would be ObjectDisposedException. It works on Windows, because Windows has a completely different implementation, where ExportParameters still works after Dispose().

Note that you're never getting a null object in your code, the NullReferenceException is thrown from the implementation of ExportParameters.

What I think should be done:

1. Consider whether ExportParameters should work on Linux even after the object has been disposed, to make it more consistent with Windows.

   My opinion is that this is not worth doing. If the user uses a disposed object, they have a bug and throwing an exception is okay in that case.

   If it's decided that this still should not work, the code should still be changed, to ensure it throws ObjectDisposedException, not NullReferenceException.

2. Consider whether ExportParameters should be changed on Windows to throw if the object has been disposed, to make it more consistent with Linux.

   This would be a breaking change. And even though it's only breaking code that is incorrect, it's probably not worth changing.

---

For the record, the code in question is:

• Linux: ECDsaImplementation.ECDsaOpenSsl.Dispose, ECOpenSsl.FreeKey, ECDsaImplementation.ECDsaOpenSsl.ExportParameters
• Windows: ECDsaImplementation.ECDsaCng.ExportParameters

[bartonjs]

The Windows implementation seems to be accidentally doing resurrection (it thinks it's in pre-generation); but we should clean up both sides to properly report ObjectDisposedException.

[CameronGoodwin]

Thanks all. I agree that you shouldn't be using a disposed object. As to why my code did that or how I stumbled across this... Was learning more in .Net and been a while since I did C# so it was a learning mistake :). Just wanted to call out the disparity between behavior on Windows vs Linux in .Net Core. I think the two posts above properly call out that it should be returning an ObjectDisposedException at least.

[joshudson]

On Linux, forgetting to dispose your crypto keys is a security vulnerability.

[jnm2]

You'd still not return a disposed object because disposed objects should not be interacted with.

[joshudson]

@jnm2 : All right, I'll be explicit. "Consider whether ExportParameters should work on Linux even after the object has been disposed". If you make this change you will have introduced a security vulnerability because the object must actually be disposed.

[jnm2]

@joshudson The object should always be disposed. The question is how soon. What you should do is wait until no code needs the object anymore, then dispose it.
The ECDsa object above was first disposed, then returned to the caller. That is disposing too soon. The responsibility to dispose needs to be passed to the caller so long as the caller is responsible to call ExportParameters. The code that calls ExportParameters is the code that should immediately follow that by disposing the ECDsa (with a using block if possible), assuming that is the last thing it needs the ECDsa object for.

[bartonjs]

"Consider whether ExportParameters should work on Linux even after the object has been disposed". If you make this change you will have introduced a security vulnerability because the object must actually be disposed.

The bug on Windows is that Dispose put the object into a state that looked like no key had been generated again, so ExportParameters actually caused a new key to be generated, then exported (keys aren't generated by the ctor, so that Import isn't wasteful). It's not that Dispose did nothing, or kept the key/keyref alive too long.

That said, the fix will be that both implementations should be throwing an ObjectDisposedException, not one doing accidental state reset and the other throwing a NullReferenceException.