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
>
>