You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Keith Wannamaker <Ke...@Wannamaker.org> on 2002/11/08 06:36:11 UTC
auth bug fix for 4.0.6
It turns out TC 4.0.6 has the same auth bug as 3.3--
it challenges prior to redirects. The immediate problem
this causes is that some browsers will cache and send
credentials for the entire domain after being challenged
for a top level directory without a trailing slash.
So 4.0.6 exhibits this wrong behavior:
GET /foo -> 401
GET /foo with auth -> 301 to /foo/
GET /foo/ with auth -> 200
GET /bar with auth .. (browser will send auth to other realms!)
With the following patch it will exhibit this correct behavior:
GET /foo -> 301 to /foo/
GET /foo/ -> 401
GET /foo/ with auth -> 200
GET /bar WITHOUT auth
I'll be glad to ci it, but those more in the know may
have a better location for the fix in mind.
Keith
Index: catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java
===================================================================
RCS file: /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java,v
retrieving revision 1.23.2.5
diff -u -r1.23.2.5 AuthenticatorBase.java
--- catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java
27 Feb 2002 17:42:58 -0000 1.23.2.5
+++ catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java
8 Nov 2002 05:25:06 -0000
@@ -422,8 +422,18 @@
context.invokeNext(request, response);
return;
}
HttpRequest hrequest = (HttpRequest) request;
HttpResponse hresponse = (HttpResponse) response;
+
+ // Do not authenticate prior to redirects
+ String uri = ((HttpServletRequest) request.getRequest()).getRequestURI();
+ if (uri.length() > 0 && ! uri.endsWith("/") &&
+ uri.equals(request.getContext().getName())) {
+ context.invokeNext(request, response);
+ return;
+ }
+
if (debug >= 1)
log("Security checking request " +
((HttpServletRequest) request.getRequest()).getMethod() + " " +
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
[Coyote] Special headers handling
Posted by Remy Maucherat <re...@apache.org>.
As I believe enforcing the protocol should be the responsability of the
protocol handler, I have done (and will commit soon) a change which
delegates setting the special headers (including "content-language" and
"content-type") to the protocol handler. I will commit the change soon.
The cause of this refactoring is the fact that once a header like
"content-length" has been added in the MimeHeaders, it is not possible
to remove it, which then breaks the protocol for some status codes.
This change is likely to cause minor problems in Coyote JK 2 (please
review), but it should be very easy to fix.
Thanks,
Rémy
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: auth bug fix for 4.0.6
Posted by Remy Maucherat <re...@apache.org>.
Bill Barker wrote:
> Replying to an older version of the thread, since I share messages the
> other
> way around.
>
> Personally, I think that Remy needs to work on his people skills.
> Keith has
> been a very valuable committer on the 3.3 branch. Rather than
> shooting him
> down, you could have given him pointers on how to improve his patch (which
> I'll probably do, but it takes me much longer then it does you :). Just
> remember that Keith has the right to veto (although I doubt that he'll use
> it) any 4.x release until his bug is fixed.
I just woke up (yawn). I'd like to apologize to Keith if he's been
offended. I didn't see any reason to: since he's new to the codebase, I
don't see how you can expect to get everything right the first time. So
I agree to fix the bug (as I understand the issue thanks to his
explnations), but don't think the patch is right at the moment.
It's just that in 4.1 and 5.0, you have to be careful about using the
decodedURI rather than the original URI to do any mapping or check.
Otherwise, it leaves the door open to hacks using URI encoding.
Rémy
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: auth bug fix for 4.0.6
Posted by Bill Barker <wb...@wilshire.com>.
Replying to an older version of the thread, since I share messages the other
way around.
Personally, I think that Remy needs to work on his people skills. Keith has
been a very valuable committer on the 3.3 branch. Rather than shooting him
down, you could have given him pointers on how to improve his patch (which
I'll probably do, but it takes me much longer then it does you :). Just
remember that Keith has the right to veto (although I doubt that he'll use
it) any 4.x release until his bug is fixed.
----- Original Message -----
From: "Remy Maucherat" <re...@apache.org>
To: "Tomcat Developers List" <to...@jakarta.apache.org>
Sent: Thursday, November 07, 2002 11:41 PM
Subject: Re: auth bug fix for 4.0.6
> Bill Barker wrote:
>
> > As a non-4.x expert, your patch looks ok. I would guess that it would
> > still
> > have problems with a request to /foo/protected where the
> > security-constraint
> > is only for /foo/protected/*.
>
> I don't agree, the patch is bad for 4.1.x and 5.0 (at least, you must
> use the decoded URI there). Tomcat 4.0.x is probably ok.
>
> I also don't agree with Keith's interpretation depending on what the
> constraint is. Can you give examples ?
>
> Remy
>
> >
> > >It turns out TC 4.0.6 has the same auth bug as 3.3--
> > >it challenges prior to redirects. The immediate problem
> > >this causes is that some browsers will cache and send
> > >credentials for the entire domain after being challenged
> > >for a top level directory without a trailing slash.
> > >
> > >So 4.0.6 exhibits this wrong behavior:
> > > GET /foo -> 401
> > > GET /foo with auth -> 301 to /foo/
> > > GET /foo/ with auth -> 200
> > > GET /bar with auth .. (browser will send auth to other realms!)
> > >
> > >With the following patch it will exhibit this correct behavior:
> > > GET /foo -> 301 to /foo/
> > > GET /foo/ -> 401
> > > GET /foo/ with auth -> 200
> > > GET /bar WITHOUT auth
> > >
> > >
> > >I'll be glad to ci it, but those more in the know may
> > >have a better location for the fix in mind.
> > >
> > >Keith
> > >
> > >
> > >Index:
> >
> >
catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java
> >
> > >===================================================================
> > >RCS file:
> >
> >
/home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/authenti
> > cator/AuthenticatorBase.java,v
> >
> > >retrieving revision 1.23.2.5
> > >diff -u -r1.23.2.5 AuthenticatorBase.java
> > >---
> >
> >
catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java
> >
> > >27 Feb 2002 17:42:58 -0000 1.23.2.5
> > >+++
> >
> >
catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java
> >
> > >8 Nov 2002 05:25:06 -0000
> > >@@ -422,8 +422,18 @@
> > > context.invokeNext(request, response);
> > > return;
> > > }
> > > HttpRequest hrequest = (HttpRequest) request;
> > > HttpResponse hresponse = (HttpResponse) response;
> > >+
> > >+ // Do not authenticate prior to redirects
> > >+ String uri = ((HttpServletRequest)
> >
> > request.getRequest()).getRequestURI();
> >
> > >+ if (uri.length() > 0 && ! uri.endsWith("/") &&
> > >+ uri.equals(request.getContext().getName())) {
> > >+ context.invokeNext(request, response);
> > >+ return;
> > >+ }
> > >+
> > > if (debug >= 1)
> > > log("Security checking request " +
> > > ((HttpServletRequest)
> > request.getRequest()).getMethod() +
> >
> > " " +
> >
> > >
> > >--
> > >To unsubscribe, e-mail:
> >
> >
> >
> > >For additional commands, e-mail:
> >
> >
> >
> >
> >
> > --
> > To unsubscribe, e-mail:
> > For additional commands, e-mail:
> >
> >
>
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
> For additional commands, e-mail:
<ma...@jakarta.apache.org>
>
>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: auth bug fix for 4.0.6
Posted by Keith Wannamaker <Ke...@Wannamaker.org>.
I regret it wasn't in line with your expectations.
Keith
| -----Original Message-----
| From: Remy Maucherat [mailto:remm@apache.org]
| Sent: Friday, November 08, 2002 1:57 PM
| To: Tomcat Developers List
| Subject: Re: auth bug fix for 4.0.6
|
|
| No, I was just complaining about the patch quality, and the fact that
| there was no examples (although I had figured out what you were talking
| about, it was to avoid a misunderstanding).
|
| Rémy
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: auth bug fix for 4.0.6
Posted by Remy Maucherat <re...@apache.org>.
Keith Wannamaker wrote:
> Remy, I don't even know if 4.1.x and 5.0 share the bug or not;
> I haven't tested it, though I suspect they do. I do know 4.0.6
> has the bug.
>
> I'm not sure what interpretation you are questioning -- if it
> is the placement or nature of the fix, sure, I said someone may
> want to tweak the location and method of the fix. However the
> behavior is very standard and necessary (Apache handles auth
> and redirects the same way for the same reason).
>
> In the example I gave, the security constraint was /* for the
> context.
No, I was just complaining about the patch quality, and the fact that
there was no examples (although I had figured out what you were talking
about, it was to avoid a misunderstanding).
Rémy
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: auth bug fix for 4.0.6
Posted by Keith Wannamaker <Ke...@Wannamaker.org>.
Remy, I don't even know if 4.1.x and 5.0 share the bug or not;
I haven't tested it, though I suspect they do. I do know 4.0.6
has the bug.
I'm not sure what interpretation you are questioning -- if it
is the placement or nature of the fix, sure, I said someone may
want to tweak the location and method of the fix. However the
behavior is very standard and necessary (Apache handles auth
and redirects the same way for the same reason).
In the example I gave, the security constraint was /* for the
context.
Keith
| -----Original Message-----
| From: Remy Maucherat [mailto:remm@apache.org]
| Sent: Friday, November 08, 2002 2:42 AM
| To: Tomcat Developers List
| Subject: Re: auth bug fix for 4.0.6
|
|
| Bill Barker wrote:
|
| > As a non-4.x expert, your patch looks ok. I would guess that it would
| > still
| > have problems with a request to /foo/protected where the
| > security-constraint
| > is only for /foo/protected/*.
|
| I don't agree, the patch is bad for 4.1.x and 5.0 (at least, you must
| use the decoded URI there). Tomcat 4.0.x is probably ok.
|
| I also don't agree with Keith's interpretation depending on what the
| constraint is. Can you give examples ?
|
| Remy
|
| >
| > >It turns out TC 4.0.6 has the same auth bug as 3.3--
| > >it challenges prior to redirects. The immediate problem
| > >this causes is that some browsers will cache and send
| > >credentials for the entire domain after being challenged
| > >for a top level directory without a trailing slash.
| > >
| > >So 4.0.6 exhibits this wrong behavior:
| > > GET /foo -> 401
| > > GET /foo with auth -> 301 to /foo/
| > > GET /foo/ with auth -> 200
| > > GET /bar with auth .. (browser will send auth to other realms!)
| > >
| > >With the following patch it will exhibit this correct behavior:
| > > GET /foo -> 301 to /foo/
| > > GET /foo/ -> 401
| > > GET /foo/ with auth -> 200
| > > GET /bar WITHOUT auth
| > >
| > >
| > >I'll be glad to ci it, but those more in the know may
| > >have a better location for the fix in mind.
| > >
| > >Keith
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: auth bug fix for 4.0.6
Posted by Remy Maucherat <re...@apache.org>.
Bill Barker wrote:
> As a non-4.x expert, your patch looks ok. I would guess that it would
> still
> have problems with a request to /foo/protected where the
> security-constraint
> is only for /foo/protected/*.
I don't agree, the patch is bad for 4.1.x and 5.0 (at least, you must
use the decoded URI there). Tomcat 4.0.x is probably ok.
I also don't agree with Keith's interpretation depending on what the
constraint is. Can you give examples ?
Remy
>
> >It turns out TC 4.0.6 has the same auth bug as 3.3--
> >it challenges prior to redirects. The immediate problem
> >this causes is that some browsers will cache and send
> >credentials for the entire domain after being challenged
> >for a top level directory without a trailing slash.
> >
> >So 4.0.6 exhibits this wrong behavior:
> > GET /foo -> 401
> > GET /foo with auth -> 301 to /foo/
> > GET /foo/ with auth -> 200
> > GET /bar with auth .. (browser will send auth to other realms!)
> >
> >With the following patch it will exhibit this correct behavior:
> > GET /foo -> 301 to /foo/
> > GET /foo/ -> 401
> > GET /foo/ with auth -> 200
> > GET /bar WITHOUT auth
> >
> >
> >I'll be glad to ci it, but those more in the know may
> >have a better location for the fix in mind.
> >
> >Keith
> >
> >
> >Index:
>
> catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java
>
> >===================================================================
> >RCS file:
>
> /home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/authenti
> cator/AuthenticatorBase.java,v
>
> >retrieving revision 1.23.2.5
> >diff -u -r1.23.2.5 AuthenticatorBase.java
> >---
>
> catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java
>
> >27 Feb 2002 17:42:58 -0000 1.23.2.5
> >+++
>
> catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java
>
> >8 Nov 2002 05:25:06 -0000
> >@@ -422,8 +422,18 @@
> > context.invokeNext(request, response);
> > return;
> > }
> > HttpRequest hrequest = (HttpRequest) request;
> > HttpResponse hresponse = (HttpResponse) response;
> >+
> >+ // Do not authenticate prior to redirects
> >+ String uri = ((HttpServletRequest)
>
> request.getRequest()).getRequestURI();
>
> >+ if (uri.length() > 0 && ! uri.endsWith("/") &&
> >+ uri.equals(request.getContext().getName())) {
> >+ context.invokeNext(request, response);
> >+ return;
> >+ }
> >+
> > if (debug >= 1)
> > log("Security checking request " +
> > ((HttpServletRequest)
> request.getRequest()).getMethod() +
>
> " " +
>
> >
> >--
> >To unsubscribe, e-mail:
>
>
>
> >For additional commands, e-mail:
>
>
>
>
>
> --
> To unsubscribe, e-mail:
> For additional commands, e-mail:
>
>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
RE: auth bug fix for 4.0.6
Posted by Keith Wannamaker <Ke...@Wannamaker.org>.
Very good eyes Bill, I agree. The correct fix would have to
incorporate both the context name and the constraint URI.
Keith
| -----Original Message-----
| From: Bill Barker [mailto:wbarker@wilshire.com]
| Sent: Friday, November 08, 2002 2:11 AM
| To: Tomcat Developers List
| Subject: Re: auth bug fix for 4.0.6
|
|
| I would guess that it would still
| have problems with a request to /foo/protected where the security-constraint
| is only for /foo/protected/*.
|
|
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>
Re: auth bug fix for 4.0.6
Posted by Bill Barker <wb...@wilshire.com>.
As a non-4.x expert, your patch looks ok. I would guess that it would still
have problems with a request to /foo/protected where the security-constraint
is only for /foo/protected/*.
----- Original Message -----
From: "Keith Wannamaker" <Ke...@Wannamaker.org>
To: <to...@jakarta.apache.org>
Sent: Thursday, November 07, 2002 9:36 PM
Subject: auth bug fix for 4.0.6
> It turns out TC 4.0.6 has the same auth bug as 3.3--
> it challenges prior to redirects. The immediate problem
> this causes is that some browsers will cache and send
> credentials for the entire domain after being challenged
> for a top level directory without a trailing slash.
>
> So 4.0.6 exhibits this wrong behavior:
> GET /foo -> 401
> GET /foo with auth -> 301 to /foo/
> GET /foo/ with auth -> 200
> GET /bar with auth .. (browser will send auth to other realms!)
>
> With the following patch it will exhibit this correct behavior:
> GET /foo -> 301 to /foo/
> GET /foo/ -> 401
> GET /foo/ with auth -> 200
> GET /bar WITHOUT auth
>
>
> I'll be glad to ci it, but those more in the know may
> have a better location for the fix in mind.
>
> Keith
>
>
> Index:
catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java
> ===================================================================
> RCS file:
/home/cvs/jakarta-tomcat-4.0/catalina/src/share/org/apache/catalina/authenti
cator/AuthenticatorBase.java,v
> retrieving revision 1.23.2.5
> diff -u -r1.23.2.5 AuthenticatorBase.java
> ---
catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java
> 27 Feb 2002 17:42:58 -0000 1.23.2.5
> +++
catalina/src/share/org/apache/catalina/authenticator/AuthenticatorBase.java
> 8 Nov 2002 05:25:06 -0000
> @@ -422,8 +422,18 @@
> context.invokeNext(request, response);
> return;
> }
> HttpRequest hrequest = (HttpRequest) request;
> HttpResponse hresponse = (HttpResponse) response;
> +
> + // Do not authenticate prior to redirects
> + String uri = ((HttpServletRequest)
request.getRequest()).getRequestURI();
> + if (uri.length() > 0 && ! uri.endsWith("/") &&
> + uri.equals(request.getContext().getName())) {
> + context.invokeNext(request, response);
> + return;
> + }
> +
> if (debug >= 1)
> log("Security checking request " +
> ((HttpServletRequest) request.getRequest()).getMethod() +
" " +
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
> For additional commands, e-mail:
<ma...@jakarta.apache.org>
>
--
To unsubscribe, e-mail: <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>