Need to secure remote access by default

[GoogleCodeExporter]

I wrote about the issue here:
http://haacked.com/archive/2007/07/24/securely-implement-elmah-for-plug-and-play
-error-logging.aspx

Basically, our sample config makes elmah.axd wide open for anyone to view.
We should provide an example that shows how to secure it.

As a matter of fact, the download link at the bottom of my post includes a
fully functioning app that demonstrates that.

Original issue reported on code.google.com by haac...@gmail.com on 24 Jul 2007 at 8:56

[GoogleCodeExporter]

So you're suggesting that there should be a <location> tag in the sample 
web.config 
(http://elmah.googlecode.com/svn/trunk/samples/web.config) that secures the 
handler 
path by default? The problem is that the sample web.config assumes people will 
be 
copying, pasting and uncommenting the interesting sections and bits into their 
own 
web.config rather than using its contents verbatim. It may very well be 
therefore 
that they will forget to copy the location tag.

A more draconian approach could be to refuse to serve anything out of the 
handler 
factory unless at least the request is authenticated (in other words 
HttpRequest.IsAuthenticated must be true). This would force authentication and 
authorization to be setup on the site correctly before the log can be viewed. 
For 
the casual trial, this can be done easily using Forms-based authentication and 
credentials set up directly in the web.config. However, both solutions 
(including 
the sample accompanying your blog entry) do not prevent a regular, yet 
authenticated, user of the site from viewing the log. You would have to ensure 
that 
the visitor belongs to one or more administrative roles. Perhaps a second 
drastic 
measure could be to assume a certain role (could be made configurable) so that 
a 
check is made to ensure the principal belongs to that role. Again, all this 
could be 
a bit draconian.

A gentler yet equally effective approach could be to put up a 
very-much-in-the-face 
warning on the pages if the request is not authenticated that reminds that user 
to 
secure via the location tag and authorization by some role.

Meanwhile, I'll setup a wiki that points to your blog entry as an interim 
measure.

What do you think? Is this a code or documentation defect/enhancement?

Original comment by azizatif on 24 Jul 2007 at 9:35

[GoogleCodeExporter]

OK, I've added the wiki over at 
http://code.google.com/p/elmah/wiki/SecuringErrorLogPages and marked it as 
featured. :) For now, it points directly to the blog entry in the opening 
comment 
assuming it needs fleshing out. Perhaps it makes sense to add the relevant blog 
bits 
directly into the wiki? If we go with the annoying message in every page if the 
request is unauthenticated then one could directly provide a link to the wiki 
from 
within ELMAH.

Original comment by azizatif on 24 Jul 2007 at 9:46

[GoogleCodeExporter]

For now, I have also added notes to the sample web.config on how to secure the 
end-
point and added the location tag that denies access to unauthenticated users by 
default. Also added pointers to SecuringErrorLogPages wiki. Can be considered 
fixed 
now?

Original comment by azizatif on 27 Jul 2007 at 8:57

[GoogleCodeExporter]

I'd like to suggest that there be a configurable way to only allow elmah.axd be
viewed from a local request. I looked at the notes on using a location element, 
but
we're not using any authentication on our site. So adding that in simply to 
allow
authentication to see elmah.axd is a non-starter.

I tweaked my version of elmah by this code to the GetHandler method in
ErrorLogPageFactory:

            if (context.Request.IsLocal == false)
            {
                context.Response.Redirect("~/default.aspx");
                return null;
            }

Not terribly clean or configurable, but works great for our situation. If there 
was a
way to make this configurable, that would be terrific.

Original comment by donnie.h...@gmail.com on 3 Oct 2007 at 2:04

[GoogleCodeExporter]

This can already be achieved without changing the base code in ELMAH. 
Essentially, 
write your own class (say in App_Code) that inherits from 
Elmah.ErrorLogPageFactory. 
Override GetHandler and plug in your code above. If the request is local then 
simply 
call the base implementation. So something along the lines of...

public class MyErrorLogPageFactory : Elmah.ErrorLogPageFactory {
    public virtual IHttpHandler GetHandler(...) {
        if (context.Request.IsLocal == false) {
            context.Response.Redirect("~/default.aspx");
            return null;
        }
        base.GetHandler(...);
    }
}

Now just register your factory instead of the one from ELMAH for elmah.axd. The 
benefit of this approach is that you can customize it to your liking by as much 
as 
you want. :) Also, if it's in App_Code, you don't need any configuration since 
changing the code (e.g. changing the string from "~/default.aspx" to just "~/") 
will 
simply casue it to re-compile dynamically.

Original comment by azizatif on 3 Oct 2007 at 2:40

[GoogleCodeExporter]

Are you saying this isn't a feature that warrants being put directly in elmah? I
understand where you're coming from, to a point, but those of us using 
something like
elmah are hoping to have less code to develop and maintain for these scenarios. 
;)

Original comment by donnie.h...@gmail.com on 3 Oct 2007 at 3:00

[GoogleCodeExporter]

> Are you saying this isn't a feature that warrants being put directly in elmah?

No, just that it's about time to move ELMAH to BETA 2 stage and would focus on 
absolute defects rather than enhancements that can be candidates for a future 
release. The above scenario can be worked in one way or another and requires 
neither 
a change to your web application code nor ELMAH. Still, let's see what can be 
done 
here and what effort it would entail (new configuration section, handler and 
testing). Why would you propose a redirection? Wouldn't a 403 Forbidden 
response be 
more appropriate here?

Original comment by azizatif on 3 Oct 2007 at 7:08

[GoogleCodeExporter]

First, I didn't mean to imply that the feature had to be part of the next 
release. It
sounds like you think it's a reasonable feature request, so I'd just ask it be
scheduled accordingly. Second, I also didn't mean to imply that a Redirect was 
the
"right" solution. For what I needed and my app, it was the simplest thing that
required no knowledge on the part of the app using elmah. I agree that a 403 is
probably the most precise response per the protocol. Third, when you said a new
"handler", did you mean for the config section, or a new HttpHandler? If the 
latter,
I'm not sure where that would be required.

Original comment by donnie.h...@gmail.com on 3 Oct 2007 at 8:44

[GoogleCodeExporter]

> didn't mean to imply that the feature had to be part of the next release

Fair enough. :)

> it's a reasonable feature request

Definitely.

> I agree that a 403 is probably the most precise response per the protocol

403 it is then.

> did you mean for the config section, or a new HttpHandler?

I meant both. I imagine a new section like the following that would be child of 
the 
<elmah> section:

        <security allowRemoteAccess="BOOLEAN" />

where BOOLEAN would be true when the value is "yes", "on", "true" or "1" and 
false 
in all other cases. By default, if this section is missing then remote access 
is 
denied. 

As for a new HttpHandler, that would be used to provide help in the body of the 
403 
response.

Original comment by azizatif on 3 Oct 2007 at 9:06

[GoogleCodeExporter]

Original comment by azizatif on 4 Oct 2007 at 8:24

• Changed title: Need to secure remote access by default
• Changed state: Started
• Added labels: Security

[GoogleCodeExporter]

Fixed in r165. By default, remote access is denied even if the request is 
authenticated. Only requests from local machine will be entertained. Would 
appreciate it tremendously if someone can check out the overally functionality 
and 
provide feedback. Meanwhile, the issue is primarily considered fixed.

Original comment by azizatif on 4 Oct 2007 at 8:41

• Changed state: Fixed