dev@glassfish.java.net

Re: REST API and slashes in resource names

From: Jason Lee <jason.d.lee_at_oracle.com>
Date: Thu, 03 Jun 2010 09:22:42 -0500

That works for me. Hardcoding UDecoder(true) was mostly for PoC purposes.

Is everyone OK with the http protocol configuration change? If so, who
will implement that and when can it be done. I think, for now, I can
continue what I'm working on without this change (which depends on the
integration of a new version of Grizzly anyway), but we'll need that
fairly quickly, as tests will fail without it, which we must have for
our MS2 commitment.

Thanks!

On 6/3/10 4:01 AM, Oleksiy Stashok wrote:
>
>> QL is not good enough. You may like to run web devtest in this case.
>>
>> We may like to consider whether we should UDecoder initialized
>> depending on the connector
>> rather than it is fixed ("true") all the time.
> Agree. Because if we will just apply
>
>>> - private final UDecoder urlDecoder = new UDecoder();
>>> + private final UDecoder urlDecoder = new UDecoder(true);
>
> we will enable slashesEncoding for *all* listeners, not just admin.
> I like the idea to introduce "allowEncodedSlash" (or to be consistent
> "encodedSlashEnabled") attribute per network-listener, or better per
> <http> protocol.
> Here is the snippet how network config looks like [1], where we have
> <http> protocol description for each network listener. We can
> introduce additional <http> element's attribute "allowEncodedSlash",
> which will be *false* by default. This way we will have
> allowEncodedSlash parameter per listener.
>
> Finally <http> protocol description for the admin listener will look like:
>
> <protocol name="admin-listener">
> <http default-virtual-server="__asadmin" max-connections="250"
> *allowEncodedSlash="true"*>
> <file-cache />
> </http>
> </protocol>
>
> WBR,
> Alexey.
>
> [1]
> <network-config>
> <protocols>
> <protocol name="http-listener-1">
> <http default-virtual-server="server">
> <file-cache />
> </http>
> </protocol>
> <protocol security-enabled="true" name="http-listener-2">
> <http default-virtual-server="server">
> <file-cache />
> </http>
> <ssl ssl3-enabled="false" cert-nickname="s1as" />
> </protocol>
> * <protocol name="admin-listener">*
> * <http default-virtual-server="__asadmin" max-connections="250">*
> * <file-cache />*
> * </http>*
> * </protocol>*
> </protocols>
> <network-listeners>
> <network-listener port="${HTTP_LISTENER_PORT}"
> protocol="http-listener-1" transport="tcp" name="http-listener-1"
> thread-pool="http-thread-pool" />
> <network-listener port="${HTTP_SSL_LISTENER_PORT}"
> protocol="http-listener-2" transport="tcp" name="http-listener-2"
> thread-pool="http-thread-pool" />
> <network-listener port="${ASADMIN_LISTENER_PORT}"
> protocol="admin-listener" transport="tcp" name="admin-listener"
> thread-pool="http-thread-pool" />
> </network-listeners>
>
>
>
>>
>> Shing Wai Chan
>>
>> On 6/2/10 12:43 PM, Jason Lee wrote:
>>> On 6/2/10 11:59 AM, Abhijit Kumar wrote:
>>>> I have to jump in here. The vulnerability is around being able to
>>>> traverse outside of the so called "document root" of the server.
>>>> Using an encoded URL to get to a file under "document root" is not
>>>> a vulnerability.
>>>>
>>>> IMHO, the answer is fairly simple (and sorry if this was already
>>>> discussed). Grizzly should implement a feature to allow
>>>> configuration of this feature per connector. Admin connector (port
>>>> 4848) should be configured to allow %2F and all the applications
>>>> running there should make sure that combination of encoded ".." and
>>>> "/" can not be used to get to unauthorized areas of the server.
>>> I have a status report on two approaches. The first approach --
>>> disable decoding in ContainerMapper and pushing it to the Adapter --
>>> works, it seems. It does, at least for the RestAdapter, and
>>> QuickLook didn't seem to be bothered by it. I'm a little worried,
>>> though, that it might break something else that's not covered by the
>>> QL tests.
>>>
>>> The second approach, allowing encoded slashes for EVERY Adapter on
>>> 4848, also seems to work. The diff for that is pretty simple:
>>>
>>> Index:
>>> kernel/src/main/java/com/sun/enterprise/v3/services/impl/ContainerMapper.java
>>> ===================================================================
>>> ---
>>> kernel/src/main/java/com/sun/enterprise/v3/services/impl/ContainerMapper.java
>>> (revision 37388)
>>> +++
>>> kernel/src/main/java/com/sun/enterprise/v3/services/impl/ContainerMapper.java
>>> (working copy)
>>> @@ -81,7 +81,7 @@
>>> private Mapper mapper;
>>> private GrizzlyEmbeddedHttp grizzlyEmbeddedHttp;
>>> private String defaultHostName = "server";
>>> - private final UDecoder urlDecoder = new UDecoder();
>>> + private final UDecoder urlDecoder = new UDecoder(true);
>>> private final Habitat habitat;
>>> private final GrizzlyService grizzlyService;
>>> protected final static int MAPPING_DATA = 12;
>>>
>>> Once I updated the jar in my GF instance, URLs like this now work:
>>>
>>> http://localhost:4848/management/domain/resources/admin-object-resource/jndi%2Ffoo
>>>
>>> URLs like this, though, do not:
>>>
>>> http://localhost:4848/management%2F..%2Fmonitoring/domain/server
>>> http://localhost:4848/management/..%2Fmonitoring/domain/server
>>> http://localhost:4848/management%2F../monitoring/domain/server
>>>
>>> For the record, this does:
>>>
>>> http://localhost:4848/management/../monitoring/domain/server
>>>
>>> I'm not sure if that's a problem or not.
>>>
>>> Another approach would be to get the Adapter before decoding, create
>>> a new UDecoder on every request (new
>>> UDecoder(adapter.isAllowEncodedSlash()), then decode the URL. While
>>> that shrinks any potential attack surface, it may also run afoul of
>>> the possibility of the multibyte characters Shing Wai mentioned,
>>> which may or may not be an issue with the GF administration port.
>>>
>>> Now, having said all of that, we really need to make a decision so
>>> this can move on. Aesthetically, single encoding looks better, and
>>> it's certainly more intuitive to the end user. In case this hasn't
>>> been clear, though, I'm a bit nervous making changes like this due
>>> to unintended consequences. I've tried to exploit this change to
>>> get to things I'm not supposed to, but haven't been able to yet, for
>>> what it's worth.
>>>
>>> The double encoding approach, while slightly uglier, also works and
>>> seems to be free of any security concerns. I also have it
>>> implemented already. :P
>>>
>>> I don't have a strong preference either way, so I'm perfectly
>>> content with adopting whatever the general consensus here is, which
>>> I'd like to determine as quickly as we can, as we've spent a lot of
>>> time on this already, and this issue is holding up MS2 deliverables.
>>>
>>> Let the voting commence. :)
>>> --
>>> Jason Lee
>>> Senior Member of Technical Staff
>>> GlassFish Administration Console
>>>
>>> Oracle Corporation
>>> Phone +1 405-216-3193
>>> Bloghttp://blogs.steeplesoft.com
>>
>


-- 
Jason Lee
Senior Member of Technical Staff
GlassFish Administration Console
Oracle Corporation
Phone +1 405-216-3193
Blog http://blogs.steeplesoft.com