[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