Bug 34560 - AuthenticatorBase tests and applies disableProxyCaching even if no auth-constraints
Summary: AuthenticatorBase tests and applies disableProxyCaching even if no auth-const...
Status: RESOLVED WONTFIX
Alias: None
Product: Tomcat 5
Classification: Unclassified
Component: Catalina (show other bugs)
Version: 5.0.24
Hardware: Other other
: P2 major (vote)
Target Milestone: ---
Assignee: Tomcat Developers Mailing List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-04-22 04:08 UTC by quartz
Modified: 2006-09-24 06:33 UTC (History)
0 users



Attachments
patch to head cvs (1.30) (27.27 KB, patch)
2005-04-24 00:13 UTC, quartz
Details | Diff
'diff' version of the patch (833 bytes, patch)
2005-04-24 18:56 UTC, quartz
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description quartz 2005-04-22 04:08:42 UTC
The web.xml contains

	<security-constraint>
			<display-name>Security Constraint</display-name>
			<web-resource-collection>
				<web-resource-name>HTTP Non Protected Area</web-resource-name>
				<url-pattern>/favicon.ico</url-pattern>
				<url-pattern>*.gif</url-pattern>
				<url-pattern>*.js</url-pattern>
				<url-pattern>*.html</url-pattern>
				<url-pattern>*.css</url-pattern>
				<url-pattern>/css/*</url-pattern>
				<url-pattern>/images/*</url-pattern>
				<url-pattern>/js/*</url-pattern>
			</web-resource-collection>
	 		<user-data-constraint>
	 			<transport-guarantee>
	 				CONFIDENTIAL
	 			</transport-guarantee>
	 		</user-data-constraint>
	</security-constraint>

Although it is https (CONFIDENTIAL), it doesn't have any
<auth-constraint><role-name>...
yet the valve FormAuthenticator (extends AuthenticatorBase, 5.0.24, line 458)
only tests for existence of constraints, not roles:

       if ((constraints == null) /* &&
            (!Constants.FORM_METHOD.equals(config.getAuthMethod())) */ ) {
            if (log.isDebugEnabled())
                log.debug(" Not subject to any constraint");
            context.invokeNext(request, response);
            return;
        }

        // Make sure that constrained resources are not cached by web proxies
        // or browsers as caching can provide a security hole
        HttpServletRequest hsrequest = (HttpServletRequest)hrequest.getRequest();
        if (disableProxyCaching && 
            // FIXME: Disabled for Mozilla FORM support over SSL 
            // (improper caching issue)
            //!hsrequest.isSecure() &&
            !"POST".equalsIgnoreCase(hsrequest.getMethod())) {
            HttpServletResponse sresponse = 
                (HttpServletResponse) response.getResponse();
            sresponse.setHeader("Pragma", "No-cache");
            sresponse.setHeader("Cache-Control", "no-cache");
            sresponse.setHeader("Expires", DATE_ONE);
        }


As a result, it is not allowing caching of static ressources in the patterns.
(Slow site performance)
Comment 1 william.barker 2005-04-22 21:05:21 UTC
You might have a case for INTEGRAL, but not CONFIDENTIAL.  But even then it's 
a corner-case that is unlikely to attract much developer interest.

You can always configure disableProxyCaching="false" on the Authenticator to 
disable this, or add a Filter that overrides Tomcat's choice of headers.
Comment 2 quartz 2005-04-23 04:56:51 UTC
In order to respect the authentication spec rfc2616-14.8, although the
authorization is made by a form and not a header, the FormAuthenticator valve
was capable of emulating the proper caching constraints. The code is
manipulating the correct headers but under innacurate circumstances.

The problem is not related to the <user-data-constraint><transport-guarantee>
tags. It has to do with the abscence of <auth-constraint><role-name> tags.

The FormAuthenticator valve is visited for mappings that do not require
authentication. That alone is questionnable, but assuming the valve may perform
other contract, I supposed this visit is unavoidable. However, within the
mandate of performing authentication based operations, the valve should restrict
itself to mappings that strictly have at least 1 role.

Like I said, every tomcat application out there is silently suffering from
non-cached static ressources because:
1-the valve intercepts EVERY request, even if not matching the url pattern
AND 
2-the valve do not recognize the abscence of authentication constraints.

Thanks for reconsidering.

PS:...especially since the fix is trivial:
(skip if constraints==null || constraints.length=0 || all of
constraints[i].getAuthConstraint()==false)

PS:You might want to consult http://www.mnot.net/cache_docs/
and other doc like the rfc 2616
http://www.w3.org/Protocols/rfc2616/rfc2616.html
http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.9
http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.8
Comment 3 Remy Maucherat 2005-04-23 09:40:15 UTC
If the fix is trivial, and you want us to reconsider, you should submit a tested
patch against CVS HEAD.
Comment 4 quartz 2005-04-24 00:13:54 UTC
Created attachment 14814 [details]
patch to head cvs (1.30)

