admin@glassfish.java.net

Re: REST API and slashes in resource names

From: Jason Lee <jason.d.lee_at_oracle.com>
Date: Mon, 21 Jun 2010 23:16:06 -0500

I think I found the place. Here's the diff:

Index: packager/nucleus-base/lib/templates/domain.xml
===================================================================
--- packager/nucleus-base/lib/templates/domain.xml (revision 38014)
+++ packager/nucleus-base/lib/templates/domain.xml (working copy)
@@ -179,7 +179,7 @@
<ssl ssl3-enabled="false" cert-nickname="s1as"></ssl>
</protocol>
<protocol name="admin-listener">
- <http default-virtual-server="__asadmin" max-connections="250">
+ <http default-virtual-server="__asadmin" max-connections="250"
encoded-slash-enabled="true" >
<file-cache enabled="false"></file-cache>
</http>
</protocol>

Without this change, REST tests as Admin Console tests fail (which
indicates, of course, that a feature is not working correctly). This
does affect MS2 deliverables.

Are there complaints with me committing this change?

On 6/21/10 5:39 PM, Jason Lee wrote:
> All the pieces now seem to be in place, and I've verified that things
> are working. We now need to set this option to true for the admin
> listener. Where is that done, and who can do that?
>
> asadmin set
> configs.config.server-config.network-config.protocols.protocol.admin-listener.http.encoded-slash-enabled=true
>
> asadmin stop-domain
> asadmin start-domain
> curl
> http://localhost:4848/management/domain/resources/jdbc-resource/jdbc%2F__TimerPool.xml
>
>
> On 6/21/10 10:45 AM, Justin Lee wrote:
>> The support glassfish changes are in now. the allowEncodedSlash web
>> devtest exercises this new code if you need a reference.
>>
>> On 6/21/10 10:34 AM, Anissa Lam wrote:
>>>
>>> Thanks !!
>>>
>>> Anissa.
>>>
>>> Ryan Lubke wrote:
>>>> On 6/20/10 3:28 PM, Anissa Lam wrote:
>>>>>
>>>>> Thanks for the update, it comes so quickly :)
>>>>> Hope the devtests can pass soon.
>>>> Integration was completed yesterday evening. Please let us know if
>>>> you run into any issues.
>>>>
>>>> Thanks,
>>>> -rl
>>>>>
>>>>> thanks
>>>>> Anissa.
>>>>>
>>>>> Justin Lee wrote:
>>>>>> We released a new grizzly release with this fix. Ryan is working
>>>>>> on integrating it into glassfish but is having some issues with
>>>>>> the devtest run last I heard. I know, personally, when I run the
>>>>>> devtests, I get timeouts on start up after running most of the
>>>>>> tests. The tests try to restart the server and the server is
>>>>>> never heard from again.
>>>>>>
>>>>>> On 6/20/10 6:18 PM, Anissa Lam wrote:
>>>>>>>
>>>>>>> Hi Justin /Oleksiy,
>>>>>>>
>>>>>>> I would like to know whats the plan for integrating the version
>>>>>>> of Grizzly that has this issue fixed.
>>>>>>> According to Justin's comment, the issue was fixed on 6/10. GUI
>>>>>>> has been waiting for the integration and currently the JDBC
>>>>>>> resources screen is broken. I really hope that the integration
>>>>>>> can happen for MS2 build, otherwise, a very common and
>>>>>>> heavily used functionality in GUI will be broken for MS2.
>>>>>>>
>>>>>>> thanks
>>>>>>> Anissa.
>>>>>>>
>>>>>>> Jason Lee wrote:
>>>>>>>> Issue filed (12116). Thanks.
>>>>>>>>
>>>>>>>> On 6/3/10 10:00 AM, Justin Lee wrote:
>>>>>>>>> I'm currently adding a new attribute to network-listener so
>>>>>>>>> file an issue for this and assign it to me.
>>>>>>>>>
>>>>>>>>> On 6/3/10 10:36 AM, Oleksiy Stashok wrote:
>>>>>>>>>> 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
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> ---------------------------------------------------------------------
>>>>>>>>>
>>>>>>>>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>>>>>>>>> For additional commands, e-mail:
>>>>>>>>> admin-help_at_glassfish.dev.java.net
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: admin-unsubscribe_at_glassfish.dev.java.net
>> For additional commands, e-mail: admin-help_at_glassfish.dev.java.net
>>
>
>


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