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

Add StrongName signing to managed assemblies #29

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
@LinglingTong
Copy link
Member

LinglingTong commented Nov 13, 2015

No description provided.

@msftclas

This comment has been minimized.

Copy link

msftclas commented Nov 13, 2015

Hi @LinglingTong, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Lingling Tong). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@mgoertz-msft

This comment has been minimized.

Copy link
Member

mgoertz-msft commented Nov 13, 2015

We should only have a single SNK file that all projects use. Perhaps under src\shared or something like that.

@brianlagunas

This comment has been minimized.

Copy link
Collaborator

brianlagunas commented Nov 13, 2015

We shouldn't check in the SNK file to the repo. There should be a private distribution of that key.

@brianlagunas

This comment has been minimized.

Copy link
Collaborator

brianlagunas commented Nov 13, 2015

Also keep in mind, if there are any 3rd party vendors using an existing version of the behaviors in their distributed products, they will have to re-reference, rebuild, and then re-release their product or else it will be a breaking change.

@brianlagunas

This comment has been minimized.

Copy link
Collaborator

brianlagunas commented Nov 13, 2015

We do have issue #24 that we are still reviewing that will address this. Please monitor this issue for any updates.

@mgoertz-msft

This comment has been minimized.

Copy link
Member

mgoertz-msft commented Nov 13, 2015

That's to be expected. This is a new Behaviors SDK for UWP.

As far as the SNK is concerned, our stance on strong naming and how it is not a security feature is documented on MSDN: https://msdn.microsoft.com/en-us/library/wd40t7ad(v=vs.110).aspx

Specifically in these two comments:
"Do not rely on strong names for security. They provide a unique identity only."

"If you are an open-source developer and you want the identity benefits of a strong-named assembly, consider checking in the private key associated with an assembly into your source control system."

@mgoertz-msft mgoertz-msft reopened this Nov 13, 2015

@msftclas

This comment has been minimized.

Copy link

msftclas commented Nov 13, 2015

Hi @LinglingTong, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Lingling Tong). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@brianlagunas

This comment has been minimized.

Copy link
Collaborator

brianlagunas commented Nov 13, 2015

By checking in the key, other assemblies can impersonate this project. I would never recommend checking in the key even for OSS. We do want enterprises to use these behaviors.

@mgoertz-msft

This comment has been minimized.

Copy link
Member

mgoertz-msft commented Nov 13, 2015

Well, those are our official Microsoft guidelines. Whenever we create an official release though we will Authenticode sign all of the included binaries with an official Microsoft certificate. Does that address your concerns?

@brianlagunas

This comment has been minimized.

Copy link
Collaborator

brianlagunas commented Nov 13, 2015

Authenticode will validate these assemblies, but other malicious assemblies could be strong signed with the key in the repo. I don't see the benefit of checking in the key in the first place. If someone is doing a custom build of the project, they won't have the authenticated version so they will still have to reference their own build which means they could use their own key if they need strong naming.

@LinglingTong LinglingTong deleted the dev/ltong/sn branch Nov 16, 2015

@PedroLamas

This comment has been minimized.

Copy link
Collaborator

PedroLamas commented Dec 7, 2015

Though I understand and agree with the reasoning for authenticode, I have a hard time understanding why you decided to add a strong name to the Xaml Behaviors assemblies!

I might be opening quite a can of worms here (this is a long debated subject) but given that the Xaml Behaviors provides base classes for other libraries to use (such as Cimbalino Toolkit) I see the usage of strong naming as a storm waiting to happen!

As far as I know, the main reason to SN an assembly would be to allow it to be hosted in the GAC, thus providing "side by side" versioning... but the thing is that the GAC is dead!!

Plus, the Xaml Behaviors are targeting Windows 10 apps which completely ignore what the GAC is, so whats the point?

@jozefizso

This comment has been minimized.

Copy link

jozefizso commented Dec 7, 2015

@PedroLamas classic Desktop applications can be strong named so to reference Xaml Behaviors it must be strong named too.

The opposite is not required. Cimbalino does not have to be strong named to reference Xaml Behaviors.

@ScottIsAFool

This comment has been minimized.

Copy link
Contributor

ScottIsAFool commented Dec 7, 2015

Not to be too blunt, but why do we care about classic Desktop applications? Isn't this library for UWP?

@PedroLamas

This comment has been minimized.

Copy link
Collaborator

PedroLamas commented Dec 7, 2015

@jozefizso I'm not sure you're following the whole problem here!

