Possible memory leak in ECDiffieHellmanOpenSsl

[TobiasBladhSinch]

Problem

Creating ECDiffieHellmanOpenSsl objects using ECParameters causes a memory leak with each new object created. Same is true when calling ECDiffieHellmanOpenSsl.DeriveKeyFromHmac with a custom implementation of ECDiffieHellmanPublicKey. (see test application below)

Description

First found when memory consumption kept rising in linux docker container using the base image mcr.microsoft.com/dotnet/runtime:5.0.4-buster-slim while creating new ECDiffieHellmanOpenSsl using named ECParameters with set ECPoint (like in BuildECParameters() below).
Replicated on Ubuntu 20.04 (WSL) using the console application below with SDK Version: 5.0.400. Here to monitor the memory consumption the dotnet-counters tool was used. Note that the GC Heap Size does not grow, only the Working Set.

Since the TestCreatingECDHWithECParams() method does not leak (which uses a named curve to define the ECDiffieHellman object) but both TestCreatingECDHWithECParams() and TestCustomPublicKeyImplementation() does, and one big common difference between the two scenarios is what unmanaged code is called to create the SafeEcKeyHandle. I suspect the problem is related to the Interop calls referenced in the method descriptions below. But I might be wrong.

Or have I missed something with the scenarios showcased below?

Memory leak console test class

using System;
using System.Globalization;
using System.Security.Cryptography;

namespace MemLeakTest
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("Starting test...");
            if (args[0] == "namedCurve")
                TestCreatingECDHWithNamedCurve();
            else if (args[0] == "ecParameters")
                TestCreatingECDHWithECParams();
            else if (args[0] == "publicKey")
                TestCustomPublicKeyImplementation();
            else
                Console.WriteLine("Invalid params: " + args[0]);
            Console.WriteLine("Exiting test.");
        }

        /// <summary>
        /// NO MEMORY LEAK
        /// Looks like it ends up calling this:
        /// https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcKey.cs#L31
        /// </summary>
        private static void TestCreatingECDHWithNamedCurve()
        {
            Console.WriteLine("Testing creating ECDH with NamedCurves.");

            for (int i = 0; i < 500000; i++)
            {
                using var serverEcdh = ECDiffieHellman.Create(ECCurve.NamedCurves.nistP256);
            }
            Console.WriteLine("Done!");
            Console.ReadLine();
        }

        /// <summary>
        /// GROWING WORKING SET WHILE EXECUTING.
        /// Looks like it ends up calling this:
        /// https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.ImportExport.cs#L22
        /// </summary>
        private static void TestCreatingECDHWithECParams()
        {
            Console.WriteLine("Testing creating ECDH with ECParameters.");

            for (int i = 0; i < 500000; i++)
            {
                // Creating the ECDiffieHellman object causes a memory leak
                using var serverEcdh = ECDiffieHellman.Create(CustomECDiffieHellmanPublicKey.BuildECParameters());
            }
            Console.WriteLine("Done!");
            Console.ReadLine();
        }

        /// <summary>
        /// GROWING WORKING SET WHILE EXECUTING.
        /// Looks like it ends up calling this:
        /// https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.ImportExport.cs#L22
        /// </summary>
        private static void TestCustomPublicKeyImplementation()
        {
            Console.WriteLine("Testing DeriveKeyFromHmac with custom ECDiffieHellmanPublicKey.");

            using (var userAgentPublicKey = new CustomECDiffieHellmanPublicKey())
            using (var serverEcdh = ECDiffieHellman.Create(ECCurve.NamedCurves.nistP256))
            {
                for (int i = 0; i < 500000; i++)
                {
                    // Calling DeriveKeyFromHmac causes a memory leak
                    serverEcdh.DeriveKeyFromHmac(userAgentPublicKey, HashAlgorithmName.SHA256, null);
                }
                Console.WriteLine("Done!");
                Console.ReadLine();
            }
        }
    }

    public sealed class CustomECDiffieHellmanPublicKey : ECDiffieHellmanPublicKey
    {
        public override ECParameters ExportExplicitParameters() =>
            BuildECParameters();

        public override ECParameters ExportParameters() =>
            BuildECParameters();

        protected override void Dispose(bool disposing)
        {
            base.Dispose(disposing);
        }

        internal static ECParameters BuildECParameters()
        {
            var parameters = new ECParameters
            {
                Curve = ECCurve.NamedCurves.nistP256,
                Q = new ECPoint
                {
                    X = HexToByteArray("fa719e6b556b83d413969196afdf2b07ce1ad14829f48b4c290fe276925148c7"),
                    Y = HexToByteArray("984a4a8d4686f162feefee023bc77184ea705e32bc304f0dbd166a2fe2ed204f")
                }
            };

            return parameters;
        }

        private static byte[] HexToByteArray(string hexString)
        {
            byte[] bytes = new byte[hexString.Length / 2];

            for (int i = 0; i < hexString.Length; i += 2)
            {
                string s = hexString.Substring(i, 2);
                bytes[i / 2] = byte.Parse(s, NumberStyles.HexNumber, null);
            }

            return bytes;
        }
    }
}

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

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

Issue Details

---

Problem

Creating ECDiffieHellmanOpenSsl objects using ECParameters causes a memory leak with each new object created. Same is true when calling ECDiffieHellmanOpenSsl.DeriveKeyFromHmac with a custom implementation of ECDiffieHellmanPublicKey. (see test application below)

