Costin Manolache wrote:
I'm not sure the security discussion is that simple, this seems quite a
dangerous change.
Currently  the user is restricted  to the webapps/ directory ( well,  he can
add a context
with the base in /etc and expose passwd I guess - but hopefully if a deploy
tool is used
or some automation is done on adding webapps, it can be controlled ). At
least this
introduces one more risk.
what does httpd do when you do set up an alias or document root for the /etc directory? Since it does, would that mean that httpd should not include the Alias feature? This is the same scenario, adding a useful feature, though it can go wrong when when misconfigured, doesn't mean we shouldn't do it.
Tomcat already would allow you do to docBase=/etc"
so the risk already exists, and no, the user is not restricted to the webapps directory.

It may also have some implications on other use cases - deployment ( the
current pattern
is that all files for a webapp are in one place ), replication ( i.e. if
someone wants same webapps
on a pool of servers ).

Also questions about servlet API - are the aliases considered as part of the
webapp and returned
as resources ? Portability ? Will anything break or be confusing ?
This is important, it shouldn't break anything from spec compliant, but that goes with every single feature we put in Tomcat.
If it does break the spec, then a -1 is justified in a heartbeat.
But the most important question I have - why at this level ? If you have
files in a different
 directory - just add a servlet that is configured to serve files from
there, doesn't have to
be a tomcat-specific feature.
Back to the httpd analogy, that would be equivalent of saying, remove the Alias feature, and write a perl/php/asp script that does this for you I find the feature useful, and if it doesn't go against the spec, I've seen the feature requested on tomcat-user as well.
Filip
Costin

On 9/14/07, Tim Funk <[EMAIL PROTECTED]> wrote:
The basic concept behind the logic is:
- Look for prefix
- Replace with new base path

The new path replacement is done before the symlink and case check is
done so those checks are still preserved. (Just with the new path alias).

If there is a security manager in play - it should already be blocking
access to directories outside of one's control.

For file/directory redirection such as replacing requesting /foo and
then redirecting to /foo/ - I will add (if this is accepted) an entry to
the FAQ to strongly recommend replacements include the trailing / - such
that a user shoudl enter: /foo/bar/=/docs/tmp/ AND NOT
/foo/bar=/docs/tmp since wierd behaviors can arise since a request of
/foo/barry/file.pdf would yield /docs/tmpry/file.pdf (notice the last ry
being preserved). I left this behavior in instead of requiring a
trailing / since ... well if someone wishes to shoot themself in the
foot - I won't stop them.

For /WEB-INF/ handling - this does provide an ability to have WEB-INF
live somewhere else on the filesystem other than the webapp. When I
first made the patch - I thought about making an exception for that -
but then decided not to since I couldn't ponder a reason security wise
to do so.

From a security point of view - the only gotcha I see is in a shared
environment where you might suck in part of someone else's webapp in a
shared environment. If this can be done - then it would be worthwhile
adding a flag which would allow any aliasing to occur. Or a simpler
alternative is if you are running with a security manager turned on -
then aliasing is disabled.


-Tim

Remy Maucherat wrote:
Remy Maucherat wrote:
It's not a real veto anyway, but no proper review mechanism exists at
the moment, and it's hard to integrate feature additions in 6.0.x
without prior discussion.
I did review the patch:
- the syntax seems appropriate
- I don't know if it allows redirecting a single fine, but I think it
should if it does not (I did not test it; at least the list feature
would not be working right now)
- it seems like it will still validate going out of the remapped "base"
path, which is good
- interaction with the webapp classloader, which might have special
handling for /WEB-INF on the file based resources, is a question mark
(compatibility with that would be good, if possible)
- security wise, it needs to be verified if the security manager
prevents usage of the feature (normally it should, there are no
privileged actions)
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]



------------------------------------------------------------------------

No virus found in this incoming message.
Checked by AVG Free Edition. Version: 7.5.487 / Virus Database: 269.13.19/1008 - Release Date: 9/14/2007 8:59 AM


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to