[SSSD] [PATCHES] sss_seed related patches

Jakub Hrozek jhrozek at redhat.com
Wed Oct 3 16:23:24 UTC 2012


On Wed, Oct 03, 2012 at 10:07:42AM +0200, Michal Židek wrote:
> 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

[PATCH 1/4] sss_seed: Option --debug did not work in sss_seed tool.
Ack

[PATCH 2/4] sss_seed: Show error message when interactive input
Ack

[PATCH 3/4] sss_seed: Make only first line of password file valid.
Ack

[PATCH 4/4] sss_seed: Passwords longer then PASS_MAX not allowed.
Ack



More information about the sssd-devel mailing list