Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Stop signing apphost.exe and comhost.dll in nupkg#7547

Merged
dagood merged 2 commits intodotnet:masterfrom
dagood:stop-signing-templates
Aug 2, 2019
Merged

Stop signing apphost.exe and comhost.dll in nupkg#7547
dagood merged 2 commits intodotnet:masterfrom
dagood:stop-signing-templates

Conversation

@dagood
Copy link
Member

@dagood dagood commented Aug 2, 2019

The nupkg is recursively signed, so leaving apphost.exe and comhost.dll off the list of files to sign is not enough, and they still end up signed. We also need to set up FileSignInfo to tell recursive signing what to do.

@dagood dagood self-assigned this Aug 2, 2019
It shouldn't be limited to binaries signing stage. It needs to be active during SignFinalPackages, but it might as well be set during any signing to match intent.
Copy link
Member

@vitek-karas vitek-karas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will need to port this to release/3.0 and probably communicate this broadly - as it's broken in P8.

@dagood
Copy link
Member Author

dagood commented Aug 2, 2019

What breaks when these files are signed? Could you maybe file a issue with details to start with?

(I'm happy to create the port PR to 3.0.)

@dagood
Copy link
Member Author

dagood commented Aug 2, 2019

I'm going to go ahead and merge: this has no affect on PR validation builds, and I want to see it in action in an official build.

@vitek-karas
Copy link
Member

@elinor-fung can you please file the issue - since you played with it in detail.

The broken scenario is: Build an app with app.exe - try to sign the app.exe with my own cert - fails.

@dagood dagood merged commit 0245522 into dotnet:master Aug 2, 2019
@dagood dagood deleted the stop-signing-templates branch August 2, 2019 20:35
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants