Re: [SSSD] [PATCHES] SSH: Fix several client bugs

[Jan Cholasta]

On 7.5.2013 10:30, Jakub Hrozek wrote:
>  On Fri, Apr 26, 2013 at 02:49:05PM +0200, Jan Cholasta wrote:
> > Hi,
> >
> > the attached patches fix
> > <https://fedorahosted.org/sssd/ticket/1897>;. See commit messages for
> > more info. Backward compatibility with older clients is maintained.
> >
> > Honza
> >
> > --
> > Jan Cholasta
>
>  [PATCH 1/4] UTIL: Add function sss_names_init_static
>  The functionality is OK, but I would prefer a different name..maybe
>  something like sss_names_init_ex or sss_names_init_pattern ? _static
>  sounds to me like the patterns are hardcoded internally. 
I renamed the function to sss_names_init_from_args. I have also moved 
default value assignment back to sss_names_init, as I think it makes 
more sense to have it in there.

>  [PATCH 2/4] SSH: Fix parsing of names from client requests
>  Can you please set is_user=false in sss_ssh_cmd_get_host_pubkeys()
>  explicitly so that a search for is_user makes it clear what it is set to
>  in different functions? 
Sure.

> > @@ -675,6 +676,7 @@ static errno_t
> >   ssh_cmd_parse_request(struct ssh_cmd_ctx *cmd_ctx)
> >   {
> >       struct cli_ctx *cctx = cmd_ctx->cctx;
> > +    struct ssh_ctx *ssh_ctx = (struct ssh_ctx *)cctx->rctx->pvt_ctx;
>
>  Please use talloc_get_type here so that if there was a type mismatch, we
>  would get an error message in the core file. 
Done.

>  [PATCH 3/4] SSH: Use separate field for domain name in client
>  Looks good to me and Honza confirmed that the changes are
>  backwards-compatible.
>
>  [PATCH 4/4] SSH: Do not skip domains with use_fully_qualified_names
>  Ack
>  Why were the domains skipped in the first place? 
I initially handled hosts exactly the same way as users. As it turns 
out, that was not a very good idea.

Updated patches attached.

Honza

-- 
Jan Cholasta