Let's say that your app requires both Cimbalino Toolkit and Caliburn Micro.

If Cimbalino requires Xaml Behaviors v1.0 while Caliburn requires v1.1... you have a serious problem because (AFAIK) you can't redirect the assembly bindings in UWP and you can't have both assemblies referenced!

But in any case, why do you care about desktop apps? Isn't this for UWP apps only?

@caioproiete

This comment has been minimized.

Copy link

caioproiete commented Dec 7, 2015

Is having strong name really a requirement for OSS nowadays? (considering the pain that it brings w/ binding redirects, etc.)

I thought it was a solved problem with things like StrongNamer and similar. If an enterprise needs to use an OSS library and needs that library to be signed, they can just sign it during the build process.

Why put the burden of strong name to everyone else, because of the few that actually need it?

Worst case, you could follow what some OSS projects do and release two NuGet packages (with different package IDs), one with no strong name (XamlBehaviors), and another one with strong name (XamlBehaviors.StrongNamed)

@PedroLamas PedroLamas referenced this pull request Dec 7, 2015

Open

Signing #24

@dsplaisted

This comment has been minimized.

Copy link
Member

dsplaisted commented Dec 9, 2015

@brianlagunas wrote:

Authenticode will validate these assemblies, but other malicious assemblies could be strong signed with the key in the repo. I don't see the benefit of checking in the key in the first place. If someone is doing a custom build of the project, they won't have the authenticated version so they will still have to reference their own build which means they could use their own key if they need strong naming.

I think you're underestimating the pain that can be caused when it's not possible to build your own version of an OSS library with the same strong name as the "official" version. All assemblies that reference the library would need to be rebuilt. So if you wanted to fix a bug in XamlBehaviors, you would end up needing to also build your own versions of any third-party libraries that depend on it.

@PedroLamas

This comment has been minimized.

Copy link
Collaborator

PedroLamas commented Dec 9, 2015

@dsplaisted been there... and that's why I'd prefer to get this not strongly signed!

@paulcbetts

This comment has been minimized.

Copy link

paulcbetts commented Dec 9, 2015

By checking in the key, other assemblies can impersonate this project. I would never recommend checking in the key even for OSS. We do want enterprises to use these behaviors.

@brianlagunas This is a common misconception, but it's incorrect. Strong naming is not a security feature. Authenticode is the solution for preventing impersonation, Strong Naming is not. Microsoft should Authenticode sign the Official Version.

I won't weigh in on whether or not to SN, but if you do SN and you don't check in the key, you're both not Free Software (because users can't drop in their own binary), and from a practical perspective, you're causing a lot of difficulties to other developers and the .NET Community for no benefit.

@brianlagunas

This comment has been minimized.

Copy link
Collaborator

brianlagunas commented Dec 9, 2015

@dsplaisted Good point. Didn't consider that. Signing adds a lot of extra headaches in a number of different scenarios.

@paulcbetts I disagree with your perspective of not being free software, but you also make a good point.

@paulcbetts

This comment has been minimized.

Copy link

paulcbetts commented Dec 9, 2015

@brianlagunas It's not just my perspective, it's the Free Software Foundation's perspective: https://en.wikipedia.org/wiki/Tivoization - when you hide your SN key, users can no longer compile a version of the library that works with anyone else's binaries.

@brianlagunas

This comment has been minimized.

Copy link
Collaborator

brianlagunas commented Dec 9, 2015

@paulcbetts Well, I still don't agree with that. Just because you can't compile with an SN so that it works with anyone's binaries doesn't mean the software isn't free. It's just a pain in the ass to use when using a third party that is using a different version. You can still download the code and use it as you see fit freely.

@vbandi

This comment has been minimized.

Copy link

vbandi commented Jan 21, 2016

I'm trying to compose a complex Pull request, but strong names make compiling, design time and pretty much everything very hard. :(

@PedroLamas

This comment has been minimized.

Copy link
Collaborator

PedroLamas commented Jan 21, 2016

@vbandi I'm still struggling with what are we trying to fix by strong naming these assemblies...

@brianlagunas

This comment has been minimized.

Copy link
Collaborator

brianlagunas commented Jan 22, 2016

Yup, strong naming definitely a pain in the ass. I can't even get a build to pass:

