[PATCH] addgroup , adduser series.

haroon maqsood maqsood3525 at live.com
Mon Aug 6 23:07:11 UTC 2018


Hi,

you are right my mistake,

i thought of implementing --disabled-login and password .

regarding the changes hmm, i agree the fixe(s)( i just tested adduser with no arguments no error was returned. so another fix)

should be done indiviually.

just taking advice here, if there is a rewrite should that be broken up into parts , that would be kind of a bit difficult (IMO) and adduser is not such a big piece of code.

sure i will post the patches by breaking them down :), thanks for your help.

Haroon


________________________________
From: busybox <busybox-bounces at busybox.net> on behalf of Tito <farmatito at tiscali.it>
Sent: Monday, August 6, 2018 10:07 PM
To: busybox
Subject: Re: [PATCH] addgroup , adduser series.

Hi,
please don't add your replay to the top but add it to the end
of the mail so that the mail could be read from top to bottom.
So let us continue the discussions there....

On 06/08/2018 21:54, haroon maqsood wrote:
> Hi,
>
> i think it might be misunderstanding on my part  about the options.
>
> i was trying to fix this bug https://bugs.busybox.net/show_bug.cgi?id=10981
>
> and restructure the code a bit, but i might be looking at the wrong
> version of adduser.c (apologies in advance ).
>
> the patch i have so far fixes the bug as i understand it is there (i
> might be wrong in my understanding)and it removes 110 bytes, if i am
> understanding the output of make bloatcheck, again you know busybox
> better then me so you might be able to guide me in this.
>
> regarding the config i know and totally agree with you on this,
>
> it was more from a readability/understanding point of view
>
> Haroon
>
>
>
>
> ------------------------------------------------------------------------
> *From:* busybox <busybox-bounces at busybox.net> on behalf of Tito
> <farmatito at tiscali.it>
> *Sent:* Sunday, August 5, 2018 6:45 PM
> *To:* busybox at busybox.net
> *Subject:* Re: [PATCH] addgroup , adduser series.
> On 05/08/2018 10:51, haroon maqsood wrote:
>> Hi
>>
>> I started working on a patch for adduser utility i.e removing the fixme
>> and restructuring the code and implementing options S and D as they are
>> currently disabled.
>
> Hi,
> S Create a system user/group should be working in both adduser and
> addgroup.
> D as Disabled or empty password should be working in adduser and
> is not needed in addgroup.
> Can you explain a little more what you plan to do?
>
>> i though it would make sense to send the first patch that is for
>> addgroup, as in my opinion adduser depends on addgroup
>
> Why? Busybox's adduser can use an external addgroup binary
> if need be.
>>
>> and the configs are needed in both places , so they should be in addgroup.
>
> Configs are generated by busybox's config system (e.g. make menuconfig)
> and are shared by adduser and addgroup. Why do you need to move them
> around?
>
> [*]   Enable sanity check on user/group names in adduser and adgroup
> (60000) Last valid uid or gid for adduser and addgroup
> (100) First valid system uid or gid for adduser and addgroup
>             (999) Last valid system uid or gid for adduser and addgroup
>
>
> Ciao,
> Tito
>>
>> let me know what you think.
>>
>> Haroon
>>

The bug  you want to fix is:
https://bugs.busybox.net/show_bug.cgi?id=10981

"The use of flag '-D' while creating an account seems to disable the
created account (analogue of 'passwd -l') instead of simply leaving the
account passwordless (analogue of 'passwd -u'). Which I find contrasting
against the documentation;"

In fact the -D flag is used for 2 options (for compatibility or
some other reason as this code was already there in busybox 1.10 in
2008) probably because both don't ask interactively the user to input a
password:

"disabled-password\0"   No_argument       "D"
"empty-password\0"      No_argument       "D"

so we need to decide if we want to fix the code or the documentation
or do nothing.
BTW in debian for example there is no -D at all but only
the long options [--disabled-password] [--disabled-login].

Busybox help text says:

usage:     "    -D              Don't assign a password"

but does not specify if empty or disabled,
however for the sake of security a disabled password
is far better than a empty password so disabled
looks as the better default value to me.

Best would be if you post the patch with the least
changes to the code that in your opinion fixes the
bug, without changing unrelated or cosmetic stuff,
then the list members  can discuss, criticize, and improve it
and the maintainer apply or dismiss it.

Hope this helps,
Ciao,
Tito







_______________________________________________
busybox mailing list
busybox at busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/busybox/attachments/20180806/a9149663/attachment-0001.html>


More information about the busybox mailing list