Hi there. First thanks for taking the time to thoroughly reply to it.
>> external_acl_type ldap_HTTP %LOGIN %URI
>> /usr/lib/squid/ext_ldap_group_acl -D "cn=admin,dc=example,dc=com" -w
>> test -R -b "ou=authorization,dc=example,dc=com" -B
>> "ou=people,dc=example,dc=com" -f
>> '(&(objectclass=groupOfNames)(cn=%g)(member=uid=%u,ou=people,dc=example,dc=com))'
>> -h ldap01.example.com -d
>
>
> Please be aware that the %URI format does not perform any type of shell
> or LDAP escaping to protect this helper lookup against shell-injection
> attacks.
I have since the message was sent to the mailing list stopped using "%URI and changed to "%DST" - only because %URI will also add scheme and for SSL, port number.
>> external_acl_type ldap_HTTP %LOGIN %URI
>> /usr/lib/squid/ext_ldap_group_
>> test -R -b "ou=authorization,dc=example,
>> "ou=people,dc=example,dc=com" -f
>> '(&(objectclass=groupOfNames)(
>> -h ldap01.example.com -d
>
>
> Please be aware that the %URI format does not perform any type of shell
> or LDAP escaping to protect this helper lookup against shell-injection
> attacks.
>
> It is possible that a remote client can end a URL with ')' followed by
> any LDAP commands they like and have that executed by your helper.
> It is possible that a remote client can end a URL with ')' followed by
> any LDAP commands they like and have that executed by your helper.
I was also concerned about shell injection and LDAP injection but:
- group value is not really passed as shell argument but read from stdin AFAIU
- I could not see ")" reflected in the LDAP filter. When performing the following request, for example:
$ curl --proxy-negotiate --negotiate -u : http://web.example")".com/
I see the following lines in the debug log:
ext_ldap_group_acl.cc(579): pid=31325 :Connected OK
ext_ldap_group_acl.cc(718): pid=31325 :group filter '(&(objectclass=groupOfNames)(
That's because "group" is ldap-escaped when building the LDAP search filter (https://github.com/squid-cache/squid/tree/master/helpers/external_acl/LDAP_group#L654 ) AFAIU.
I have since the message was sent to the mailing list stopped using "%URI and changed to "%DST" - only because %URI will also add scheme and for SSL, port number.
Regardless, your point may still be valid for those passing argument to the binary. Minor pentests I did didn't show much of a security risk here.
> If you want to do things like this safely please upgrade to Squid-4
> where the logformat codes are available. Those codes provide
> customizable escaping and quoting styles so you can set one that
> protects LDAP against these attacks to be ued on the URI field value
> sent by Squid.
ext_ldap_group_acl: ERROR: Failed to construct LDAP search filter.
filter="(&(objectclass=groupOfNames)(test=?,?U", user="john_doe",
>
> So somehow the HTTP request URL/URI is the whole string:
> "GET http://web.example.com/ 80"
>
> -> very odd. It is not even a valid HTTP request-line. Looks more like
> some outdated Squid-1.1 URL re-write helper is mangling the URL. But the
> order is slightly wrong even for that (GET would be after the actual URL).
>
>
> Anyhow, that resuls in the ACL group helper receiving:
> john_does GET http://web.example.com/ 80
>
> Meaning,
> username: "john_doe"
> group #1: "GET"
> group #2: "http://web.example.com/"
> group #3: "80"
>
>
>>
>> This is all pretty much happening here
>> [https://github.com/squid-cache/squid/blob/master/helpers/external_acl/LDAP_group/ext_ldap_group_acl.cc#L638 ]
>>
>> So conclusions:
>> - %v and %u both map to "user", which is expected (historical reasons
>> & compatibility)
>
> As documented.
>
>> - %g and %a both map to "group", which is expected (historical reasons
>> & compatibility)
>
> As documented.
>
>> - any other template filter (%b, %c, %test, etc) is trash (only %a,
>> %u, %g, %v won't yield error)
>
> Nod. The helper autor(s) reserve other %-code to be defined with any
> arbitrary meaning at any time. So there is simply no documented
> behaviour for them.
> A you found v and a have old meanings that are still supported, though
> deprecated so removed from the documentation intentionally to prevent
> future use.
>
>
>> - when "" is passed to the acl ("acl <ACL_name> external ldap_HTTP
>> ""), the helper will attempt all FORMAT values, mapping then to
>> "group" (%a or %g)
>
> It should mean Squid loads a file with undefined name (\0) and sends
> that files content as the list of group names. Each line in the file
> being a group name.
>
>
>>
>> Although I can move on with this for now, I would be actually more
>> relieved if I could use:
>> acl allow_HTTP_ACL external ldap_HTTP
>> <a_var_which_is_available_here_representing_URI>
>> instead of
>> acl allow_HTTP_ACL external ldap_HTTP "" + non-documented behavior of
>> ext_ldap_group_acl
>
> Dont specify anything at all in that position. The %URI field you
> defined to be sent to the helper should be formatted in the place it
> expects to find a group name "word" as mentioned above.
>
> Just use:
> acl allow_HTTP_ACL external ldap_HTTP
>
>
> Amos
>
> _______________________________________________
> squid-users mailing list
> squid-users@lists.squid-cache.org
> http://lists.squid-cache.org/listinfo/squid-users
--
--------
Diogenes S. de Jesus
> If you want to do things like this safely please upgrade to Squid-4
> where the logformat codes are available. Those codes provide
> customizable escaping and quoting styles so you can set one that
> protects LDAP against these attacks to be ued on the URI field value
> sent by Squid.
.....
>
> What is happening is that the helper expects the %LOGIN field to be
> followed by a list of space-separated 'words'. Each 'word' is a group
> name to be checked against the users account memberships. So the list of
> words is looked up individually until one matches or none left to check.
> What is happening is that the helper expects the %LOGIN field to be
> followed by a list of space-separated 'words'. Each 'word' is a group
> name to be checked against the users account memberships. So the list of
> words is looked up individually until one matches or none left to check.
I forgot to mention that for the debugging I used the following:
external_acl_type ldap_HTTP %LOGIN %URI %METHOD %PORT
So it's perfectly in line with what you mentioned.
So in:
4) '(&(objectclass=groupOfNames)(
ext_ldap_group_acl: ERROR: Failed to construct LDAP search filter.
filter="(&(objectclass=groupOf
group="http://web.example.com/
The garbage is here: ?,?U",
> So somehow the HTTP request URL/URI is the whole string:
> "GET http://web.example.com/ 80"
>
> -> very odd. It is not even a valid HTTP request-line. Looks more like
> some outdated Squid-1.1 URL re-write helper is mangling the URL. But the
> order is slightly wrong even for that (GET would be after the actual URL).
>
Sorry - that's because the "%URI %METHOD %PORT" was added as FORMAT, as I mentioned above.
>
> Anyhow, that resuls in the ACL group helper receiving:
> john_does GET http://web.example.com/ 80
>
> Meaning,
> username: "john_doe"
> group #1: "GET"
> group #2: "http://web.example.com/"
> group #3: "80"
>
>
>>
>> This is all pretty much happening here
>> [https://github.com/squid-
>>
>> So conclusions:
>> - %v and %u both map to "user", which is expected (historical reasons
>> & compatibility)
>
> As documented.
>
>> - %g and %a both map to "group", which is expected (historical reasons
>> & compatibility)
>
> As documented.
>
>> - any other template filter (%b, %c, %test, etc) is trash (only %a,
>> %u, %g, %v won't yield error)
>
> Nod. The helper autor(s) reserve other %-code to be defined with any
> arbitrary meaning at any time. So there is simply no documented
> behaviour for them.
> A you found v and a have old meanings that are still supported, though
> deprecated so removed from the documentation intentionally to prevent
> future use.
>
>
>> - when "" is passed to the acl ("acl <ACL_name> external ldap_HTTP
>> ""), the helper will attempt all FORMAT values, mapping then to
>> "group" (%a or %g)
>
> It should mean Squid loads a file with undefined name (\0) and sends
> that files content as the list of group names. Each line in the file
> being a group name.
Oh, ok. Got that now.
>
>
>>
>> Although I can move on with this for now, I would be actually more
>> relieved if I could use:
>> acl allow_HTTP_ACL external ldap_HTTP
>> <a_var_which_is_available_
>> instead of
>> acl allow_HTTP_ACL external ldap_HTTP "" + non-documented behavior of
>> ext_ldap_group_acl
>
> Dont specify anything at all in that position. The %URI field you
> defined to be sent to the helper should be formatted in the place it
> expects to find a group name "word" as mentioned above.
>
> Just use:
> acl allow_HTTP_ACL external ldap_HTTP
>
I had tried that early on without success - clearly the problem was somewhere else as this indeed proves to work now. Thanks.
>
> Amos
>
> ______________________________
> squid-users mailing list
> squid-users@lists.squid-cache.
> http://lists.squid-cache.org/
--
--------
Diogenes S. de Jesus
_______________________________________________ squid-users mailing list squid-users@xxxxxxxxxxxxxxxxxxxxx http://lists.squid-cache.org/listinfo/squid-users
