do not add fully-qualified suffix to already fully-qualified externalUser values in sudoers for IPA provider

[abbra]

I'm working on enabling users and groups from trusted Active Directory domains to be part of sudo rules in IPA. The end result is that fully qualified AD users and groups are stored in externalUser attribute. Here is an example for non-POSIX groups with %: prefix.

$ ipa sudorule-show --all --raw testrule
  dn: ipaUniqueID=379fd310-aa8b-11ea-9c04-fa163e9c3b88,cn=sudorules,cn=sudo,dc=ipa,dc=test
  cn: testrule
  ipaenabledflag: TRUE
  hostcategory: all
  cmdcategory: all
  externaluser: %:domain admins@win2016.test
  ipaUniqueID: 379fd310-aa8b-11ea-9c04-fa163e9c3b88
  objectClass: ipaassociation
  objectClass: ipasudorule

SSSD normalizes externalUser content the same way as normal sudoUser which supposed to be non-fully qualified. This, however, breaks for trusted AD users/groups because they are already qualified.

in src/providers/ipa/ipa_sudo_conversion.c:

static const char *
convert_ext_user(TALLOC_CTX *mem_ctx,
                 struct ipa_sudo_conv *conv,
                 const char *value,
                 bool *skip_entry)
{
    return sss_create_internal_fqname(mem_ctx, value, conv->dom->name);
}

sss_create_internal_fqname then unconditionally adds conf->dom->name. Perhaps, the easiest way would be to skip calling sss_create_internal_fqname in case the value is already fully qualified and just do talloc_asprintf() of the original name.

[abbra]

For the reference here is how the rule looks in the SSSD database:

# record 44
dn: name=testrule,cn=sudorules,cn=custom,cn=ipa.test,cn=sysdb
cn: testrule
dataExpireTimestamp: 1591779038
name: testrule
objectClass: sudoRule
sudoCommand: ALL
sudoHost: ALL
sudoUser: %:domain admins@win2016.test@ipa.test
distinguishedName: name=testrule,cn=sudorules,cn=custom,cn=ipa.test,cn=sysdb

[abbra]

A tentative fix could be:

$ git diff
diff --git a/src/providers/ipa/ipa_sudo_conversion.c b/src/providers/ipa/ipa_sudo_conversion.c
index 9586e6a2a..b5fc49379 100644
--- a/src/providers/ipa/ipa_sudo_conversion.c
+++ b/src/providers/ipa/ipa_sudo_conversion.c
@@ -879,6 +879,10 @@ convert_ext_user(TALLOC_CTX *mem_ctx,
                  const char *value,
                  bool *skip_entry)
 {
+    /* If value is already fully qualified, return it as it is */
+    if (strrchr(value, '@') != NULL) {
+        return talloc_strdup(mem_ctx, value);
+    }
     return sss_create_internal_fqname(mem_ctx, value, conv->dom->name);
 }

[abbra]

Related FreeIPA part: freeipa/freeipa#4792

[sumit-bose]

@abbra, the patch looks reasonable.

@pbrezina, do you think more checks are needed here, e.g. if the domain part of the fully-qualified name is known by SSSD?

[abbra]

Any update?

[pbrezina]

Is it possible to have other domain separator then @ in externalUser? Like 'DOMAIN\name'?

[abbra]

@pbrezina no, FreeIPA will normalize the user name to always be user@domain. This is not upstream yet, in the current version you cannot even add such user or group directly to the rule.

[pbrezina]

Ok, so I think your proposed patch should be enough since after this point we just treat it as plain sudoUser attribute. Do you plan to open a PR?

[abbra]

@pbrezina I opened #5216