dev@glassfish.java.net

Re: REST API and slashes in resource names

From: Oleksiy Stashok <Oleksiy.Stashok_at_Sun.COM>
Date: Thu, 03 Jun 2010 16:36:33 +0200

If there will be no objections regarding the actual idea of having
allowEncodedSlash configurable per <http> protocol, I think we can
have that implemented very quickly like today/tomorrow.

Thanks.

WBR,
Alexey.

On Jun 3, 2010, at 16:22 , Jason Lee wrote:

> 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
>>>> Blog http://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