Reference source for System.Security.Cryptography is generated inconsistently between platforms

[vcsjones]

Somewhat recently, it appears that the GenerateReferenceAssemblySource is creating changes that are invalid, or at least unwanted.

On macOS or Linux, do:

# Tried on 2737da5
./dotnet.sh build src/libraries/System.Security.Cryptography/src /t:GenerateReferenceAssemblySource

on macOS, this generates a lot of changes that are incorrect. A diff can be seen at main...vcsjones:diff-ssc

Notable things that are incorrect:

1. Removing many overrides from AesCng, for example TryDecryptCbcCore. AesCng does override it.
2. Removing IDisposable from CngKey.

This behavior only appears in macOS / Linux. On Windows, its results are reasonable, simple ordering or whitespace changes.

/cc @bartonjs @ViktorHofer

Tagging subscribers to this area: @dotnet/area-infrastructure-libraries
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Somewhat recently, it appears that the GenerateReferenceAssemblySource is creating changes that are invalid, or at least unwanted.

On macOS or Linux, do:

# Tried on 2737da5
./dotnet.sh build src/libraries/System.Security.Cryptography/src /t:GenerateReferenceAssemblySource

on macOS, this generates a lot of changes that are incorrect. A diff can be seen at main...vcsjones:diff-ssc

Notable things that are incorrect:

1. Removing many overrides from AesCng, for example TryDecryptCbcCore. AesCng does override it.
2. Removing IDisposable from CngKey.

This behavior only appears in macOS / Linux. On Windows, its results are reasonable, simple ordering or whitespace changes.

/cc @bartonjs

Author:	vcsjones
Assignees:	-
Labels:	area-Infrastructure-libraries
Milestone:	-

[ViktorHofer]

Taking a look now

[ViktorHofer]

Just verified that 7667e41 is unrelated and not causing the diff.

[ViktorHofer]

This doesn't seem to be a tooling issue but a code issue. As an example AesCng.Key is missing from the Unix implementations:

runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/Cng.NotSupported.cs

Line 70 in bc5e386

public sealed partial class AesCng : Aes

@ericstj @joperezr It's concerning that ApiCompat doesn't flag the missing overrides and the missing interface

[vcsjones]

I think you are right. The ref file is "wrong" on Windows, too. When I generate the ref on Linux, I get a bunch of overrides added for RSAOpenSsl, a "Linux / macOS only" type.

Link because GitHub is doing weird things to diff links

The NotSupported RSAOpenSsl is missing those overrides.

runtime/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/OpenSsl.NotSupported.cs

Line 180 in 466b0f6

public sealed class RSAOpenSsl : RSA

I'm guessing the ref file was generated on Windows, which is why running it on Windows produces a "cleaner" diff, but running it on Linux is producing needed things, too.

Tagging subscribers to this area: @dotnet/area-system-security, @vcsjones
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Somewhat recently, it appears that the GenerateReferenceAssemblySource is creating changes that are invalid, or at least unwanted.

On macOS or Linux, do:

# Tried on 2737da5
./dotnet.sh build src/libraries/System.Security.Cryptography/src /t:GenerateReferenceAssemblySource

on macOS, this generates a lot of changes that are incorrect. A diff can be seen at main...vcsjones:diff-ssc

Notable things that are incorrect:

1. Removing many overrides from AesCng, for example TryDecryptCbcCore. AesCng does override it.
2. Removing IDisposable from CngKey.

This behavior only appears in macOS / Linux. On Windows, its results are reasonable, simple ordering or whitespace changes.

/cc @bartonjs @ViktorHofer

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

[ViktorHofer]

I think you are right. The ref file is "wrong" on Windows, too. When I generate the ref on Linux, I get a bunch of overrides added for RSAOpenSsl, a "Linux / macOS only" type.

It's important that the public API matches between different runtimes. There must not be a difference between Unix and Windows. That's one of the purposes of the ApiCompat tooling which runs as part of a library's build (even though it didn't catch these cases). Presumably the right next step would be to make sure that the API matches and investigate why ApiCompat doesn't flag the differences.

[vcsjones]

I can fix the differences but I'm worried we are going to drift again if ApiCompat doesn't warn us of this. Should I open a separate issue for that?

[vcsjones]

@ViktorHofer Looking at this a little bit more that I understand what is going on, I don't quite see why CngKey is getting Disposable removed. Every implementation of CngKey that I can see implements IDisposable.