line 437:

	//+++++ ASF Bugzilla Bug 34560 fix:
	boolean requireAuthentication = false;
	for(i=0; i < constraints.length; i++) {
	    if (constraints[i].getAuthConstraint()) {
		requireAuthentication=true;
		break;
	    }
	}
	//+++++


	// Make sure that constrained resources are not cached by web proxies
	// or browsers as caching can provide a security hole
	if (requireAuthentication && disableProxyCaching && //+++++ SSL can be
cached (by browser only, and by user choice), authenticated resources must not.


[...]
Comment 5 william.barker 2005-04-24 01:36:11 UTC
Please see http://jakarta.apache.org/site/source.html#Patches for the format 
that patches should be submitted in.

That having been said, I'm -1 for the patch as is.  As I read section 12.8 of 
the servlet spec, the headers should be added for a transport-guarantee of 
CONFIDENTIAL.  I agree that it is optional for a transport-guarantee of 
INTEGRAL.
Comment 6 quartz 2005-04-24 18:26:39 UTC
Thank you for your dedication and research.
I read that servlet spec 12.8.

It is very clear to me that the transport constraint is orthogonal to the
authentication constraint.

That is, a 'confidential' transport may not obviously require authentication.
That is especially true for web site that are fully https to avoid mixed
secure/unsecure content warnings on browsers, while allowing decent caching for
ressources that do not need authentication/autorization, like js, css, gifs...

I'm not suggesting to change any of the current logic surrounding
confidential/integral/none. I'm highlighting that the 'de-caching' headers must
only be applied when the authentication is required, which has nothing to do
with transport contraints.

Meanwhile, the http spec is stating that autorization must be challenged
everytime and resources, if cached, cannot bypass the authentication. It doesn't
mention anything specific to the ssl nature (or else) of the lower layer
transporting http content.

Thanks again.
Comment 7 quartz 2005-04-24 18:56:33 UTC
Created attachment 14824 [details]
'diff' version of the patch

diff made with textpad, a nice alternative to the non-pc-friendly ways
suggested.
Comment 8 Yoav Shapira 2005-12-01 23:02:27 UTC
Bill, does the 2nd patch look reasonable?
Comment 9 william.barker 2005-12-02 00:47:12 UTC
(In reply to comment #8)
> Bill, does the 2nd patch look reasonable?

Actually, we're doing the authRequired check now already :).

It's still my opinion that, given that we are setting these headers, they can 
be omitted for INTEGRAL.  However, not setting them for CONFIDENTIAL seems to 
defeat the purpose of CONFIDENTIAL (since that is what these headers do :).

The lack of a distiction between INTEGRAL and CONFIDENTIAL is why I haven't 
touched it.  However, I won't veto it if you want to apply it as is (I set 
disableProxyCaching="false" personally, so it doesn't affect me either way :).
Comment 10 quartz 2005-12-02 03:05:30 UTC
Let me try to be short and clear with the following summary.

Transport constraints (CONFIDENTIAL, NONE) make it https or not. It has nothing
to do with auth.constraints (identification of roles).

In real life, you can be anonymous over http, anonymous over https, logged in
over http, logged in over https. This is the orthogonality I was referring to,
in case I wasn't clear.

Now, the headers in questions are not about transport nor authentication at all.
They are about caching. HOWEVER, caching must not occur in front of
aurhenticated ressources, since the authentication must be challenged on every
hit from end to end (user-agent to server).

Therefore, anti-caching headers must be added only to authenticated ressources,
NO MATTER what the transport constraint is.

Sorry if I sound irritated, the issue is fairly simple. So is the fix. Without
that fix, tomcat will never be able to perform well under https as demonstrated
in the same xml above.

The reality is horrible: all page components like css, js, gif, jpg, etc...
served under https (to avoid mixed security page warnings) are polled every
refresh. They may result in 304, but that is already too much since it took a
full network round trip for every resource. Please trust me, it is night and day
after we hacked in a 're-caching' servletfilter workaround.
Comment 11 Mark Thomas 2006-09-23 21:05:33 UTC
The specs do state that authenticated resources must not be cached. However,
this does not mean that all unauthenticated resources may be cached. If you can
provide a reference in a relevant specification that does state this then you
have a much stronger case.

The servlet spec does not specify caching behaviour for CONFIDENTIAL. Given the
meaning of confidential it is arguable that it should not be cached it in order
to keep it private. Without a clear specification breach (which I don't see
having reviewed the specs quoted in this report) this behaviour is always going
to be open to interpretation.

If you don't like the behaviour as currently interpretted there are a number of
options (disableProxyCaching, securePagesWithPragma) you can use or if these
don't give you want you want you can, as the you and others have, implement a
filter to set the cache control headers.
Comment 12 quartz 2006-09-24 13:33:01 UTC
(In reply to comment #11)
> The servlet spec does not specify caching behaviour for CONFIDENTIAL.

Because https transport has nothing to do with caching... You are mixing
attributes of layer 6 (presentation/encryption) and layer 7 (application/http)
of the OSI protocol stack (higher layers being merely the payload of the lower
layers, i.e. the russian dolls metaphor).
http://www.webopedia.com/quick_ref/OSI_Layers.asp
If you want to discuss caching (and authentication), you must locate yourself at
the layer 7. Therefore the transport choice is irrelevant and the http spec
prevails: caching headers aren't mandatory and so are added as an explicit
choice by the servers/proxies. As it stands, tomcat disregards that choice and
requires extra configuration to remove http headers.


> Given the meaning of confidential it is arguable that it should
> not be cached it in order to keep it private.

This is a browser-side privacy issue, a user preference, not a server issue.

You can assert the concepts exposed here with anyone with a bachelor degree in
computer science (or electrical engineering with networking speciality). Maybe
http and servlets don't sound very OSI, but they do partition communication
roles. I don't know what else I can give you to help you understand how
CONFIDENTIAL transport is not related to caching.

But in the end, the fact that both use-case are legitimate enforces the fact
that caching of unauthenticated confidential resources must be configurable. My
point is to provide the performant alternative, not the slower one and force
users to cope with extraneous hits, server load and exra servlet filters and config.