Re: [SPAM] Password strength bug
Drew Wells <[email protected]> Mon, 21 Sep 2015 14:55:54 +0100
| Newsgroups | gmane.mail.vpopmail |
|---|---|
| Message-ID | <[email protected]> |
This is a multi-part message in MIME format.
--------------040803020105010300070402
Content-Type: text/plain; charset=iso-8859-15; format=flowed
Content-Transfer-Encoding: 7bit
On 09/21/2015 02:26 PM, Tonix - Antonio Nati wrote:
> Il 21/09/2015 14:59, Drew Wells ha scritto:
>> On 09/17/2015 12:28 PM, Tonix - Antonio Nati wrote:
>>> Il 17/09/2015 13:18, Drew Wells ha scritto:
>>>> On 09/15/2015 03:27 PM, Tonix - Antonio Nati wrote:
>>>>> Il 15/09/2015 15:03, Drew Wells ha scritto:
>>>>>> On 09/15/2015 11:00 AM, Tonix - Antonio Nati wrote:
>>>>>>> Il 15/09/2015 11:03, Drew Wells ha scritto:
>>>>>>>> In vpopmail-5.5.0 there seems to be a bug in vpopmail.c where
>>>>>>>> the password strength is checked even if a password isn't used
>>>>>>>> (such as when -e is used to add the encrypted password). Patch
>>>>>>>> attached.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> I do not understand the problem.
>>>>>>>
>>>>>>> Of course password strenght is checked every time, and if it
>>>>>>> founds a null/empty password it gives error back if password
>>>>>>> must have a minimum lenght.
>>>>>>>
>>>>>>> Your patch instead permit to have null password even if strenght
>>>>>>> policy would not allow it.
>>>>>>>
>>>>>>> Regards,
>>>>>>>
>>>>>>> Tonino
>>>>>> The problem is is that vadduser.c can call vadduser() (in
>>>>>> vpopmail.c) without a password. It does this in the situation
>>>>>> where vadduser.c has had the options "-e" or "-n" passed to it,
>>>>>> so if this is the case the password can't be checked againts the
>>>>>> password strength rules. The underlying function vadduser() needs
>>>>>> to be able to add a user with no password.
>>>>>>
>>>>>
>>>>> I realize additional controls are done before calling vadduser();
>>>>> but I personally would prefer an explicit parameter added to
>>>>> vadduser for avoiding password check (it may be a further
>>>>> parameter having default = "check").
>>>>> It would make developers more protected against unwanted security
>>>>> bugs.
>>>>>
>>>>> Regards,
>>>>>
>>>>> Tonino
>>>>>
>>>> I agree that it would be better to explicitly indicate to
>>>> vadduser() that no password is wanted. I even looked quicky at
>>>> setting the password to NULL to indicate no password, but both this
>>>> and an explicit parameter would need changes to all the backends,
>>>> so have left it as is for now.
>>>
>>> It could be done in two ways:
>>>
>>> * considering most od c compilers are c++ compilers, and that
>>> means we can add an implicit parameter (, nocheck_pwd = 0)
>>> * duplicate the function for this usage, and call the duplicated
>>> function from avdduser when needed.
>>>
>>> Regards,
>>>
>>> Tonino
>>>
>> I have looked at the backends and it turns out that some of the
>> backends can handle a NULL gecos, so expanding on this I have changed
>> all the backends to be able to handle a NULL gecos (in which case
>> they now all use the user as a gecos) and also handle a NULL
>> password. So vadduser.c can pass a NULL password to vadduser(),
>> vadduser() can then check the password_strength() when the password
>> is not NULL.
>
> I think that permitting a null password, if policy does not admit it,
> is a security hole.
> Prefer you you add another explicit call to be called for no password
> checking (at all).
>
> Regards,
>
> Tonino
>
>
>>
>> This is going to be the patch I use here, does anyone want this patch ?
>
Wouldn't it actually be easier to remove the password parameter from
vadduser() and then vadduser.c can add a user (without a password) and
then optionally set a password using vauth_setpw() ? This is exactly
what it should do at the moment for adding a user with a crypted
password, the user is added, then the crypted password is set using
vauth_setpw().
!DSPAM:56000c6d41552022747047!
--------------040803020105010300070402
Content-Type: text/html; charset=iso-8859-15
Content-Transfer-Encoding: 8bit
<html>
<head>
<meta content="text/html; charset=iso-8859-15"
http-equiv="Content-Type">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 09/21/2015 02:26 PM, Tonix - Antonio
Nati wrote:<br>
</div>
<blockquote cite="mid:[email protected]" type="cite">
<meta content="text/html; charset=iso-8859-15"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Il 21/09/2015 14:59, Drew Wells ha
scritto:<br>
</div>
<blockquote cite="mid:[email protected]" type="cite">
<meta content="text/html; charset=iso-8859-15"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 09/17/2015 12:28 PM, Tonix -
Antonio Nati wrote:<br>
</div>
<blockquote cite="mid:[email protected]"
type="cite">
<meta content="text/html; charset=iso-8859-15"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Il 17/09/2015 13:18, Drew Wells
ha scritto:<br>
</div>
<blockquote cite="mid:[email protected]"
type="cite">
<meta content="text/html; charset=iso-8859-15"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 09/15/2015 03:27 PM, Tonix -
Antonio Nati wrote:<br>
</div>
<blockquote cite="mid:[email protected]"
type="cite">
<meta content="text/html; charset=iso-8859-15"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Il 15/09/2015 15:03, Drew
Wells ha scritto:<br>
</div>
<blockquote cite="mid:[email protected]"
type="cite">
<meta content="text/html; charset=iso-8859-15"
http-equiv="Content-Type">
<div class="moz-cite-prefix">On 09/15/2015 11:00 AM,
Tonix - Antonio Nati wrote:<br>
</div>
<blockquote cite="mid:[email protected]"
type="cite">
<meta content="text/html; charset=iso-8859-15"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Il 15/09/2015 11:03, Drew
Wells ha scritto:<br>
</div>
<blockquote cite="mid:[email protected]"
type="cite">In vpopmail-5.5.0 there seems to be a
bug in vpopmail.c where the password strength is
checked even if a password isn't used (such as when
-e is used to add the encrypted password). Patch
attached. <br>
<br>
<br>
<br>
</blockquote>
<br>
<br>
<font size="-1"><font face="Verdana">I do not
understand the problem.<br>
<br>
Of course password strenght is checked every time,
and if it founds a null/empty password it gives
error back if password must have a minimum lenght.<br>
<br>
Your patch instead permit to have null password
even if strenght policy would not allow it.<br>
<br>
Regards,<br>
<br>
Tonino</font></font><br>
</blockquote>
The problem is is that vadduser.c can call vadduser()
(in vpopmail.c) without a password. It does this in the
situation where vadduser.c has had the options "-e" or
"-n" passed to it, so if this is the case the password
can't be checked againts the password strength rules.
The underlying function vadduser() needs to be able to
add a user with no password.<br>
<br>
</blockquote>
<br>
I realize additional controls are done before calling
vadduser(); but I personally would prefer an explicit
parameter added to vadduser for avoiding password check
(it may be a further parameter having default = "check").<br>
It would make developers more protected against unwanted
security bugs.<br>
<br>
Regards,<br>
<br>
Tonino<br>
<br>
</blockquote>
I agree that it would be better to explicitly indicate to
vadduser() that no password is wanted. I even looked quicky
at setting the password to NULL to indicate no password, but
both this and an explicit parameter would need changes to
all the backends, so have left it as is for now.<br>
</blockquote>
<br>
It could be done in two ways:<br>
<ul>
<li>considering most od c compilers are c++ compilers, and
that means we can add an implicit parameter (, nocheck_pwd
= 0)</li>
<li>duplicate the function for this usage, and call the
duplicated function from avdduser when needed.</li>
</ul>
<p>Regards,<br>
<br>
Tonino<br>
</p>
</blockquote>
I have looked at the backends and it turns out that some of the
backends can handle a NULL gecos, so expanding on this I have
changed all the backends to be able to handle a NULL gecos (in
which case they now all use the user as a gecos) and also handle
a NULL password. So vadduser.c can pass a NULL password to
vadduser(), vadduser() can then check the password_strength()
when the password is not NULL.<br>
</blockquote>
<br>
I think that permitting a null password, if policy does not admit
it, is a security hole.<br>
Prefer you you add another explicit call to be called for no
password checking (at all).<br>
<br>
Regards,<br>
<br>
Tonino<br>
<br>
<br>
<blockquote cite="mid:[email protected]" type="cite">
<br>
This is going to be the patch I use here, does anyone want this
patch ?<br>
</blockquote>
<br>
</blockquote>
Wouldn't it actually be easier to remove the password parameter from
vadduser() and then vadduser.c can add a user (without a password)
and then optionally set a password using vauth_setpw() ? This is
exactly what it should do at the moment for adding a user with a
crypted password, the user is added, then the crypted password is
set using vauth_setpw().<br>
!DSPAM:56000c6d41552022747047!
</body>
</html>
--------------040803020105010300070402--