nhibernate / nhibernate-core

NHibernate Object Relational Mapper
https://nhibernate.info
GNU Lesser General Public License v2.1
2.11k stars 921 forks source link

BinaryFormatter is obsolete in AspNet Core in .net5.0 #2603

Open danielegiallonardo opened 3 years ago

danielegiallonardo commented 3 years ago

AspNet Core in .net5.0 applications won't start since BinaryFormatter use is prohibited, as stated here. Please consider replacing BinaryFormatter with something "AspNet Core in 5.0" friendly. It's something AspNet Core related, since .net5.0 console applications don't have such an issue.

bahusoid commented 3 years ago

It's something AspNet Core related, since .net5.0 console applications don't have such an issue.

Well according to docs you mentioned you can still use it with ASP.NET. You just need to update your csproj file:

To continue using BinaryFormatter in ASP.NET apps, you can re-enable it in the project file.

<PropertyGroup>
<TargetFramework>net5.0</TargetFramework>
<!-- Warning: Setting the following switch is *NOT* recommended in web apps. -->
<EnableUnsafeBinaryFormatterSerialization>true</EnableUnsafeBinaryFormatterSerialization>
</PropertyGroup>
danielegiallonardo commented 3 years ago

Yes, they've introduced that tag in order for legacy applications to still run until all the references to BinaryFormatter in the depedencies are replaced, but it's still something they strongly DON'T recommend, since BinaryFormatter has security issues and it has been marked as deprecated in .net5+. In order to the library to be fully .Net5+-compliant, all the references to BinaryFormatter should be replaced with one of the suggested alternatives.

hazzik commented 3 years ago

BinaryFormatter should be replaced with one of the suggested alternatives.

It could be replaced with one of the alternatives.

NHibernate uses BinaryFormatter in just two places: SerializableType and SerializationHelper. It is totally safe to enable BinaryFormatter if you are not using any of these features.

danielegiallonardo commented 3 years ago

I know that a workaround exists, and I'm currently forced to adopt it, but are you suggesting that I should disable (for a long-term) an application-wide block of a high-risk OWASP reported vulnerability, implemented in a core class that is reported as deprecated (and I expect it to be removed in future versions of the .net sdk)? Anyway, I'm not asking for a hot-fix, I'm just reporting an existing issue about NHibernate compatibiliy with .net5+.

fredericDelaporte commented 3 years ago

Do we actually need to disable that block for running NHibernate on .Net 5? I think it is not even needed.

The application will run just fine provided it does not use the few features of NHibernate which use BinaryFormatter. If it uses it, it will raise a NotImplementedException according to your link, and so you will be easily able to detect the offending part in your application for ceasing to use the few features relying on BinaryFormatter.

So I do not see a reason for enabling binary formatter in .Net 5 if your code does not use any feature requiring binary serialization. That should not prevent using NHibernate at all, since it seems to be a runtime check triggered only on actual use of a binary formatter.

danielegiallonardo commented 3 years ago

Unfortunately the asp.net app won't start at all, whether you use a feature requiring binary serialization or not (BTW, I don't use it), throwing the following exception (an InnerException thrown inside a FluentConfigurationException) when calling BuildConfiguration():

"NotSupportedException: BinaryFormatter serialization and deserialization are disabled within this application. See https://aka.ms/binaryformatter for more information."

I suppose the only fact that you reference (somewhere in your code) the BinaryFormatter class causes the app to block at the startup.

bahusoid commented 3 years ago

I suppose the only fact you reference (somewhere in your code) the BinaryFormatter class causes the app to block at the startup.

I doubt it. Can you share full stack trace? I suspect it's FluentNhibernate to blame and BinaryFormatter is actually used there.

danielegiallonardo commented 3 years ago

Ok, it seems you're right. Here's the full stacktrace.

FluentNHibernate.Cfg.FluentConfigurationException: An invalid or incomplete configuration was used while creating a SessionFactory. Check PotentialReasons collection, and InnerException for more detail.

 ---> System.NotSupportedException: BinaryFormatter serialization and deserialization are disabled within this application. See https://aka.ms/binaryformatter for more information.
   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Serialize(Stream serializationStream, Object graph)
   at FluentNHibernate.Utils.Extensions.DeepClone[T](T obj)
   at FluentNHibernate.Mapping.SubclassMap`1.FluentNHibernate.Mapping.Providers.IIndeterminateSubclassMappingProvider.GetSubclassMapping(SubclassType type)
   at FluentNHibernate.Visitors.SeparateSubclassVisitor.ProcessClass(ClassMapping mapping)
   at FluentNHibernate.MappingModel.ClassBased.ClassMapping.AcceptVisitor(IMappingModelVisitor visitor)
   at FluentNHibernate.Visitors.DefaultMappingModelVisitor.Visit(ClassMapping classMapping)
   at FluentNHibernate.MappingModel.HibernateMapping.AcceptVisitor(IMappingModelVisitor visitor)
   at FluentNHibernate.Visitors.DefaultMappingModelVisitor.<Visit>b__10_0(HibernateMapping x)
   at FluentNHibernate.Utils.CollectionExtensions.Each[T](IEnumerable`1 enumerable, Action`1 each)
   at FluentNHibernate.Visitors.DefaultMappingModelVisitor.Visit(IEnumerable`1 mappings)
   at FluentNHibernate.PersistenceModel.ApplyVisitors(IEnumerable`1 mappings)
   at FluentNHibernate.PersistenceModel.BuildMappings()
   at FluentNHibernate.PersistenceModel.EnsureMappingsBuilt()
   at FluentNHibernate.PersistenceModel.Configure(Configuration cfg)
   at FluentNHibernate.Cfg.FluentConfiguration.BuildConfiguration()
   --- End of inner exception stack trace ---
   at FluentNHibernate.Cfg.FluentConfiguration.BuildConfiguration()
oyzar commented 3 years ago

While FluentNhibernate's use of BinaryFormater is more obvious as it fails on startup, the usages in NHibernate should still be changed. It's used in 5 places: NHibernate.Type.SerializableType.FromBytes NHibernate.Type.SerializableType.ToBytes NHibernate.Util.SerializationHelper.CreateFormatter NHibernate.Util.SerializationHelper.Deserialize NHibernate.Util.SerializationHelper.Serialize

danielegiallonardo commented 3 years ago

Here is the same issue in the FluentNHibernate repo: https://github.com/nhibernate/fluent-nhibernate/issues/479 I think we can close here or keep it open for the NHibernate-Core parts. Thanks

gokhanabatay commented 3 years ago

I have the same issue with .net 5.0 NHibernate second level cache. What could we do except setting "EnableUnsafeBinaryFormatterSerialization=true" at project file?

at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Serialize(Stream serializationStream, Object graph)
   at NHibernate.Util.SerializationHelper.Serialize(Object obj)
   at NHibernate.Caches.Common.BinaryCacheSerializer.Serialize(Object value)
   at NHibernate.Caches.StackExchangeRedis.DistributedLocalCacheRegionStrategy.ExecuteOperation(String cacheKey, Object value, Int64 timestamp, Int32 clientId, Boolean publish, Boolean remove)
   at NHibernate.Caches.StackExchangeRedis.DistributedLocalCacheRegionStrategy.TryPutLocal(String cacheKey, Object value, Int64 timestamp, Int32 clientId, Boolean publish)
   at NHibernate.Caches.StackExchangeRedis.DistributedLocalCacheRegionStrategy.ExecutePut(String cacheKey, Object value)
   at NHibernate.Caches.StackExchangeRedis.AbstractRegionStrategy.Put(Object key, Object value)
   at NHibernate.Caches.StackExchangeRedis.DistributedLocalCacheRegionStrategy.ExecutePutMany(Object[] keys, Object[] values)
   at NHibernate.Caches.StackExchangeRedis.AbstractRegionStrategy.PutMany(Object[] keys, Object[] values)
   at NHibernate.Caches.StackExchangeRedis.RedisCache.PutMany(Object[] keys, Object[] values)
   at NHibernate.Cache.UpdateTimestampsCache.SetSpacesTimestamp(IReadOnlyCollection`1 spaces, Int64 ts)
   at NHibernate.Cache.UpdateTimestampsCache.PreInvalidate(IReadOnlyCollection`1 spaces)
   at Framework.Core.DataAccess.Cache.UpdateTimestampsCache.PreInvalidate(IReadOnlyCollection`1 spaces) in ...
   at NHibernate.Engine.ActionQueue.PreInvalidateCaches()
   at NHibernate.Engine.ActionQueue.ExecuteActions()
   at NHibernate.Event.Default.AbstractFlushingEventListener.PerformExecutions(IEventSource session)
   at NHibernate.Event.Default.DefaultFlushEventListener.OnFlush(FlushEvent event)
   at NHibernate.Impl.SessionImpl.Flush()
   at Framework.Core.DataAccess.UnitOfWork.UnitOfWorkProvider.Commit() in ...
   at Framework.Core.DataAccess.UnitOfWork.UnitOfWorkProvider.Execute(UnitOfWorkAttribute unitOfWork, Action action, ILogger logger) in ...
   at Framework.Core.DataAccess.UnitOfWork.UnitOfWorkInterceptor.Intercept(IInvocation invocation) in ...
   at Castle.DynamicProxy.AbstractInvocation.Proceed()
   at Castle.Proxies.ICfgExchangeFunctionCodeDefServiceProxy.Merge(CfgExchangeFunctionCodeDef entity)
   at Framework.Facilities.Web.Api.Controller.BaseController`3.Update(TEntity entity) in ...
   at lambda_method329(Closure , Object , Object[] )
   at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.SyncActionResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeActionMethodAsync()
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeNextActionFilterAsync()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeInnerFilterAsync()
--- End of stack trace from previous location ---
   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextExceptionFilterAsync>g__Awaited|25_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)
fredericDelaporte commented 3 years ago

@gokhanabatay, this is not a NHibernate issue but a cache question, you should have opened your case there. But anyway binary serialization is only a default method of serialization for some caches. You can choose another cache implementation, or configure another serializer, depending on the actual cache you use. Just read their documentation: there is a cache.serializer setting for StackExchangeRedis, and a JsonSerializer is available in a NuGet package.

I have opened an adequate issue about this: nhibernate/NHibernate-Caches#91.

fredericDelaporte commented 3 years ago

@oyzar

the usages in NHibernate should still be changed.

Why? Can you elaborate on this?

SerializableType is meant to binary serialize, there is no point in "changing" it. No one forces you to use this type with NHibernate in your mappings. Just do not use it. We will obviously flag it as obsolete at some point like BinaryFormatter is, but that is all.

SerializationHelper is just an utility for those needing binary serialization. NHibernate does not use it directly. So that is the same: there is no point in changing it, just do not use it. And it will just be flagged obsolete too at some point, I guess when NHibernate will target directly .Net 5.

oyzar commented 3 years ago

It would be better to change to an implementation that doesn't use an unsafe part of the .net API. There are alternatives listed here: https://aka.ms/binaryformatter are none of those possible to use?

fredericDelaporte commented 3 years ago

There are not any such implementations, as you could infer by reading your link. The recommended alternatives are serialization to XML or JSON, neither of which are a binary serialization. The other alternatives are put in a Dangerous alternatives section, because they suffer the same threats than the BinaryFormatter.

Having some code referencing those unsafe types does not cause the project to be unsafe, since the default in .Net 5 will be to block the execution of those unsafe features. People trying to use them will get appropriate warning, and that will be their responsibility and freedom if they choose to use them and disable the block for doing this.

This change in .Net 5 means binary serialization should no more be used. The two types using it in NHibernate are to be used only by those needing binary serialization. So the only action we should take about them is to flag them as obsolete once targeting .Net 5, and eventually offer some alternative if needed.

By example, SerializableType is a fallback type when NHibernate encounters an entity class with a property having an unsupported type. Ending-up throwing a NotImplementedException in such case makes sense, but we could consider adding something like a JsonType and use it as a fallback instead of SerializableType. (Changing this fallback will be a possible breaking change.)

sirpenski commented 3 years ago

Stupid nanny state incels. Without the binary serialization, deserialization, you can't efficiently store complex session data objects. And don't say cookie either because anything on user side is unsecure and costs thousands of cycles to verify integrity. Elimination of this feature turns asp net core/net 5 efficiency into that of an uncompiled php script when session variables are used. Geez.

ameer-thasthahir commented 2 years ago

It's something AspNet Core related, since .net5.0 console applications don't have such an issue.

Well according to docs you mentioned you can still use it with ASP.NET. You just need to update your csproj file:

To continue using BinaryFormatter in ASP.NET apps, you can re-enable it in the project file.

<PropertyGroup>
  <TargetFramework>net5.0</TargetFramework>
  <!-- Warning: Setting the following switch is *NOT* recommended in web apps. -->
  <EnableUnsafeBinaryFormatterSerialization>true</EnableUnsafeBinaryFormatterSerialization>
</PropertyGroup>

Still it's not working

walter-psjr commented 2 years ago

I had this error using the package NHibernate.Caches.StackExchangeRedis and I solved it by configuring JSON Serializer instead (package NHibernate.Caches.Util.JsonSerializer).