You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by sterling <st...@covalent.net> on 2001/11/21 04:37:03 UTC
[PATCH] apache core dumps if you call ap_note_basic_auth_failure
when auth type is null
Hi -
Set up an auth directory without AuthType but with require valid-user and
AuthName and load an auth module that uses ap_note_basic_auth_failure...
el kabong!! this patch stops the coro dumpo.
sterling
Index: server/protocol.c
===================================================================
RCS file: /home/cvspublic/httpd-2.0/server/protocol.c,v
retrieving revision 1.52
diff -u -r1.52 protocol.c
--- server/protocol.c 2001/11/12 23:49:06 1.52
+++ server/protocol.c 2001/11/21 02:14:43
@@ -764,7 +764,7 @@
AP_DECLARE(void) ap_note_basic_auth_failure(request_rec *r)
{
- if (strcasecmp(ap_auth_type(r), "Basic"))
+ if( ap_auth_type(r) && strcasecmp(ap_auth_type(r), "Basic"))
ap_note_auth_failure(r);
else
apr_table_setn(r->err_headers_out,
Re: [PATCH] apache core dumps if you call ap_note_basic_auth_failure when auth type is null
Posted by Ryan Bloom <rb...@covalent.net>.
On Tuesday 20 November 2001 07:38 pm, Doug MacEachern wrote:
> On Tue, 20 Nov 2001, sterling wrote:
> > Hi -
> >
> > Set up an auth directory without AuthType but with require valid-user and
> > AuthName and load an auth module that uses ap_note_basic_auth_failure...
> > el kabong!! this patch stops the coro dumpo.
>
> this has bitten others in 1.x too. ended up adding protection in the
> modperl wrapper functions. i applied a slightly different version to
> prevent the same problem in ap_note_auth_failure(). and also changed
> if (type && strcasecmp(ap_auth_type(r), "Basic"))
> to
> if (!type || ...)
> cause i don't think it should set the *-Authenticate header if there is no
> AuthType configured, right? or maybe ap_auth_type() should default to
> Basic?
I like both fixes, and I agree that we need one of them. I already saw the
commit, too. I also have another suggested improvement that I think we should
add.
This is a configuration error. We should be able to detect this kind of
configuration error and report it. With the config tree logic, we can do this, by
adding a bit of logic.
I can do the work to make sure that when you have AuthType, you also have
AuthName and Require directives. Before I do however, does anybody disagree
that we should have this logic?
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: [PATCH] apache core dumps if you call ap_note_basic_auth_failure
when auth type is null
Posted by sterling <st...@covalent.net>.
On Tue, 20 Nov 2001, Ryan Bloom wrote:
> On Tuesday 20 November 2001 09:31 pm, sterling wrote:
> > On Tue, 20 Nov 2001, Doug MacEachern wrote:
> > > On Tue, 20 Nov 2001, sterling wrote:
> > Yeah -
> >
> > I pondered that for a bit... We should probably log an error (like bloom
> > suggested) so the user is aware of the misconfiguration, and then send
> > none of the headers (like your patch does).
> >
> > I don't think we should default to Basic.
>
> I was actually thinking of not starting. The configuration is invalid, so we
> should exit with an error IMO.
>
> Ryan
Well, If you set AuthType without AuthName you get an Internal Server
Error at request time, if thats any indication. Maybe we should make them
do the same thing (whether at startup time or request time).
sterling
Re: [PATCH] apache core dumps if you call ap_note_basic_auth_failure when auth type is null
Posted by Ryan Bloom <rb...@covalent.net>.
On Tuesday 20 November 2001 09:31 pm, sterling wrote:
> On Tue, 20 Nov 2001, Doug MacEachern wrote:
> > On Tue, 20 Nov 2001, sterling wrote:
> > > Hi -
> > >
> > > Set up an auth directory without AuthType but with require valid-user
> > > and AuthName and load an auth module that uses
> > > ap_note_basic_auth_failure... el kabong!! this patch stops the coro
> > > dumpo.
> >
> > this has bitten others in 1.x too. ended up adding protection in the
> > modperl wrapper functions. i applied a slightly different version to
> > prevent the same problem in ap_note_auth_failure(). and also changed
> > if (type && strcasecmp(ap_auth_type(r), "Basic"))
> > to
> > if (!type || ...)
> > cause i don't think it should set the *-Authenticate header if there is
> > no AuthType configured, right? or maybe ap_auth_type() should default to
> > Basic?
>
> Yeah -
>
> I pondered that for a bit... We should probably log an error (like bloom
> suggested) so the user is aware of the misconfiguration, and then send
> none of the headers (like your patch does).
>
> I don't think we should default to Basic.
I was actually thinking of not starting. The configuration is invalid, so we
should exit with an error IMO.
Ryan
______________________________________________________________
Ryan Bloom rbb@apache.org
Covalent Technologies rbb@covalent.net
--------------------------------------------------------------
Re: [PATCH] apache core dumps if you call ap_note_basic_auth_failure
when auth type is null
Posted by sterling <st...@covalent.net>.
On Tue, 20 Nov 2001, Doug MacEachern wrote:
> On Tue, 20 Nov 2001, sterling wrote:
>
> > Hi -
> >
> > Set up an auth directory without AuthType but with require valid-user and
> > AuthName and load an auth module that uses ap_note_basic_auth_failure...
> > el kabong!! this patch stops the coro dumpo.
>
> this has bitten others in 1.x too. ended up adding protection in the
> modperl wrapper functions. i applied a slightly different version to
> prevent the same problem in ap_note_auth_failure(). and also changed
> if (type && strcasecmp(ap_auth_type(r), "Basic"))
> to
> if (!type || ...)
> cause i don't think it should set the *-Authenticate header if there is no
> AuthType configured, right? or maybe ap_auth_type() should default to
> Basic?
>
Yeah -
I pondered that for a bit... We should probably log an error (like bloom
suggested) so the user is aware of the misconfiguration, and then send
none of the headers (like your patch does).
I don't think we should default to Basic.
sterling
Re: [PATCH] apache core dumps if you call ap_note_basic_auth_failure
when auth type is null
Posted by Doug MacEachern <do...@covalent.net>.
On Tue, 20 Nov 2001, sterling wrote:
> Hi -
>
> Set up an auth directory without AuthType but with require valid-user and
> AuthName and load an auth module that uses ap_note_basic_auth_failure...
> el kabong!! this patch stops the coro dumpo.
this has bitten others in 1.x too. ended up adding protection in the
modperl wrapper functions. i applied a slightly different version to
prevent the same problem in ap_note_auth_failure(). and also changed
if (type && strcasecmp(ap_auth_type(r), "Basic"))
to
if (!type || ...)
cause i don't think it should set the *-Authenticate header if there is no
AuthType configured, right? or maybe ap_auth_type() should default to
Basic?
Index: server/protocol.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/protocol.c,v
retrieving revision 1.52
diff -u -r1.52 protocol.c
--- server/protocol.c 2001/11/12 23:49:06 1.52
+++ server/protocol.c 2001/11/21 03:10:39
@@ -756,15 +756,25 @@
AP_DECLARE(void) ap_note_auth_failure(request_rec *r)
{
- if (!strcasecmp(ap_auth_type(r), "Basic"))
- ap_note_basic_auth_failure(r);
- else if (!strcasecmp(ap_auth_type(r), "Digest"))
- ap_note_digest_auth_failure(r);
+ const char *type = ap_auth_type(r);
+ if (type) {
+ if (!strcasecmp(type, "Basic"))
+ ap_note_basic_auth_failure(r);
+ else if (!strcasecmp(type, "Digest"))
+ ap_note_digest_auth_failure(r);
+ }
+ /* XXX: else there is no AuthType configured
+ * should we log an error or something ?
+ */
}
AP_DECLARE(void) ap_note_basic_auth_failure(request_rec *r)
{
- if (strcasecmp(ap_auth_type(r), "Basic"))
+ const char *type = ap_auth_type(r);
+ /* if there is no AuthType configure or it is something other than
+ * Basic, let ap_note_auth_failure() deal with it
+ */
+ if (!type || strcasecmp(type, "Basic"))
ap_note_auth_failure(r);
else
apr_table_setn(r->err_headers_out,