[SSSD] [PATCHES] krb5: new option krb5_map_user

Pavel Reichl preichl at redhat.com
Thu May 14 16:37:37 UTC 2015



On 05/14/2015 05:10 PM, Jakub Hrozek wrote:
> On Thu, May 14, 2015 at 01:55:01PM +0200, Pavel Reichl wrote:
>> Please see updated patch.
> Thanks, this looks much better. I only have some nitpicks now:
>
>>  From 4f0bd09258e52ae633bc91603e30edf9ce02b279 Mon Sep 17 00:00:00 2001
>> From: Pavel Reichl <preichl at redhat.com>
>> Date: Thu, 30 Apr 2015 06:43:05 -0400
>> Subject: [PATCH] krb5: new option krb5_map_user
>>
>> New option `krb5_map_user` providing mapping of ID provider names to
>> Kerberos principals.
>>
>> Resolves:
>> https://fedorahosted.org/sssd/ticket/2509
> [...]
>
>> +                        </para>
>> +                        <para>
>> +                            `joe` and `vince` are UNIX user names and `juser`
>> +                            and `rraines` are primaries of kerberos principals.
>> +                            For user `joe` resp. `dick` SSSD will try to kinit
>> +                            as `juser at REALM` resp. `richard at REALM`
> Could we use something else than backticks in the XML. Remember we used
> XML because it's possible to render other formats than troff, we already
> render HTML. Would <quote> be a better choice?
done
>
>> +                        </para>
>> +
>> +                        <para>
>> +                            Default: not set
>> +                        </para>
>> +                    </listitem>
>> +                </varlistentry>
>> +
>>               </variablelist>
>>           </para>
>>       </refsect1>
> [...]
>
>> +static errno_t split_tuple(TALLOC_CTX *mem_ctx, const char *tuple,
>> +                           const char **_first, const char **_second)
>> +{
>> +    errno_t ret;
>> +    char **list;
>> +    int n;
>> +    talloc(mem_ctx, int);
> huh?
Sorry, but I need more changed lines to stats :-).
fixed
>
>> +
>> +    ret = split_on_separator(mem_ctx, tuple, ':', true, true, &list, &n);
>> +
>> +    if (ret != EOK) {
>> +        DEBUG(SSSDBG_MINOR_FAILURE,
>> +              "split_on_separator failed - %s:[%d]\n",
>> +              sss_strerror(ret), ret);
>> +        goto done;
>> +    } else if (n != 2) {
>> +        DEBUG(SSSDBG_MINOR_FAILURE, "split_on_separator failed.\n");
>> +        ret = EINVAL;
>> +        goto done;
>> +    }
>> +
>> +    *_first = list[0];
>> +    *_second = list[1];
>> +
>> +done:
>> +    return ret;
>> +}
>> +
>> +static errno_t
>> +fill_name_to_primary_map(TALLOC_CTX *mem_ctx, char **map,
>> +                       struct map_id_name_to_krb_primary *name_to_primary,
>> +                       int size)
> Can we use size_t for size?
sure, but it takes input from  split_on_separator() which is int, should 
I fix split_on_separator() to use size_t instead if int?
>
>> +{
>> +    int i;
>> +    errno_t ret;
>> +
>> +    for (i = 0; i < size; i++) {
>> +        ret = split_tuple(mem_ctx, map[i],
>> +                          &name_to_primary[i].id_name,
>> +                          &name_to_primary[i].krb_primary);
>> +        if (ret != EOK) {
>> +            DEBUG(SSSDBG_MINOR_FAILURE,
>> +                  "split_tuple failed - %s:[%d]\n", sss_strerror(ret), ret);
>> +            goto done;
>> +        }
>> +    }
>> +
>> +    ret = EOK;
>> +
>> +done:
>> +    return ret;
>> +}
>> +
>> +errno_t
>> +parse_krb5_map_user(TALLOC_CTX *mem_ctx, const char *krb5_map_user,
>> +                    struct map_id_name_to_krb_primary **_name_to_primary)
>> +{
>> +    int size;
>> +    char **map;
>> +    errno_t ret;
>> +    TALLOC_CTX *tmp_ctx;
>> +    struct map_id_name_to_krb_primary *name_to_primary;
>> +
>> +    tmp_ctx = talloc_new(NULL);
>> +    if (tmp_ctx == NULL) {
>> +        ret = ENOMEM;
>> +        goto done;
>> +    }
>> +
>> +    if (krb5_map_user == NULL || strlen(krb5_map_user) == 0) {
>> +        DEBUG(SSSDBG_FUNC_DATA, "Warning: krb5_map_user is empty!\n");
>> +        size = 0;
>> +        ret = EOK;
>> +        goto done;
>> +    }
> I won't block the patch over this but I still don't understand why you
> prefer to use the NULL pointer for "an empty mapping" instead of always
> allocating the sentinel. I'd argue that using the sentinel would
> ultimately make the code easier because you wouldn't have to check for
> NULL mapping pointer, but just loop over the mapping array..one less
> special case..
done
>
>> +
>> +    ret = split_on_separator(tmp_ctx, krb5_map_user, ',', true, true,
>> +                             &map, &size);
>> +    if (ret != EOK) {
>> +        DEBUG(SSSDBG_OP_FAILURE, "Failed to parse krb5_map_user!\n");
>> +        goto done;
>> +    }
>> +
>> +    name_to_primary = talloc_zero_array(tmp_ctx,
>> +                                        struct map_id_name_to_krb_primary,
>> +                                        size + 1);
>> +    if (name_to_primary == NULL) {
>> +        ret = ENOMEM;
>> +        goto done;
>> +    }
>> +    /* sentinel */
>> +    name_to_primary[size].id_name = NULL;
>> +    name_to_primary[size].krb_primary = NULL;
>> +
>> +    ret = fill_name_to_primary_map(name_to_primary, map, name_to_primary,
>> +                                   size);
>> +    if (ret != EOK) {
>> +        DEBUG(SSSDBG_MINOR_FAILURE,
>> +              "fill_name_to_primary_map failed: %s:[%d]\n",
>> +              sss_strerror(ret), ret);
>> +        goto done;
>> +    }
>> +
>> +    ret = EOK;
>> +
>> +done:
>> +    if (ret == EOK) {
>> +        if (size == 0) {
>> +            *_name_to_primary = NULL;
>> +        } else {
>> +            *_name_to_primary = talloc_steal(mem_ctx, name_to_primary);
>> +        }
>> +    }
>> +    talloc_free(tmp_ctx);
>> +    return ret;
>> +}
> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-krb5-new-option-krb5_map_user.patch
Type: text/x-patch
Size: 24200 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20150514/e5d67e32/attachment.bin>


More information about the sssd-devel mailing list