Log4net
  1. Log4net
  2. LOG4NET-347

Log4net not working in an ASP.Net environment with medium trust

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.2.12
    • Component/s: Core
    • Labels:
      None
    • Environment:
      Asp.Net environment running in medium trust

      Description

      As you know, .net 4 security policy are changed and are a lot more strict.

      First of all, I'm not an expert about .net 4 security =) and I never
      developed web apps for medium trust: this is my fist time.

      The problem is simple: log4net doesn't work in medium trust.

      the exception is thrown by the [SecurityCritical] Attribute of the

      System.Reflection.TargetInvocationException: Exception has been thrown
      "GetObjectData" method of ReadOnlyPropertiesDictionary class.

      by the target of an invocation. ---> System.TypeLoadException:
      Inheritance security rules violated while overriding member:
      'log4net.Util.ReadOnlyPropertiesDictionary.GetObjectData(System.Runtim
      e.Serialization.SerializationInfo,
      System.Runtime.Serialization.StreamingContext)'. Security
      accessibility of the overriding method must match the security
      accessibility of the method being overriden.
      at log4net.Repository.Hierarchy.Hierarchy..ctor(ILoggerFactory
      loggerFactory)
      at log4net.Repository.Hierarchy.Hierarchy..ctor()
      — End of inner exception stack trace —
      at System.RuntimeTypeHandle.CreateInstance(RuntimeType type,
      Boolean publicOnly, Boolean noCheck, Boolean& canBeCached,
      RuntimeMethodHandleInternal& ctor, Boolean& bNeedSecurityCheck)
      at System.RuntimeType.CreateInstanceSlow(Boolean publicOnly,
      Boolean skipCheckThis, Boolean fillCache)
      at System.RuntimeType.CreateInstanceDefaultCtor(Boolean publicOnly,
      Boolean skipVisibilityChecks, Boolean skipCheckThis, Boolean fillCache)
      at System.Activator.CreateInstance(Type type, Boolean nonPublic)
      at log4net.Core.DefaultRepositorySelector.CreateRepository(String
      repositoryName, Type repositoryType)
      at log4net.Core.DefaultRepositorySelector.CreateRepository(Assembly
      repositoryAssembly, Type repositoryType, String repositoryName,
      Boolean
      readAssemblyAttributes)
      at log4net.Core.DefaultRepositorySelector.CreateRepository(Assembly
      repositoryAssembly, Type repositoryType)
      at log4net.Core.DefaultRepositorySelector.GetRepository(Assembly
      repositoryAssembly)
      at log4net.Core.LoggerManager.GetRepository(Assembly repositoryAssembly)
      at log4net.Config.XmlConfigurator.Configure()
      [CUT]

      According to this:
      http://msdn.microsoft.com/en-us/library/bb924412.aspx

      Serialization in a partially-trusted application should be done in
      another way ([DataContract] attribute must used, you can't use
      [Serializable] attribute and can't use ISerializable interface to
      control the serialization process.

      I patched log4net in a insane way, but it works now: after removed the
      [Serializable] and the ISerializable interface all work fine, but of
      course i "lose" all the code performed by the implementation of the
      ISerializable interface.
      By the way the log is fine for me now and all I need seem to work.
      To be really onest, i don't know what I should loose doing a thing like
      that..

      Other library (like Ninject) provide a separate build for medium trust
      environment and it works great.

      I think would be really great to have a version like that for log4net.

      1. log4net-347.patch
        1 kB
        Andrew Arnott

        Activity

        Hide
        Andrew Arnott added a comment -

        The fix is actually much simpler than the one described earlier. Please see the accepted answer on http://stackoverflow.com/questions/9700625/c-sharp-webapp-log4net-partial-trust-high-or-medium-not-working for two approaches to fixing the problem.

        Preferably, if you compile log4net differently for .NET 4.x vs. before .NET 4.0, then use #if CLR4 around adding the [SecurityCritical] attribute to all your GetObjectData methods.
        As another option, just add this attribute to your assembly:
        [assembly: System.Security.SecurityRules(System.Security.SecurityRuleSet.Level1)]

        Show
        Andrew Arnott added a comment - The fix is actually much simpler than the one described earlier. Please see the accepted answer on http://stackoverflow.com/questions/9700625/c-sharp-webapp-log4net-partial-trust-high-or-medium-not-working for two approaches to fixing the problem. Preferably, if you compile log4net differently for .NET 4.x vs. before .NET 4.0, then use #if CLR4 around adding the [SecurityCritical] attribute to all your GetObjectData methods. As another option, just add this attribute to your assembly: [assembly: System.Security.SecurityRules(System.Security.SecurityRuleSet.Level1)]
        Hide
        Michele Lepri added a comment -

        Hello Andrew,
        thanks for the reply.

        For what I can remember, if you use the CLR4 complation constant, you will only get your library run in 4.0, but not in medium trust.

        For the solution of the
        [assembly: System.Security.SecurityRules(System.Security.SecurityRuleSet.Level1)]
        attribute, the result is to ask to the system some sort of "please don't run my dll in medium trust, but higher", but of course, if your system doesn't allow it, will run into an exception.

        Regargs
        michele

        Show
        Michele Lepri added a comment - Hello Andrew, thanks for the reply. For what I can remember, if you use the CLR4 complation constant, you will only get your library run in 4.0, but not in medium trust. For the solution of the [assembly: System.Security.SecurityRules(System.Security.SecurityRuleSet.Level1)] attribute, the result is to ask to the system some sort of "please don't run my dll in medium trust, but higher", but of course, if your system doesn't allow it, will run into an exception. Regargs michele
        Hide
        Andrew Arnott added a comment -

        The point of either of these approaches is not to reduce the security requirements to invoke the GetObjectData method, but rather to correctly decorate it so that .NET 4.x won't reject the type when the type is loaded. Either of these attributes should get the CLR to allow loading of the type – invoking the method still requires high trust, but then no one (that I know of) ever invokes that method anyway.

        Show
        Andrew Arnott added a comment - The point of either of these approaches is not to reduce the security requirements to invoke the GetObjectData method, but rather to correctly decorate it so that .NET 4.x won't reject the type when the type is loaded. Either of these attributes should get the CLR to allow loading of the type – invoking the method still requires high trust, but then no one (that I know of) ever invokes that method anyway.
        Hide
        Dominik Psenner added a comment - - edited

        GetObjectData is indirectly invoked on log4net instantiation (see above stacktrace). So this fix moves the exception just to a later point and does not solve the problem. Would you think over the issue again and see if you can come up with a fix? I will accept any sensible patch for the current log4net trunk.

        Show
        Dominik Psenner added a comment - - edited GetObjectData is indirectly invoked on log4net instantiation (see above stacktrace). So this fix moves the exception just to a later point and does not solve the problem. Would you think over the issue again and see if you can come up with a fix? I will accept any sensible patch for the current log4net trunk.
        Hide
        Andrew Arnott added a comment -

        Dominik, I believe you are misreading the exception from the above stacktrace. GetObjectData is not invoked (directly or indirectly) by instantiating log4net. But code that runs upon log4net initialization does cause the faulty code to be loaded, causing a TypeLoadException that we see here. The StackOverflow answer I linked to shows one user who has confirmed that the prescribed fix is effective.

        I am happy to prepare a one-liner patch, if you really think that would save you time. Since log4net evidently uses SVN, how would you like for me to prepare the patch? Should I just use the "diff" tool?

        Show
        Andrew Arnott added a comment - Dominik, I believe you are misreading the exception from the above stacktrace. GetObjectData is not invoked (directly or indirectly) by instantiating log4net. But code that runs upon log4net initialization does cause the faulty code to be loaded, causing a TypeLoadException that we see here. The StackOverflow answer I linked to shows one user who has confirmed that the prescribed fix is effective. I am happy to prepare a one-liner patch, if you really think that would save you time. Since log4net evidently uses SVN, how would you like for me to prepare the patch? Should I just use the "diff" tool?
        Hide
        Dominik Psenner added a comment -

        Something like https://ariejan.net/2007/07/03/how-to-create-and-apply-a-patch-with-subversion would certainly do the job (i.e. "svn diff").

        Show
        Dominik Psenner added a comment - Something like https://ariejan.net/2007/07/03/how-to-create-and-apply-a-patch-with-subversion would certainly do the job (i.e. "svn diff").
        Hide
        Andrew Arnott added a comment -

        I see that the [SecurityCritical] attribute is already defined in the source code within an "#if NET_4_0" preprocessor symbol. Which leads me to believe that this should already work, if only log4net.dll shipped with that symbol defined.

        Show
        Andrew Arnott added a comment - I see that the [SecurityCritical] attribute is already defined in the source code within an "#if NET_4_0" preprocessor symbol. Which leads me to believe that this should already work, if only log4net.dll shipped with that symbol defined.
        Hide
        Andrew Arnott added a comment -

        This may be a bug in the CLR, actually. I have a very simple repro without log4net at all, that demonstrates this exception is generated any time you implement ISerializable even if you follow all security rules. I'm reaching out to the CLR team now to see what they say.

        Show
        Andrew Arnott added a comment - This may be a bug in the CLR, actually. I have a very simple repro without log4net at all, that demonstrates this exception is generated any time you implement ISerializable even if you follow all security rules. I'm reaching out to the CLR team now to see what they say.
        Hide
        Andrew Arnott added a comment -

        This patch has been tested and shown to be effective at fixing log4net in partially trusted ASP.NET web applications.

        Show
        Andrew Arnott added a comment - This patch has been tested and shown to be effective at fixing log4net in partially trusted ASP.NET web applications.
        Hide
        Dominik Psenner added a comment -

        This looks almost good. According to http://msdn.microsoft.com/en-us/library/system.security.securityruleset(v=vs.100).aspx this enumeration is available only in .NET 4.0 and .NET 4.5. Did you try if the patch breaks log4net on Mono, .NET 2.0, 3.0, 3.5? I'm quite sure we can cope those cases with IFDEF's if needed.

        Show
        Dominik Psenner added a comment - This looks almost good. According to http://msdn.microsoft.com/en-us/library/system.security.securityruleset(v=vs.100).aspx this enumeration is available only in .NET 4.0 and .NET 4.5. Did you try if the patch breaks log4net on Mono, .NET 2.0, 3.0, 3.5? I'm quite sure we can cope those cases with IFDEF's if needed.
        Hide
        Andrew Arnott added a comment -

        Thanks, Dominik.
        I don't know whether mono defines this enum, but .NET 2.0, 3.x do not define them. So yes, it seems that an #ifdef is appropriate around this.
        Is this something you can correct while merging it in, or should I update the diff?

        Show
        Andrew Arnott added a comment - Thanks, Dominik. I don't know whether mono defines this enum, but .NET 2.0, 3.x do not define them. So yes, it seems that an #ifdef is appropriate around this. Is this something you can correct while merging it in, or should I update the diff?
        Hide
        Dominik Psenner added a comment - - edited

        Much more interesting would be the answer to the question whether this issue affects other .NET versions and Mono or not?

        Show
        Dominik Psenner added a comment - - edited Much more interesting would be the answer to the question whether this issue affects other .NET versions and Mono or not?
        Hide
        Andrew Arnott added a comment -

        Other .NET versions (prior to 4.0) don't have this issue to begin with, so there isn't a problem to solve there.
        I haven't heard the mono community mention whether they have this problem.
        But the .NET 4.x crowd seems quite vocal.

        Show
        Andrew Arnott added a comment - Other .NET versions (prior to 4.0) don't have this issue to begin with, so there isn't a problem to solve there. I haven't heard the mono community mention whether they have this problem. But the .NET 4.x crowd seems quite vocal.
        Hide
        Dominik Psenner added a comment -

        Patch commited as revision: 1446054

        Please check if the issue is fixed there and feed back so that we can close the issue.

        Show
        Dominik Psenner added a comment - Patch commited as revision: 1446054 Please check if the issue is fixed there and feed back so that we can close the issue.
        Hide
        Andrew Arnott added a comment -

        The patch diff looks good. And I obviously already tested my own private build. Do you have a continuous integration build that I can download from to test that your patch works effectively in the official build?

        Show
        Andrew Arnott added a comment - The patch diff looks good. And I obviously already tested my own private build. Do you have a continuous integration build that I can download from to test that your patch works effectively in the official build?
        Hide
        Dominik Psenner added a comment -

        Apologies, there are currently no official nightly builds. If you're interested you can look up the discussion about that topic on the log4net-dev mailing list for more information. In case you feel like setting up jenkins nightly builds, please contact us. I can assure a warm welcome to you.

        Show
        Dominik Psenner added a comment - Apologies, there are currently no official nightly builds. If you're interested you can look up the discussion about that topic on the log4net-dev mailing list for more information. In case you feel like setting up jenkins nightly builds, please contact us. I can assure a warm welcome to you.
        Hide
        Andrew Arnott added a comment -

        Thanks for going to the trouble of your survey, Dominik.
        What would you suggest for testing prior to closing the issue? If it's .NET 4.x, I had already tested my private build before submitting the fix. If it's any other platform we need to verify on, I don't have a computer with those platforms installed to test on, nor the experience in building log4net for those platforms.

        Show
        Andrew Arnott added a comment - Thanks for going to the trouble of your survey, Dominik. What would you suggest for testing prior to closing the issue? If it's .NET 4.x, I had already tested my private build before submitting the fix. If it's any other platform we need to verify on, I don't have a computer with those platforms installed to test on, nor the experience in building log4net for those platforms.
        Hide
        Dominik Psenner added a comment -

        Thanks for testing it with .NET4. If this fix yields unwanted behaviour we will know it sooner or later.

        Show
        Dominik Psenner added a comment - Thanks for testing it with .NET4. If this fix yields unwanted behaviour we will know it sooner or later.

          People

          • Assignee:
            Dominik Psenner
            Reporter:
            Michele Lepri
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development