Description

First found when memory consumption kept rising in linux docker container using the base image mcr.microsoft.com/dotnet/runtime:5.0.4-buster-slim while creating new ECDiffieHellmanOpenSsl using named ECParameters with set ECPoint (like in BuildECParameters() below).
Replicated on Ubuntu 20.04 (WSL) using the console application below with SDK Version: 5.0.400. Here to monitor the memory consumption the dotnet-counters tool was used. Note that the GC Heap Size does not grow, only the Working Set.

Since the TestCreatingECDHWithECParams() method does not leak (which uses a named curve to define the ECDiffieHellman object) but both TestCreatingECDHWithECParams() and TestCustomPublicKeyImplementation() does, and one big common difference between the two scenarios is what unmanaged code is called to create the SafeEcKeyHandle. I suspect the problem is related to the Interop calls referenced in the method descriptions below. But I might be wrong.

Or have I missed something with the scenarios showcased below?

Memory leak console test class

using System;
using System.Globalization;
using System.Security.Cryptography;

namespace MemLeakTest
{
    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine("Starting test...");
            if (args[0] == "namedCurve")
                TestCreatingECDHWithNamedCurve();
            else if (args[0] == "ecParameters")
                TestCreatingECDHWithECParams();
            else if (args[0] == "publicKey")
                TestCustomPublicKeyImplementation();
            else
                Console.WriteLine("Invalid params: " + args[0]);
            Console.WriteLine("Exiting test.");
        }

        /// <summary>
        /// NO MEMORY LEAK
        /// Looks like it ends up calling this:
        /// https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcKey.cs#L31
        /// </summary>
        private static void TestCreatingECDHWithNamedCurve()
        {
            Console.WriteLine("Testing creating ECDH with NamedCurves.");

            for (int i = 0; i < 500000; i++)
            {
                using var serverEcdh = ECDiffieHellman.Create(ECCurve.NamedCurves.nistP256);
            }
            Console.WriteLine("Done!");
            Console.ReadLine();
        }

        /// <summary>
        /// GROWING WORKING SET WHILE EXECUTING.
        /// Looks like it ends up calling this:
        /// https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.ImportExport.cs#L22
        /// </summary>
        private static void TestCreatingECDHWithECParams()
        {
            Console.WriteLine("Testing creating ECDH with ECParameters.");

            for (int i = 0; i < 500000; i++)
            {
                // Creating the ECDiffieHellman object causes a memory leak
                using var serverEcdh = ECDiffieHellman.Create(CustomECDiffieHellmanPublicKey.BuildECParameters());
            }
            Console.WriteLine("Done!");
            Console.ReadLine();
        }

        /// <summary>
        /// GROWING WORKING SET WHILE EXECUTING.
        /// Looks like it ends up calling this:
        /// https://github.com/dotnet/runtime/blob/57bfe474518ab5b7cfe6bf7424a79ce3af9d6657/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.ImportExport.cs#L22
        /// </summary>
        private static void TestCustomPublicKeyImplementation()
        {
            Console.WriteLine("Testing DeriveKeyFromHmac with custom ECDiffieHellmanPublicKey.");

            using (var userAgentPublicKey = new CustomECDiffieHellmanPublicKey())
            using (var serverEcdh = ECDiffieHellman.Create(ECCurve.NamedCurves.nistP256))
            {
                for (int i = 0; i < 500000; i++)
                {
                    // Calling DeriveKeyFromHmac causes a memory leak
                    serverEcdh.DeriveKeyFromHmac(userAgentPublicKey, HashAlgorithmName.SHA256, null);
                }
                Console.WriteLine("Done!");
                Console.ReadLine();
            }
        }
    }

    public sealed class CustomECDiffieHellmanPublicKey : ECDiffieHellmanPublicKey
    {
        public override ECParameters ExportExplicitParameters() =>
            BuildECParameters();

        public override ECParameters ExportParameters() =>
            BuildECParameters();

        protected override void Dispose(bool disposing)
        {
            base.Dispose(disposing);
        }

        internal static ECParameters BuildECParameters()
        {
            var parameters = new ECParameters
            {
                Curve = ECCurve.NamedCurves.nistP256,
                Q = new ECPoint
                {
                    X = HexToByteArray("fa719e6b556b83d413969196afdf2b07ce1ad14829f48b4c290fe276925148c7"),
                    Y = HexToByteArray("984a4a8d4686f162feefee023bc77184ea705e32bc304f0dbd166a2fe2ed204f")
                }
            };

            return parameters;
        }

        private static byte[] HexToByteArray(string hexString)
        {
            byte[] bytes = new byte[hexString.Length / 2];

            for (int i = 0; i < hexString.Length; i += 2)
            {
                string s = hexString.Substring(i, 2);
                bytes[i / 2] = byte.Parse(s, NumberStyles.HexNumber, null);
            }

            return bytes;
        }
    }
}

Author:	TobiasBladhSinch
Assignees:	-
Labels:	area-System.Security, untriaged
Milestone:	-

[TobiasBladhSinch]

@bartonjs Hi, I'm wondering since the untriaged label has been removed and the issue has been added to the 7.0.0 milestone. Has this issues been accepted and verified?

Also, if so, is there any plans to fix and release a fix to this prio to the 7.0.0 milestone?

[ohroy]

follow