Severity Code Description Project File Line Suppression State
Error Loading assembly "C:\Users\username\Documents\GitHub\XamlBehaviors\out\BehaviorsSDKManaged\bin\AnyCPU\Release\Microsoft.Xaml.Interactivity.dll" failed. System.IO.FileLoadException: Could not load file or assembly 'Microsoft.Xaml.Interactivity, Version=1.0.3.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' or one of its dependencies. Strong name validation failed. (Exception from HRESULT: 0x8013141A)
File name: 'Microsoft.Xaml.Interactivity, Version=1.0.3.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35' ---> System.Security.SecurityException: Strong name validation failed. (Exception from HRESULT: 0x8013141A)

The Zone of the assembly that failed was:
MyComputer
at System.Reflection.RuntimeAssembly._nLoad(AssemblyName fileName, String codeBase, Evidence assemblySecurity, RuntimeAssembly locationHint, StackCrawlMark& stackMark, IntPtr pPrivHostBinder, Boolean throwOnFileNotFound, Boolean forIntrospection, Boolean suppressSecurityChecks)
at System.Reflection.RuntimeAssembly.nLoad(AssemblyName fileName, String codeBase, Evidence assemblySecurity, RuntimeAssembly locationHint, StackCrawlMark& stackMark, IntPtr pPrivHostBinder, Boolean throwOnFileNotFound, Boolean forIntrospection, Boolean suppressSecurityChecks)
at System.Reflection.RuntimeAssembly.InternalLoadAssemblyName(AssemblyName assemblyRef, Evidence assemblySecurity, RuntimeAssembly reqAssembly, StackCrawlMark& stackMark, IntPtr pPrivHostBinder, Boolean throwOnFileNotFound, Boolean forIntrospection, Boolean suppressSecurityChecks)
at System.Reflection.RuntimeAssembly.InternalLoadFrom(String assemblyFile, Evidence securityEvidence, Byte[] hashValue, AssemblyHashAlgorithm hashAlgorithm, Boolean forIntrospection, Boolean suppressSecurityChecks, StackCrawlMark& stackMark)
at System.Reflection.Assembly.UnsafeLoadFrom(String assemblyFile)
at Microsoft.Build.Tasks.ProcessResourceFiles.ReadAssemblyResources(String name, String outFileOrDir) XAMLBehaviorsSample

@JerryNixon

This comment has been minimized.

Copy link

JerryNixon commented Apr 15, 2016

So what's the result here. Back to useful or stay the path?

@callummoffat

This comment has been minimized.

Copy link

callummoffat commented Apr 28, 2016

This change is causing issues with Template 10. Many projects are broken (although there is a workaround, it requires turning off design mode data) as a result of this change.

Could you consider reverting this change? There are other ways of increasing security that don't involve breaking countless apps and libraries.

@mgoertz-msft

This comment has been minimized.

Copy link
Member

mgoertz-msft commented Apr 28, 2016

I'm confused. Do you want strong-name signing on or off? The latest release of the XAML Behaviors (1.1.0) has strong-name signing removed.

@callummoffat

This comment has been minimized.

Copy link

callummoffat commented Apr 29, 2016

I'd like it turned off. Something in Behaviors 1.1.0 is causing issues for Template 10 apps, and it's a strong named signing error. Makes using the designer completely impossible. :-(

@jonwchu

This comment has been minimized.

Copy link
Member

jonwchu commented Apr 29, 2016

@callummoffat The latest version of Behaviors (1.1.0) already has strong name signing turned off. Initially, there was an issue with Template 10 not working with the new version, but that has since been fixed. Is your version of Template 10 the latest?

@callummoffat

This comment has been minimized.

Copy link

callummoffat commented Apr 29, 2016

I've got the latest (non-preview) version available.

@callummoffat

This comment has been minimized.

Copy link

callummoffat commented Apr 29, 2016

The problem appears to be that Template 10 turned on strong naming in the latest release, while Behaviors has turned it off.

@PedroLamas

This comment has been minimized.

Copy link
Collaborator

PedroLamas commented Apr 29, 2016

The problem appears to be that Template 10 turned on strong naming in the latest release

Then I'd say that this is an issue more related with Template 10 than with the Behaviors! @JerryNixon should be able to help with this! :)

@JerryNixon

This comment has been minimized.

Copy link

JerryNixon commented May 11, 2016

Template 10 also is no longer strong-named.

@PedroLamas

This comment has been minimized.

Copy link
Collaborator

PedroLamas commented May 11, 2016

@JerryNixon
Well done!

@JerryNixon

This comment has been minimized.

Copy link

JerryNixon commented May 11, 2016

Nerd

@PedroLamas

This comment has been minimized.

Copy link
Collaborator

PedroLamas commented May 11, 2016

I do try! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment