[SSSD] [PATCHES] sss_seed related patches

Michal Židek mzidek at redhat.com
Wed Oct 3 08:07:42 UTC 2012


On 10/02/2012 04:23 PM, Stephen Gallagher wrote:
> On Tue 02 Oct 2012 09:21:46 AM EDT, Michal Židek wrote:
>> On 10/02/2012 01:56 PM, Stephen Gallagher wrote:
>>> 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).
>>
>> Added 4th patch to fix the PASS_MAX issue.
>>
>> New patches attached.
>>
>
> Nack,
>
> You're leaking the file descriptor. You should close it before the check.
>
> In the manpage, you have several typos:
> s/lenght/length/
> s/less then/less than/
>
> And for wording:
> s/(64 on systems where no PASS_MAX is specified)/(64 characters on
> systems with no globally-defined PASS_MAX value)/
>

New patches attached.

Thanks
Michal
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-sss_seed-Option-debug-did-not-work-in-sss_seed-tool.patch
Type: text/x-patch
Size: 1532 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20121003/21afea8d/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-sss_seed-Show-error-message-when-interactive-input-f.patch
Type: text/x-patch
Size: 714 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20121003/21afea8d/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-sss_seed-Make-only-first-line-of-password-file-valid.patch
Type: text/x-patch
Size: 2328 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20121003/21afea8d/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-sss_seed-Passwords-longer-then-PASS_MAX-not-allowed.patch
Type: text/x-patch
Size: 2010 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20121003/21afea8d/attachment-0003.bin>


More information about the sssd-devel mailing list