[Xrdp-devel] Problems with non-PAM password authentication.

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Xrdp-devel] Problems with non-PAM password authentication.

Ken Milmore
Hello,

I am using xrdp without PAM, and have been looking at the recently
modified shadow authentication code in sesman/verify_user.c.

I have noticed that sesman segfaults when attempting to authenticate a
user account which has been locked by "passwd -l username".  In fact the
segfault occurs whenever the contents of the shadow password field do
not match any of the hash formats expected by GNU crypt().

The problem lies in verify_user.c, function auth_user_pass().  The
return value of crypt() will be NULL if the account happens to be locked
or if the enctrypted password is in an unexpected format. So I would
suggest that the code at the end of auth_user_pass() should go something
like this:

   epass = crypt(pass, encr);
   if (epass == 0)
   {
     /* possibly a locked account */
     return 0;
   }
   return (strcmp(encr, epass) == 0);
}

Another point: auth_user_pass() seems to get called from a thread spun
off a listening socket, but calls getpwnam, getspnam and crypt which  re
not re-entrant. Shoudln't there be a mutex or something similar
protecting this whole function?  Apologies if I've missed some reason
why it can't be re-entered.

Best wishes,

Ken Milmore.





------------------------------------------------------------------------------
_______________________________________________
xrdp-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/xrdp-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Xrdp-devel] Problems with non-PAM password authentication.

jsorg71
Hi Kev,

Thanks, I added this to devel branch.

The other issue with the threads should go away.  I'm planning to
remove the threads from sesman because it does a bunch for forking.

Jay



On Sun, Oct 26, 2014 at 1:48 PM, Ken Milmore <[hidden email]> wrote:

> Hello,
>
> I am using xrdp without PAM, and have been looking at the recently
> modified shadow authentication code in sesman/verify_user.c.
>
> I have noticed that sesman segfaults when attempting to authenticate a
> user account which has been locked by "passwd -l username".  In fact the
> segfault occurs whenever the contents of the shadow password field do
> not match any of the hash formats expected by GNU crypt().
>
> The problem lies in verify_user.c, function auth_user_pass().  The
> return value of crypt() will be NULL if the account happens to be locked
> or if the enctrypted password is in an unexpected format. So I would
> suggest that the code at the end of auth_user_pass() should go something
> like this:
>
>    epass = crypt(pass, encr);
>    if (epass == 0)
>    {
>      /* possibly a locked account */
>      return 0;
>    }
>    return (strcmp(encr, epass) == 0);
> }
>
> Another point: auth_user_pass() seems to get called from a thread spun
> off a listening socket, but calls getpwnam, getspnam and crypt which  re
> not re-entrant. Shoudln't there be a mutex or something similar
> protecting this whole function?  Apologies if I've missed some reason
> why it can't be re-entered.
>
> Best wishes,
>
> Ken Milmore.
>
>
>
>
>
> ------------------------------------------------------------------------------
> _______________________________________________
> xrdp-devel mailing list
> [hidden email]
> https://lists.sourceforge.net/lists/listinfo/xrdp-devel

------------------------------------------------------------------------------
_______________________________________________
xrdp-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/xrdp-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Xrdp-devel] Problems with non-PAM password authentication.

Ken Milmore
Another issue:  When building with --enable-nopam, a link-time error
occurs because verify_user.c doesn't define auth_stop_session().

-Ken.


On 30/10/14 00:57, Jay Sorg wrote:

> Hi Kev,
>
> Thanks, I added this to devel branch.
>
> The other issue with the threads should go away.  I'm planning to
> remove the threads from sesman because it does a bunch for forking.
>
> Jay
>
>
>
> On Sun, Oct 26, 2014 at 1:48 PM, Ken Milmore <[hidden email]> wrote:
>> Hello,
>>
>> I am using xrdp without PAM, and have been looking at the recently
>> modified shadow authentication code in sesman/verify_user.c.
>>
>> I have noticed that sesman segfaults when attempting to authenticate a
>> user account which has been locked by "passwd -l username".  In fact the
>> segfault occurs whenever the contents of the shadow password field do
>> not match any of the hash formats expected by GNU crypt().
>>
>> The problem lies in verify_user.c, function auth_user_pass().  The
>> return value of crypt() will be NULL if the account happens to be locked
>> or if the enctrypted password is in an unexpected format. So I would
>> suggest that the code at the end of auth_user_pass() should go something
>> like this:
>>
>>    epass = crypt(pass, encr);
>>    if (epass == 0)
>>    {
>>      /* possibly a locked account */
>>      return 0;
>>    }
>>    return (strcmp(encr, epass) == 0);
>> }
>>
>> Another point: auth_user_pass() seems to get called from a thread spun
>> off a listening socket, but calls getpwnam, getspnam and crypt which  re
>> not re-entrant. Shoudln't there be a mutex or something similar
>> protecting this whole function?  Apologies if I've missed some reason
>> why it can't be re-entered.
>>
>> Best wishes,
>>
>> Ken Milmore.
>>
>>
>>
>>
>>
>> ------------------------------------------------------------------------------
>> _______________________________________________
>> xrdp-devel mailing list
>> [hidden email]
>> https://lists.sourceforge.net/lists/listinfo/xrdp-devel
>

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
xrdp-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/xrdp-devel
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Xrdp-devel] Problems with non-PAM password authentication.

jsorg71
Thanks, fixed in devel branch.
cb4f2998992d1d39fa4629c2187832249c0a5932

Jay


On Fri, Nov 21, 2014 at 1:41 AM, Ken Milmore <[hidden email]> wrote:

> Another issue:  When building with --enable-nopam, a link-time error
> occurs because verify_user.c doesn't define auth_stop_session().
>
> -Ken.
>
>
> On 30/10/14 00:57, Jay Sorg wrote:
>> Hi Kev,
>>
>> Thanks, I added this to devel branch.
>>
>> The other issue with the threads should go away.  I'm planning to
>> remove the threads from sesman because it does a bunch for forking.
>>
>> Jay
>>
>>
>>
>> On Sun, Oct 26, 2014 at 1:48 PM, Ken Milmore <[hidden email]> wrote:
>>> Hello,
>>>
>>> I am using xrdp without PAM, and have been looking at the recently
>>> modified shadow authentication code in sesman/verify_user.c.
>>>
>>> I have noticed that sesman segfaults when attempting to authenticate a
>>> user account which has been locked by "passwd -l username".  In fact the
>>> segfault occurs whenever the contents of the shadow password field do
>>> not match any of the hash formats expected by GNU crypt().
>>>
>>> The problem lies in verify_user.c, function auth_user_pass().  The
>>> return value of crypt() will be NULL if the account happens to be locked
>>> or if the enctrypted password is in an unexpected format. So I would
>>> suggest that the code at the end of auth_user_pass() should go something
>>> like this:
>>>
>>>    epass = crypt(pass, encr);
>>>    if (epass == 0)
>>>    {
>>>      /* possibly a locked account */
>>>      return 0;
>>>    }
>>>    return (strcmp(encr, epass) == 0);
>>> }
>>>
>>> Another point: auth_user_pass() seems to get called from a thread spun
>>> off a listening socket, but calls getpwnam, getspnam and crypt which  re
>>> not re-entrant. Shoudln't there be a mutex or something similar
>>> protecting this whole function?  Apologies if I've missed some reason
>>> why it can't be re-entered.
>>>
>>> Best wishes,
>>>
>>> Ken Milmore.
>>>
>>>
>>>
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> _______________________________________________
>>> xrdp-devel mailing list
>>> [hidden email]
>>> https://lists.sourceforge.net/lists/listinfo/xrdp-devel
>>

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=157005751&iu=/4140/ostg.clktrk
_______________________________________________
xrdp-devel mailing list
[hidden email]
https://lists.sourceforge.net/lists/listinfo/xrdp-devel
Loading...