[SSSD] [PATCHES] sss_seed related patches

Michal Židek mzidek at redhat.com
Tue Oct 2 13:21:46 UTC 2012


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.

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: 1602 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20121002/b6291ab5/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/20121002/b6291ab5/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/20121002/b6291ab5/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: 1966 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20121002/b6291ab5/attachment-0003.bin>


More information about the sssd-devel mailing list