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

Preview 5 has two identical copies of System.Security.Cryptography.Cng.dll #54427

Closed
ericsink opened this issue Jun 18, 2021 · 11 comments · Fixed by #54428
Closed

Preview 5 has two identical copies of System.Security.Cryptography.Cng.dll #54427

ericsink opened this issue Jun 18, 2021 · 11 comments · Fixed by #54428

Comments

@ericsink
Copy link

In .NET 6 preview 5, there are identical copies of System.​Security.​Cryptography.​Cng.dll in two different targeting packs, one in Microsoft.​NETCore.​App.Ref and another in Microsoft.​AspNetCore.​App.Ref.

I first mentioned this in a Tweet:

https://twitter.com/eric_sink/status/1405727524194242566

to which @davidfowl replied "That sounds like a bug...", so I figured it should be logged, and hopefully this is the right place.

@dotnet-issue-labeler
Copy link

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.

@davidfowl
Copy link
Member

cc @dougbu @ericstj

@ericstj
Copy link
Member

ericstj commented Jun 18, 2021

Confirmed this is the case. @dougbu, this needs to be removed from ASP.NET ref-pack. Looks like it's no longer in runtime pack/shared-fx but still present in ref-pack.

@ericstj ericstj transferred this issue from dotnet/sdk Jun 18, 2021
@dougbu
Copy link
Member

dougbu commented Jun 18, 2021

Was this assembly added to Microsoft.NETCore.App.* while we weren't looking❔ We've long (at least since 3.1.0) had a weird case where Cryptography.Cng was in our targeting packs but not our runtimes (and visa-versa for Cryptography.Pkcs).

The Microsoft.NETCore.App.Ref content changed between preview 4 and 5, adding Cryptography.Cng (and Cryptograph.OpenSsl). Was that change intentional @ericstj❔ If it matters, I don't see any changes in that period between the runtimes.

@dougbu
Copy link
Member

dougbu commented Jun 18, 2021

In any case, something odd is happening.

https://github.com/dotnet/aspnetcore/blob/1ead7696c823ca9d96a7bea65760d7fc03f375ce/src/Framework/App.Ref/src/Microsoft.AspNetCore.App.Ref.csproj#L147

in our App.Ref exclusions should mean we don't duplicate anything from Microsoft.NETCore.App.Ref in our Microsoft.AspNetCore.App.Ref.

@dougbu
Copy link
Member

dougbu commented Jun 18, 2021

@ericstj if Cryptography.Cng was intentionally added to Microsoft.NETCore.App.Ref, the bug here is leaving System.Security.Cryptography.Cng in Microsoft.AspNetCore.Internal.Transport. Our repo expects the transport package to contain only additional ref/ assemblies we need. The duplication between Microsoft.AspNetCore.Internal.Transport and Microsoft.NETCore.App.Ref can't be correct and special-casing it to avoid this bug seems Just Wrong:tm:

(Side note: We still don't need implementation assemblies in Microsoft.AspNetCore.Internal.Transport)

@ericstj
Copy link
Member

ericstj commented Jun 18, 2021

Yes it was exposed with #51853. Cc @ViktorHofer

@ericstj
Copy link
Member

ericstj commented Jun 18, 2021

To remove from the transport package we need to remove

@ericstj ericstj transferred this issue from dotnet/aspnetcore Jun 18, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Security untriaged New issue has not been triaged by the area owner labels Jun 18, 2021
@ghost
Copy link

ghost commented Jun 18, 2021

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

Issue Details

In .NET 6 preview 5, there are identical copies of System.​Security.​Cryptography.​Cng.dll in two different targeting packs, one in Microsoft.​NETCore.​App.Ref and another in Microsoft.​AspNetCore.​App.Ref.

I first mentioned this in a Tweet:

https://twitter.com/eric_sink/status/1405727524194242566

to which @davidfowl replied "That sounds like a bug...", so I figured it should be logged, and hopefully this is the right place.

Author: ericsink
Assignees: -
Labels:

area-System.Security, untriaged

Milestone: -

@dougbu
Copy link
Member

dougbu commented Jun 18, 2021

I like one line fixes 😃

@ericstj ericstj added area-Infrastructure-libraries and removed area-System.Security untriaged New issue has not been triaged by the area owner labels Jun 18, 2021
@ericstj ericstj self-assigned this Jun 18, 2021
@ghost
Copy link

ghost commented Jun 18, 2021

Tagging subscribers to this area: @Anipik, @safern, @ViktorHofer
See info in area-owners.md if you want to be subscribed.

Issue Details

In .NET 6 preview 5, there are identical copies of System.​Security.​Cryptography.​Cng.dll in two different targeting packs, one in Microsoft.​NETCore.​App.Ref and another in Microsoft.​AspNetCore.​App.Ref.

I first mentioned this in a Tweet:

https://twitter.com/eric_sink/status/1405727524194242566

to which @davidfowl replied "That sounds like a bug...", so I figured it should be logged, and hopefully this is the right place.

Author: ericsink
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 18, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 19, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants