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

Rsa ToXmlString is not supported on .NET Core 2.0 #23391

Closed
danmoseley opened this issue Aug 30, 2017 · 40 comments · Fixed by dotnet/corefx#37593
Closed

Rsa ToXmlString is not supported on .NET Core 2.0 #23391

danmoseley opened this issue Aug 30, 2017 · 40 comments · Fixed by dotnet/corefx#37593

Comments

@danmoseley
Copy link
Member

@myloveCc commented on Sun Aug 20 2017

rsa.ToXmlString(true) is not supported on .NET Core 2.0

 var rsa = RSA.Create(2048);
 var privateKey = rsa.ToXmlString(true);    // //not supported exception

@bartonjs commented on Mon Aug 21 2017

Yep. In .NET Framework networking, XML, and cryptography all live in one library (mscorlib). In .NET Core they're in three separate libraries, and making this method work requires a circle:

  • XML needs networking, because (even though we discourage it) DTDs can initiate downloads.
  • Networking needs cryptography because TLS/https have X.509 certificates.
  • Cryptography needs XML for ToXmlString/FromXmlString.

The easiest link to snip is the ToXmlString/FromXmlString methods.

Technically ToXmlString can be written without XML (like in .NET Framework's implementation, but FromXmlString requires an XML parser.

My recommendation would be to make a public static string ToXmlString(this RSAParameters rsaParameters) method, and a public static RSAParameters FromXmlString(string xml) method; borrowing from NetFx and CoreFx's SignedXml; and to ultimately try moving off of the XML representations.


@myloveCc commented on Mon Aug 21 2017

This feature is not difficult, and I have been achieved in my project。

using System;
using System.Security.Cryptography;
using System.Xml;
using Newtonsoft.Json;
using NETCore.Encrypt.Shared;
using NETCore.Encrypt.Internal;

namespace NETCore.Encrypt.Extensions.Internal
{

    internal static class RSAKeyExtensions
    {
        #region JSON
        internal static void FromJsonString(this RSA rsa, string jsonString)
        {
            Check.Argument.IsNotEmpty(jsonString, nameof(jsonString));
            try
            {
                var paramsJson = JsonConvert.DeserializeObject<RSAParametersJson>(jsonString);

                RSAParameters parameters = new RSAParameters();

                parameters.Modulus = paramsJson.Modulus != null ? Convert.FromBase64String(paramsJson.Modulus) : null;
                parameters.Exponent = paramsJson.Exponent != null ? Convert.FromBase64String(paramsJson.Exponent) : null;
                parameters.P = paramsJson.P != null ? Convert.FromBase64String(paramsJson.P) : null;
                parameters.Q = paramsJson.Q != null ? Convert.FromBase64String(paramsJson.Q) : null;
                parameters.DP = paramsJson.DP != null ? Convert.FromBase64String(paramsJson.DP) : null;
                parameters.DQ = paramsJson.DQ != null ? Convert.FromBase64String(paramsJson.DQ) : null;
                parameters.InverseQ = paramsJson.InverseQ != null ? Convert.FromBase64String(paramsJson.InverseQ) : null;
                parameters.D = paramsJson.D != null ? Convert.FromBase64String(paramsJson.D) : null;
                rsa.ImportParameters(parameters);
            }
            catch
            {
                throw new Exception("Invalid JSON RSA key.");
            }
        }

        internal static string ToJsonString(this RSA rsa, bool includePrivateParameters)
        {
            RSAParameters parameters = rsa.ExportParameters(includePrivateParameters);

            var parasJson = new RSAParametersJson()
            {
                Modulus = parameters.Modulus != null ? Convert.ToBase64String(parameters.Modulus) : null,
                Exponent = parameters.Exponent != null ? Convert.ToBase64String(parameters.Exponent) : null,
                P = parameters.P != null ? Convert.ToBase64String(parameters.P) : null,
                Q = parameters.Q != null ? Convert.ToBase64String(parameters.Q) : null,
                DP = parameters.DP != null ? Convert.ToBase64String(parameters.DP) : null,
                DQ = parameters.DQ != null ? Convert.ToBase64String(parameters.DQ) : null,
                InverseQ = parameters.InverseQ != null ? Convert.ToBase64String(parameters.InverseQ) : null,
                D = parameters.D != null ? Convert.ToBase64String(parameters.D) : null
            };

            return JsonConvert.SerializeObject(parasJson);
        }
        #endregion

        #region XML

        public static void FromXmlString(this RSA rsa, string xmlString)
        {
            RSAParameters parameters = new RSAParameters();

            XmlDocument xmlDoc = new XmlDocument();
            xmlDoc.LoadXml(xmlString);

            if (xmlDoc.DocumentElement.Name.Equals("RSAKeyValue"))
            {
                foreach (XmlNode node in xmlDoc.DocumentElement.ChildNodes)
                {
                    switch (node.Name)
                    {
                        case "Modulus": parameters.Modulus = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "Exponent": parameters.Exponent = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "P": parameters.P = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "Q": parameters.Q = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "DP": parameters.DP = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "DQ": parameters.DQ = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "InverseQ": parameters.InverseQ = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "D": parameters.D = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                    }
                }
            }
            else
            {
                throw new Exception("Invalid XML RSA key.");
            }

            rsa.ImportParameters(parameters);
        }

        public static string ToXmlString(this RSA rsa, bool includePrivateParameters)
        {
            RSAParameters parameters = rsa.ExportParameters(includePrivateParameters);

            return string.Format("<RSAKeyValue><Modulus>{0}</Modulus><Exponent>{1}</Exponent><P>{2}</P><Q>{3}</Q><DP>{4}</DP><DQ>{5}</DQ><InverseQ>{6}</InverseQ><D>{7}</D></RSAKeyValue>",
                  parameters.Modulus != null ? Convert.ToBase64String(parameters.Modulus) : null,
                  parameters.Exponent != null ? Convert.ToBase64String(parameters.Exponent) : null,
                  parameters.P != null ? Convert.ToBase64String(parameters.P) : null,
                  parameters.Q != null ? Convert.ToBase64String(parameters.Q) : null,
                  parameters.DP != null ? Convert.ToBase64String(parameters.DP) : null,
                  parameters.DQ != null ? Convert.ToBase64String(parameters.DQ) : null,
                  parameters.InverseQ != null ? Convert.ToBase64String(parameters.InverseQ) : null,
                  parameters.D != null ? Convert.ToBase64String(parameters.D) : null);
        }

        #endregion
    }
}

@Petermarcu commented on Wed Aug 30 2017

@danmosemsft can you get this moved over to dotnet/corefx to be tracked as an issue there?

@danmoseley
Copy link
Member Author

@bartonjs I wonder if we should put implementations in a separate library, that could depend on both Crypto and XML? Otherwise everyone has to write/paste similar code.

@danmoseley
Copy link
Member Author

These ToXmlSTring/FromXmlSTring methods do seem to have significant usage..

@bartonjs
Copy link
Member

This is an understood limitation in .NET Core (and on the PNSE warning list, etc). If we continue getting feedback once the PNSE warning tooling is readily available we can look into doing trickery with lightweight XML parsers.

To restate the problem: Networking, XML, and Crypto ended up in a circular dependency in mscorlib. The factoring that we've done in Core makes it not possible to do this right now.

@danmoseley
Copy link
Member Author

@bartonjs, I was referring to shipping extension methods like those above. I don't believe that would cause a circular dependency.

@bartonjs
Copy link
Member

@danmosemsft Yeah, I noticed the usage comment but missed the previous one.

I don't know if the problem is that people want

  • to use XML serialization specifically (which could be solved by a separate package)
  • a compact serialization for keys-without-certificates, in which case https://github.com/dotnet/corefx/issues/20414 is the better solution (more standardized),
  • existing code/samples to work (which would mean making this method work. By reflection, a lightweight parser, whatever)

@fletchsod-developer
Copy link

fletchsod-developer commented Aug 31, 2017

:-( :-( :-(

Suggestion as workaround to it? Because DotNetCore 2.1 and higher are still a work in progress.

@danmoseley
Copy link
Member Author

@fletchsod-developer I suggest adding to your project extension methods similar to the ones posted above. (I didn't review that code but essentially that approach)

@strombringer
Copy link

Just an FYI: The extension method doesn't generate xml in the same format as the original mscorlib method. For public keys it leaves empty tags for P, W, DP, DQ, InverseQ and D, whereas the mscorlib method just omits those tags:

mscorlib:
<RSAKeyValue><Modulus>3ZWrUY0Y6...</Modulus><Exponent>AQAB</Exponent></RSAKeyValue>

extension method
<RSAKeyValue><Modulus>3ZWrUY0Y6...</Modulus><Exponent>AQAB</Exponent><P></P><Q></Q><DP></DP><DQ></DQ><InverseQ></InverseQ><D></D></RSAKeyValue>

@eerhardt
Copy link
Member

FYI - here's another user running into this problem: https://stackoverflow.com/questions/46415078/net-core-2-0-rsa-platformnotsupportedexception

@mattiasw2
Copy link

Thank you! Worked fine. Embarrasing that it is only detected in runtime by System.PlatformNotSupportedException

@danmoseley
Copy link
Member Author

@pjanotti is this caught by the PNSE tooling?

@mattiasw2
Copy link

Sorry, do not know what PNSE tooling is. I am only moving some old crypto-code from an old ASP.NET project to Java, and seemed easier to use dotnet core, and call an console app for decryption of old data. Using Visual Studio Code as a test, more used to VS

@danmoseley
Copy link
Member Author

@mattiasw2 it is this: https://github.com/dotnet/platform-compat
The intent of it is to help flag cases where code may not be portable despite the API being available. (If the API is unavailable, of course, the compiler will catch it.

@danmoseley
Copy link
Member Author

It looks like it should flag it.
https://github.com/dotnet/platform-compat/blob/6613b6b16bc8e22aa3cc2a3debee49f5e2c4ccd6/etc/exceptions.filtered.site.csv#L1109
It woudl be great if you could try it and confirm.

@mattiasw2
Copy link

It only flags it during runtime by throwing the not supported exception. There is no warning in compile time. Just downloaded latest Visual Studio Core and netcore2.0

@danmoseley
Copy link
Member Author

@mattiasw2 the platform-compat tool I linked to has to be installed into VS. As you see in the screenshots https://github.com/dotnet/platform-compat if it works correctly it will then flag issues at compile time.

e-llumin referenced this issue in e-llumin-net-ltd/rhino-licensing Jan 22, 2018
e-llumin referenced this issue in e-llumin-net-ltd/rhino-licensing Jan 22, 2018
e-llumin referenced this issue in e-llumin-net-ltd/rhino-licensing Jan 22, 2018
e-llumin referenced this issue in e-llumin-net-ltd/rhino-licensing Jan 22, 2018
@Korporal
Copy link

Korporal commented Mar 4, 2018

@myloveCc @bartonjs @danmosemsft @Petermarcu - Hi.

I'm seeing the same problem but I must say the "solution" here is rather unimpressive. If after all these years we've come back full circle and the much lauded "code reuse" promised by .Net is now nothing more than copying/pasting source code. then we're in trouble.

The reason is I think obvious, frantically copying snippets of code every time we encounter an "issue" leads to fragility, once we actually get a non-trivial app constructed and we test it, we have much more work to do to prove if our code is flawed or something in the hand-copied-framewrork-support-logic is somehow in error, perhaps copied wrongly for example.

May I suggest that in situations like this at the very least Microsoft formally publish a recommended workaround until the platform itself gets the proper support. At least this way we'd be sure we're all using the same workaround and reduce risk.

How are companies expected to build rock solid mission critical applications when fundamental parts of their system (in this case stuff associated with identity, encryption and authentication) are gleaned by copying/pasting snippets from Github issues!

Given Microsoft's desire to be a strong player in cloud hosting it is absolutely astonishing that something as fundamental as this is apparently overlooked.

Thanks

@Korporal
Copy link

Korporal commented Mar 4, 2018

@myloveCc @bartonjs @danmosemsft @Petermarcu

Actually given that these keys are often represented as PEM files, why not (in .Net Core) add a new method that accepts that format and eliminate the need to even bother with other formats?

I'm trying to use BouncyCastle but alas that brings its own wonderful set of issues...

@bartonjs
Copy link
Member

bartonjs commented Mar 4, 2018

PKCS1/PKCS8/EncryptedPKCS8 in DER and PEM are covered by https://github.com/dotnet/corefx/issues/20414

timotei referenced this issue in timotei/RestSharp Mar 15, 2018
There are some issues on .NET's side about the
missing functionality of RSACryptoServiceProvider
ToXmlString/FromXmlString:

* https://github.com/dotnet/corefx/issues/23686
* dotnet/core#874

So, let's do it ourselves, and if in the future we'll
get it in the .NET itself, we can remove this code
@YetaWF
Copy link

YetaWF commented Apr 20, 2018

While closed, this issue is really not fixed. To/FromXmlString don't work. At least be honest about it and remove them altogether. Or is there a platform where they work?

@bartonjs
Copy link
Member

@YetaWF They are defined in .NET Core because they are defined by .NET Standard.

They don't work on any .NET Core platform, because .NET Core doesn't have mscorlib.dll, the one library which contained cryptography, XML, and networking all together.

@YetaWF
Copy link

YetaWF commented Apr 20, 2018

That's not much of an reason. There are numerous other APIs that are in mscorlib.dll and have been replaced by netstandard/netcoreapp. That's like saying .NET Core doesn't have .NET so we can't do anything. So everyone has to implement their own. I see the progress we're making.

@bartonjs
Copy link
Member

To/FromXmlString requires XML. XML requires networking (because of legacy DTD support). Networking requires X509Certificates (because of SslStream). X509Certificates requires the crypto algorithms classes.

There's a circular dependency there. The link in the circle that we decided to break was To/FromXmlString.

@YetaWF
Copy link

YetaWF commented Apr 20, 2018

Dude, I read the whole thread. Yet, the solution is as simple as a cut/paste (with minor mods). Still...
Yeah I copied that in case you missed it.

So still, the problem is not fixed.

using System;
using System.Security.Cryptography;
using System.Xml;
using Newtonsoft.Json;
using NETCore.Encrypt.Shared;
using NETCore.Encrypt.Internal;

namespace NETCore.Encrypt.Extensions.Internal
{

    internal static class RSAKeyExtensions
    {
        #region JSON
        internal static void FromJsonString(this RSA rsa, string jsonString)
        {
            Check.Argument.IsNotEmpty(jsonString, nameof(jsonString));
            try
            {
                var paramsJson = JsonConvert.DeserializeObject<RSAParametersJson>(jsonString);

                RSAParameters parameters = new RSAParameters();

                parameters.Modulus = paramsJson.Modulus != null ? Convert.FromBase64String(paramsJson.Modulus) : null;
                parameters.Exponent = paramsJson.Exponent != null ? Convert.FromBase64String(paramsJson.Exponent) : null;
                parameters.P = paramsJson.P != null ? Convert.FromBase64String(paramsJson.P) : null;
                parameters.Q = paramsJson.Q != null ? Convert.FromBase64String(paramsJson.Q) : null;
                parameters.DP = paramsJson.DP != null ? Convert.FromBase64String(paramsJson.DP) : null;
                parameters.DQ = paramsJson.DQ != null ? Convert.FromBase64String(paramsJson.DQ) : null;
                parameters.InverseQ = paramsJson.InverseQ != null ? Convert.FromBase64String(paramsJson.InverseQ) : null;
                parameters.D = paramsJson.D != null ? Convert.FromBase64String(paramsJson.D) : null;
                rsa.ImportParameters(parameters);
            }
            catch
            {
                throw new Exception("Invalid JSON RSA key.");
            }
        }

        internal static string ToJsonString(this RSA rsa, bool includePrivateParameters)
        {
            RSAParameters parameters = rsa.ExportParameters(includePrivateParameters);

            var parasJson = new RSAParametersJson()
            {
                Modulus = parameters.Modulus != null ? Convert.ToBase64String(parameters.Modulus) : null,
                Exponent = parameters.Exponent != null ? Convert.ToBase64String(parameters.Exponent) : null,
                P = parameters.P != null ? Convert.ToBase64String(parameters.P) : null,
                Q = parameters.Q != null ? Convert.ToBase64String(parameters.Q) : null,
                DP = parameters.DP != null ? Convert.ToBase64String(parameters.DP) : null,
                DQ = parameters.DQ != null ? Convert.ToBase64String(parameters.DQ) : null,
                InverseQ = parameters.InverseQ != null ? Convert.ToBase64String(parameters.InverseQ) : null,
                D = parameters.D != null ? Convert.ToBase64String(parameters.D) : null
            };

            return JsonConvert.SerializeObject(parasJson);
        }
        #endregion

        #region XML

        public static void FromXmlString(this RSA rsa, string xmlString)
        {
            RSAParameters parameters = new RSAParameters();

            XmlDocument xmlDoc = new XmlDocument();
            xmlDoc.LoadXml(xmlString);

            if (xmlDoc.DocumentElement.Name.Equals("RSAKeyValue"))
            {
                foreach (XmlNode node in xmlDoc.DocumentElement.ChildNodes)
                {
                    switch (node.Name)
                    {
                        case "Modulus": parameters.Modulus = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "Exponent": parameters.Exponent = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "P": parameters.P = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "Q": parameters.Q = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "DP": parameters.DP = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "DQ": parameters.DQ = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "InverseQ": parameters.InverseQ = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                        case "D": parameters.D = (string.IsNullOrEmpty(node.InnerText) ? null : Convert.FromBase64String(node.InnerText)); break;
                    }
                }
            }
            else
            {
                throw new Exception("Invalid XML RSA key.");
            }

            rsa.ImportParameters(parameters);
        }

        public static string ToXmlString(this RSA rsa, bool includePrivateParameters)
        {
            RSAParameters parameters = rsa.ExportParameters(includePrivateParameters);

            return string.Format("<RSAKeyValue><Modulus>{0}</Modulus><Exponent>{1}</Exponent><P>{2}</P><Q>{3}</Q><DP>{4}</DP><DQ>{5}</DQ><InverseQ>{6}</InverseQ><D>{7}</D></RSAKeyValue>",
                  parameters.Modulus != null ? Convert.ToBase64String(parameters.Modulus) : null,
                  parameters.Exponent != null ? Convert.ToBase64String(parameters.Exponent) : null,
                  parameters.P != null ? Convert.ToBase64String(parameters.P) : null,
                  parameters.Q != null ? Convert.ToBase64String(parameters.Q) : null,
                  parameters.DP != null ? Convert.ToBase64String(parameters.DP) : null,
                  parameters.DQ != null ? Convert.ToBase64String(parameters.DQ) : null,
                  parameters.InverseQ != null ? Convert.ToBase64String(parameters.InverseQ) : null,
                  parameters.D != null ? Convert.ToBase64String(parameters.D) : null);
        }

        #endregion
    }
}```

@timotei
Copy link

timotei commented Apr 22, 2018

@YetaWF The PR I did does/handles exactly the part you posted, so what exactly is missing?

@0x53A
Copy link

0x53A commented Jun 5, 2018

Is there any chance to fix this inside netcore? Even a hackish solution (reflection or a small one-off parser) would be preferably to PNSE ...

For first-party code, the workaround with the copy-pasted code is okayish, but I have the situation that a third-party dll (targeting net40) breaks because of this, making me trip on the first stone earlier than expected.

As I understand it, loading full-fx libraries from netcore is more-or-less supported, with the occasional PNSE.

It is kind of sad that such a "trivial" issue breaks this. (I know that the circular dependency isn't trivial from your perspective, but from the outside it is).

@danmoseley danmoseley reopened this Jun 5, 2018
@danmoseley
Copy link
Member Author

Reopening because discussion is continuing. @bartonjs we've broken similar circles in the past with reflection, as you mentioned above. Do you have any concerns with doing that? And if so which dependency would use reflection?

@bartonjs
Copy link
Member

ToXmlString can be written with string.Format, so that part's fine. FromXmlString will need to reflect into an XML library.

@SteveKennedy
Copy link

The fact that I can create a brand new .NET 2 application, then get an intellisense suggestion to use rsaKey.ToXmlString(), but it immediately throws a PlatformNotSupported exception is nuts.

@CreepyGnome
Copy link

Wait this is only going to be fixed in 3.0? This seems like it should be patched back into 2.x at least 2.2.x Seems safe to patch back implementations like this. I mean hacking the extension methods into the code kind of sucks.

@bartonjs
Copy link
Member

This seems like it should be patched back into 2.x at least 2.2.x

2.1, as LTS, effectively only gets security fixes and fixes required to keep the product stable as OSes change out from under it.

2.2 goes out of support in December.

@CreepyGnome
Copy link

CreepyGnome commented Jul 11, 2019

So you are saying there is plenty of time to get the fix into 2.2!! AWESOME!! Let's Do It!

So Maybe should be re-opened so that can be done. As more people will be using 2.2 for a while before they jump to 3.0 and 2.0/2.1 people may be more likely to jump to 2.2 for a fix like this than to go to 3.0 for an already released application.

@bartonjs
Copy link
Member

@CreepyGnome In general, we don't backport features. But, I'll ask:

@danmosemsft Since you'd be part of the approval process and you were pushing (or, at least, nudging) to get this fixed for 3.0, wanna weigh in?

@danmoseley
Copy link
Member Author

@CreepyGnome as @bartonjs says, we aren't planning to back port any features. 3.0 release is not far off now and previews are available to use today.

@CreepyGnome
Copy link

So @danmosemsft what you are saying is that 2.2 is not going to get ANY features backported, which would be the only type of features it could get since 3.0 is on the way soon, which means it is not really in typical full support until December as previously stated, but actually in security only fix mode like LTS.

If a fully support version will not get new features it's not really fully supported anymore. That's sad to hear. I am sure I can just go to my CTO and say we have to upgrade everything to 3.0 the day it comes out and they will be "yep, of course, let's stop everything and do that right now!"

Also sad to hear that you assume people will just move all their existing 2 core code to 3.0 just because of 3.0 being released. SO I assume you have almost no customers on .NET Core 1.x, 2.0, and 2.1 right? Because as you make it seem they all must have upgraded right away to 2.2 when it was released as you seem to be assuming that putting features in to 3.0 and never backporting anything is pointless, and the only reason I can think that it would be pointless is if everyone upgrades right away and you never have any enterprise customers lingering for years on an old version, or only willing to upgrade to minor releases due to an enterprise policy about upgrading.

I wish I could live in that dream world you seem to, as people upgrading enterprise software and services to the latest versions of frameworks seems like it would be nice.

@danmoseley
Copy link
Member Author

@CreepyGnome I hear you. I believe that's actually the policy we have always had, of making bug fixes to released products and focusing feature development on the upcoming release rather than dividing resources between them. @terrajobst do you have more context to add here?

@CreepyGnome
Copy link

@danmosemsft My understanding of the official support policy for .NET Core seems to state fairly clearly that the window is open to still add features to 2.2.

(https://dotnet.microsoft.com/platform/support/policy/dotnet-core)

Current releases will receive these same fixes and will also be updated with compatible innovations and features.
Updates may include new features, fixes (security and/or non-security), or a combination of both.

Emphasis add

It clearly states that FEATURES and compatible innovations are not closed on the CURRENT release. The current release is .NET Core 2.2. which has support end in Dec 2019 BUT will no longer be current when .NET Core 3.0 is released, so until it is no longer current, it seems clear that it is open to features in its updates.

I know you phrased it as your teams are not currently planning anything, however, the window is officially open to PLAN and to ADD features. If the window is officially closed you should change the support level from Current to EOL. Or provide another date to add another tier for features like no new features or innovations 4 months before the next current release or something like that. Or just have a column and don't add the date for it until you are 8 months out and put the date 4 months from that.

Anyway, there are ways to make sure people understand features are closed in a CURRENT version by updating the Official .NET Core Support Policy.

Thanks for your time on this though I do appreciate being heard on this even though nothing will change.

@terrajobst
Copy link
Member

terrajobst commented Jul 12, 2019

@CreepyGnome

I think you're reading the policy slightly wrong. .NET Core 2.2 is a current release. The wording you quoted is contrasting LTS to CURRENT. And yes, CURRENT may add features. However, just because we shipped a .NET Core 2.2 doesn't mean we will backport all new features from .NET Core 3.0 to .NET Core 2.2 or that there will be .NET Core 2.3. This wouldn't make any sense.

That being said, you are correct that since .NET Core 2.2 is only going out of support in December, the window for changes is still open. However, that doesn't mean that any change is fair game. As it is for shipped release, there is a bar. Not all changes will make that bar (and new features/APIs are generally out of scope for servicing; those would default to new minor- or major versions).

Right now, the question we have to decide on is whether your requested change makes that bar. Technically, it's not a bug (it's a missing feature) and given the API usage numbers (~5% according to API Port telemetry) this doesn't seem like a widespread issue to me. Besides, if you're on .NET Core 2.2 it would only buy your time until December. You'd soon have to move to .NET Core 3.0 if you wanted to stay supported which devalues this change for 2.2 even more in my eyes.

@danmosemsft @bartonjs what are your thoughts on the bar?

@danmoseley
Copy link
Member Author

@danmosemsft @bartonjs what are your thoughts on the bar?

As mentioned above, as I understand it this seems to me this is not impactful enough to warrant servicing existing product for because it is not severe, there are partial workarounds and it would only be useful for a few months.

@bartonjs
Copy link
Member

@danmosemsft @bartonjs what are your thoughts on the bar?

My neural net says that this is a feature and that our servicing bar doesn't allow backporting features (except for things like enabling new TLS versions).

If the bar was that 2.2 could get it but 2.1 couldn't, I'd say that at this point it doesn't matter enough (I don't think it'd make it into a 2.2 release before September, at which point 3.0 is right on its heels and 2.2's EoL is approaching).

For the specific feature, I'd say that ToXmlString(true) is a bad habit to be in, ToXmlString(false) and the public-key-only FromXmlString are easy enough to write in terms of ExportParameters(false) (and ImportParameters) that there's not a strong need... but it does affect port-to-core... which is why it made it for 3.0.

So: I don't think I could make a convincing argument for why it needs to be backported, but I also wouldn't say that backporting it was a bad idea. (Yeah, this is a very long winded ::shrug:: "I dunno...")

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 3.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 20, 2020
@bartonjs bartonjs removed their assignment Jul 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging a pull request may close this issue.