[SSSD] [PATCHES] sss_seed related patches

Stephen Gallagher sgallagh at redhat.com
Tue Oct 2 11:56:24 UTC 2012


On Mon 01 Oct 2012 05:32:50 PM EDT, Jakub Hrozek wrote:
> On Mon, Oct 01, 2012 at 07:45:17PM +0200, Pavel Březina wrote:
>> On 10/01/2012 06:25 PM, Michal Židek wrote:
>>> Patch 01: The debug_level variable was set before the parameters were
>>> processed, so the default debug level was used no matter what number was
>>> specified with --debug.
>>>
>>> Patch 02: When interactive input fails, error message should be displayed.
>>> https://fedorahosted.org/sssd/ticket/1549
>>>
>>> Patch 03: When file is used to specify a password in sss_seed, then only
>>> first line of this file is used. Also empty passwords are treated as
>>> errors. (My local test environment is broken so I have not tested this,
>>> please test it when reviewing.)
>>> https://fedorahosted.org/sssd/ticket/1548
>>>
>>> Patches are in attachment.
>>>
>>> Michal
>>
>> Patch 1: nack
>> Instead of moving debug level initialization further in the code,
>> you should parse the parameters earlier. The other tools uses
>> following pattern:
>> - set debug program name
>> - set locale
>> - parse parameters
>> - set debug level
>> See sss_groupadd for example.
>>
>> Patch 2: ack
>> Patch 3: I'll do this one tomorrow.
>
> My take on Patch 3:
>
> Instead of stripping everything after the first line, I would prefer
> simply failing if there's any non-whitespace character after the first
> line break ('\n').
>
> Stripping anything after the first line unconditionally may seem like
> everything was OK to the user..while he wouldn't be able to easily input
> a password like that.
>
> Also a code-style nitpick -- please put a newline after "}" and before
> the comment saying "/* Do not allow empty passwords */" in
> seed_password_input_prompt().


One more comment: please change the atomic read to be PASS_MAX+1 and 
then check whether len > PASS_MAX. If it is, we need to fail because 
we've been passed a password-file too long. The current code would just 
use the first PASS_MAX characters as the password, which is dangerous 
and fails to meet the users' expectation.

Also, the PASS_MAX limitation needs to be specified in the manpage for 
sss_seed(8).



More information about the sssd-devel mailing list