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. > > 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)(cn=web.example\29.com)(member=uid= john_doe ,ou=people,dc=example,dc=com))', searchbase 'ou=authorization,dc=example, dc=com' 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. You mean these <http://www.squid-cache.org/Doc/config/logformat/> logformats are available to be used in acl / external acls @ squid.conf? Or? ..... > > 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)(test=%test)(member=uid=%u,ou=p eople,dc=example,dc=com))': ERROR: Unknown filter template string %t ext_ldap_group_acl: ERROR: Failed to construct LDAP search filter. filter="(&(objectclass=groupOfNames)(test=?,?U", user="john_doe", 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-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. 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_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 > 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.org > http://lists.squid-cache.org/listinfo/squid-users -- -------- Diogenes S. de Jesus
_______________________________________________ squid-users mailing list squid-users@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-users
