Re: [PATCH] Strip Windows domain in PAM basic authenticator

On 6/03/2013 12:19 p.m., Amos Jeffries wrote:
> On 6/03/2013 7:17 a.m., Steve Hill wrote:
>>
>> This might be slightly controversial... :)
>
> Very slightly. Mostly on the grounds that NTLM is an officially
> deprecated protocol - adding improved support for it it counter
> productive to the goals of eradicating it from the Internet.
>
> On the security front, altering the helper is okay. It is the process
> responsible for all manipulation of the credentials received. As long
> as the user:password token received in HTTP by Squid is passed to it
> without manipulation there is nothing to worry about in respects to
> Squid.
>
>> When accessing Squid from a Windows machine that is not logged onto a
>> domain, Internet Explorer presents the user with a proxy
>> authentication dialogue box for NTLM authentication, which requires
>> the user name to be entered as DOMAIN\user. Other software may
>> instead choose to use basic auth (handled by the basic_pam_auth
>> authenticator) and pops up a similar authentication box which
>> requires the bare user name (no "DOMAIN\").
>>
>> It is often not clear to the user that there is a difference between
>> these popup boxes, so they may not know whether or not to include the
>> windows domain. The attached patch modifies basic_pam_auth so that
>> the user can enter their user name as a bare name, "DOMAIN\user" or
>> "user_at_domain" and strips the domain part off so that the bare user
>> name can be authenticated against PAM.
>>
>> This should simplify things for the users, since they can just be
>> told to enter their details in the "DOMAIN\user" format everywhere
>> and it should just work. Obviously not much use in a multi-domain
>> setup, but presumably one wouldn't be authenticating against PAM in
>> such a situation anyway (?).
>
> As submitted the patch will completely break on all installations
> which allow / or @ characters in usernames. I am aware that there are
> definitely some networks allowing those - whether they use PAM is
> unknown.
>
> This will need at least a helper command line option to enable the
> stripping. I suggest the -r option as previously used in
> negotiate_kerberos_auth.
> http://www.squid-cache.org/Versions/v3/3.2/manuals/negotiate_kerberos_auth.html
>
>
>
> Amos

I've now had time to read the code...

1) the @ format for credentials comes from Kerberos.
  In your code comment please replace "NTLM" with "NTLM or Negotiate"

2) please define the user_ptr variable at the point of first use. This
is not C code, despite what the rest of the file looks like.

3) The strchr() parse will break on input mangled like: user\foo_at_somewhere

Assuming that Negotiate/Kerberos credentials are going to be the common
format seen in future what you want is:

   char *user_ptr= strchr(user, '@');
   if (user_ptr)
     *user_ptr = 0;
   else {
     user_ptr= strchr(user, '\\');
     if (user_ptr)
       user = user_ptr +1;
   }

That seems to be it.

Amos
Received on Wed Mar 06 2013 - 07:36:38 MST