Hi Nicko,
Thanks for the feedback. Lots of good info, as usual.
> Having a global hook for supplying the credentials may be the way to
go but I would first like to explore using the config file to attach an
object to a > property on a specified appender.
I like the clarity of your the approach for specifying config file
entries.
However, for our application, it would be difficult to support. Why?
Because it requires updates to the config file. We have lots of
customers that have deployed our application which uses log4net and
associated config files. Asking them to edit the config file entries is
scary - the often muck it up with typos, which means all our logging
stops. When that happens, support becomes very difficult.
We'd like the next version of our application to be able to use the
impersonation, without requiring them to change their config files.
This would let us simplify the permissions needed to enable logging, and
not introduce a risk they will break something.
It might be a stretch to impose our deployment needs on the broader
log4net group, but I'm hoping there is a way to get the feature and
retain backward compatibility of the config files. So far, the only way
I can think of for this is via the attribute. However, it might make
sense to add support for both the attribute and the config file entries
as you outline below. Potentially we could live with just config file
support, but this would require us to write an upgrade utility that
could find config files and insert the right impersonation statements
into the config files (ugh :)
So, these are the options
1) Config file support for impersonation
2) DLL-attribute support for impersonation
3) Both 1 and 2
Seems like it would be easy enough to go with option 3, since the
underlying impersonation logic works the same, and the only difference
is how it gets wired into an appender.
Do you see any problem going with option 3?
> Another question is where in the Appender inheritance hierarchy do we
put the credential?
What would you think of putting a property on Appenders that expose
credential management? I would probably make an interface like:
public interface IImpersonationConsumer
{
// See below...IImpersonation replaces
IExternalCredential
IImpersonation Impersonation
{
get;
set;
}
}
Then, the appenders that support impersonation would implement
IImpersonationConsumer, exposing the property to set the impersonation
object to be used. During initial configuration, this could be wired up
like this:
1) Check for a <credential> node in the configuration XML for the
appender.
- If found,
- create an instance of the specified appender
- attach it to the appender (assuming the appender
implements IImpersonationConsumer)
2) If credentials not set in 1, then check for the
ImpersonationFactoryAttribute.
- If found,
- instantiate the impersonation factory
- this factory can create the impersonation object
- attach it to the appender (assuming the appender
implements IImpersonationConsumer)
> I have thought long and hard about the API syntax for the
IExternalCredential. I think that I prefer the C# using syntax. i.e.
> where the Impersonate() method returns an IDisposable instance. In the
default or null case this work well because if Impersonate() returns
null to the
I love it. IDisposable makes sense, and is very clean. Plus it is
null-friendly. Sounds like a no-brainer
> I'm not sure that we have got the terminology quite right. While I
don't necessarily want to just use the MS Windows terminology it seems
slightly odd to
Perhaps more emphasis on the impersonation and less on the 'credential'?
Instead of IExternalCredential, how about IImpersonation?
public interface IImpersonation : IDisposable
{
void Impersonate();
void Undo();
}
Note that an Undo should be implicitly called via the IDispoable if it
hasn't already.
This seems like it would be less tied to what it is in some cases
(credential), and more tied to what it does (impersonation). That way
it always works regardless of what the underlying code has going on.
For example, this way there is less need to associate it with a specific
identity, in cases where some external app wants to impersonate
something other than a single user (maybe from a user pool?), or some
other scenario where there is not a tight association with a specific
fixed identity.
So where are we?
This solution is sounding pretty good. For the most part I think we
agree on the following
1) Impersonation via objects
- objects are customer-extensible, allowing custom impersonation
2) Wiring up the impersonation objects at config time via config and/or
DLL attribute
3) Using a property (IImpersonationConsumer) on Appenders that support
impersonation
4) Appenders wrap code that accesses system resources with calls to the
Impersonation.
The only points still to resolve (or confirm) are the naming and maybe
the configuration (item 2). Can you think of any others?
-Doug
-----Original Message-----
From: Nicko Cadell [mailto:[EMAIL PROTECTED]
Sent: Sunday, September 26, 2004 5:28 PM
To: Log4NET Dev
Subject: RE: Allow log4net to impersonate a user via app-specific
mechanism.
Doug,
Having a global hook for supplying the credentials may be the way to go
but I would first like to explore using the config file to attach an
object to a property on a specified appender.
This would allow the credential object to be specified for each appender
that supported credentials.
A user could write something like this in the config file:
<appender type="log4net.Appender.FileAppender">
<credential type="log4net.Util.WindowsCredential">
<username value="DOMAIN\User"/>
<password value="****"/>
</credential>
...
</appender>
Obviously we would have other implementations of IExternalCredential
that would not encourage users to embed system passwords in plain text
files, but you see how it could instead specify registry keys to read.
Alternatively it could be a type specified by the application developer
which retrieves the credentials from the calling application.
<appender type="log4net.Appender.FileAppender">
<credential type="MyApp.MyCredential, MyAssembly" />
...
</appender>
What do you think?
Another question is where in the Appender inheritance hierarchy do we
put the credential? We could put it only onto classes that require it or
we could put it on to the AppenderSkeleton and just let the appenders
decide to use it or not. Putting it on the actual appenders that use it
will mean that users won't get confused as to why the property exists on
the appenders that don't use it.
I have thought long and hard about the API syntax for the
IExternalCredential. I think that I prefer the C# using syntax. i.e.
where the Impersonate() method returns an IDisposable instance. In the
default or null case this work well because if Impersonate() returns
null to the using block this does not cause a problem. I generally like
the syntax:
protected void RollFile(string fromFile, string toFile) {
...
using(credential.Impersonate())
{
target.Delete();
}
}
In the default case this should be fast because Impersonate would return
null. By default the credential object would be a NullCredential
singleton instance. This is probably slightly slower than checking
against null, but you don't really know how good a job the JIT will do.
Returning an object to do the revert will be slower than calling a
Revert method directly, however it should not be significant relative to
the cost of doing some sort of impersonation.
I'm not sure that we have got the terminology quite right. While I don't
necessarily want to just use the MS Windows terminology it seems
slightly odd to impersonate a credential as in windows you would
impersonate an identity. We either need to use
ExternalIdentity.Impersonate() or Identity.Impersonate() or
ExternalCredential.Enable() / Activate() something like that maybe.
What do you think?
Nicko
> -----Original Message-----
> From: de la Torre, Douglas [mailto:[EMAIL PROTECTED]
> Sent: 25 September 2004 00:43
> To: Log4NET Dev
> Subject: RE: Allow log4net to impersonate a user via app-specific
> mechanism.
>
> Hi All,
>
> Here's some sample code to illustrate how impersonation could easily
> be added to the appender classes. The goal would be to let the
> application that consumes log4net be able to handle the impersonation
> when needed.
> The sample's I'm including use an attribute called
> ImpersonationFactoryAttribute that log4net can check for on the user's
> DLL to let that DLL override the factory that supplies objects which
> do the impersonation.
>
> Here's how this would work.
>
> 1) User adds this attribute to their DLL in the AssemblyInfo.cs file,
> and specify the type of factory to read from their own DLL
>
> [assembly:
> log4net.Appender.Impersonation.ImpersonationFactoryAttribute(I
> mpersonati
> onFactoryType=typeof(LoggingTestApp.MyImpersonationFactory))]
>
> 2) In the user DLL, the factory class would look something like this
>
> public class MyImpersonationFactory : IImpersonationFactory
> {
> public IExternalCredential
> Create(log4net.Appender.IAppender appender)
> {
> if( appender is
> log4net.Appender.RollingFileAppender || appender is
> log4net.Appender.FileAppender )
> {
> return new MyImpersonator();
> }
> return null;
> }
> }
>
> 3) The appender would add code to instantiate the IExternalCredential
> object to use for impersonation. For example, in RollingFileAppender
>
> override public void ActivateOptions()
> {
> this.credential =
> Impersonation.ImpersonationFactory.Instance.Create( this );
>
> ...
> }
>
> 4) The appender would impersonate around operations that access
> system resources. For example, when the RollingFileAppender tries to
> delete a
> file:
>
> protected void RollFile(string fromFile, string toFile)
> {
> ...
>
> LogLog.Debug("RollingFileAppender:
> Deleting existing target file ["+target+"]");
> try
> {
> this.credential.Impersonate();
> target.Delete();
> }
> finally
> {
> this.credential.Revert();
> }
>
> ...
> }
>
> This is the basic mechanism, and you can see how easy this is both
> from the external DLL that consumes log4net to extend, as well as how
> simple it is for log4net to support.
>
> Note the above code is using a NullImpersonation class, which has
> empty Impersonate and Revert methods, allowing us to eliminate null
> checks for better readability. Alternatively, we could make the code
> add the null check guard clauses, in which case the appender code
> above would change to this:
>
> protected void RollFile(string fromFile, string toFile)
> {
> ...
>
> LogLog.Debug("RollingFileAppender:
> Deleting existing target file ["+target+"]");
> try
> {
> if( null != this.credential )
> this.credential.Impersonate();
> target.Delete();
> }
> finally
> {
> if( null != this.credential )
> this.credential.Revert();
> }
>
> ...
> }
>
> What do you all think of this approach?
>
> This seems like a clean way to let external code handle the
> impersonation. For example, we would like this capability so we can
> impersonate as a single user for file I/O operations, thus allowing us
> to simplify our deployments for an ASP.NET application. Currently,
> any site user must have full permissions in the log directory. A
> change like this to the appenders would allow us to increase security
> and simplify management of the logging folder by impersonating as a
> single user.
>
> Can we go ahead with adding this into the project? If this approach
> seems sound, the files I've attached can be added to CVS first, then
> once added, the next stage of work in the appenders could begin,
> changing the resource access to account for the impersonation calls.
>
> I appreciate your feedback and consideration!
>
> -Doug
>
