krb5: save canonical upn to sysdb

If the returned TGT contains a different user principal name (upn) than
used in the request, i.e. the upn was canonicalized, we currently save
it to sysdb into the same attribute where the upn coming from an LDAP
server is stored as well. This means the canonical upn might be
overwritten when the user data is re-read from the LDAP server.

To avoid this this patch add a new attribute to sysdb where the
canonical upn is stored and makes sure it is used when available.

Fixes https://fedorahosted.org/sssd/ticket/2060

4 files changed, 57 insertions, 20 deletions

diff --git a/src/providers/krb5/krb5_access.c b/src/providers/krb5/krb5_access.c
index 8caed7c6..479d0151 100644
--- a/src/providers/krb5/krb5_access.c
+++ b/src/providers/krb5/krb5_access.c
@@ -76,7 +76,7 @@ struct tevent_req *krb5_access_send(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    attrs = talloc_array(state, const char *, 4);
+    attrs = talloc_array(state, const char *, 5);
     if (attrs == NULL) {
         DEBUG(1, ("talloc_array failed.\n"));
         ret = ENOMEM;
@@ -86,7 +86,8 @@ struct tevent_req *krb5_access_send(TALLOC_CTX *mem_ctx,
     attrs[0] = SYSDB_UPN;
     attrs[1] = SYSDB_UIDNUM;
     attrs[2] = SYSDB_GIDNUM;
-    attrs[3] = NULL;
+    attrs[3] = SYSDB_CANONICAL_UPN;
+    attrs[4] = NULL;
 
     ret = sysdb_get_user_attr(state, be_ctx->domain->sysdb, be_ctx->domain,
                               state->pd->user, attrs, &res);
diff --git a/src/providers/krb5/krb5_auth.c b/src/providers/krb5/krb5_auth.c
index fe3e6aba..b373cb4c 100644
--- a/src/providers/krb5/krb5_auth.c
+++ b/src/providers/krb5/krb5_auth.c
@@ -513,7 +513,7 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
         goto done;
     }
 
-    attrs = talloc_array(state, const char *, 6);
+    attrs = talloc_array(state, const char *, 7);
     if (attrs == NULL) {
         ret = ENOMEM;
         goto done;
@@ -524,7 +524,8 @@ struct tevent_req *krb5_auth_send(TALLOC_CTX *mem_ctx,
     attrs[2] = SYSDB_CCACHE_FILE;
     attrs[3] = SYSDB_UIDNUM;
     attrs[4] = SYSDB_GIDNUM;
-    attrs[5] = NULL;
+    attrs[5] = SYSDB_CANONICAL_UPN;
+    attrs[6] = NULL;
 
     ret = krb5_setup(state, pd, krb5_ctx, &state->kr);
     if (ret != EOK) {
diff --git a/src/providers/krb5/krb5_renew_tgt.c b/src/providers/krb5/krb5_renew_tgt.c
index 9102f8ca..5d5a25b8 100644
--- a/src/providers/krb5/krb5_renew_tgt.c
+++ b/src/providers/krb5/krb5_renew_tgt.c
@@ -375,7 +375,7 @@ static errno_t check_ccache_files(struct renew_tgt_ctx *renew_tgt_ctx)
     const char *ccache_filter = "(&("SYSDB_CCACHE_FILE"=*)" \
                                   "("SYSDB_OBJECTCLASS"="SYSDB_USER_CLASS"))";
     const char *ccache_attrs[] = { SYSDB_CCACHE_FILE, SYSDB_UPN, SYSDB_NAME,
-                                   NULL };
+                                   SYSDB_CANONICAL_UPN, NULL };
     size_t msgs_count = 0;
     struct ldb_message **msgs = NULL;
     size_t c;
diff --git a/src/providers/krb5/krb5_utils.c b/src/providers/krb5/krb5_utils.c
index 8d10a834..7cf510ca 100644
--- a/src/providers/krb5/krb5_utils.c
+++ b/src/providers/krb5/krb5_utils.c
@@ -35,18 +35,36 @@ errno_t find_or_guess_upn(TALLOC_CTX *mem_ctx, struct ldb_message *msg,
                           struct sss_domain_info *dom, const char *user,
                           const char *user_dom, char **_upn)
 {
-    const char *upn;
+    const char *upn = NULL;
     int ret;
 
-    upn = ldb_msg_find_attr_as_string(msg, SYSDB_UPN, NULL);
-    if (upn == NULL) {
-        ret = krb5_get_simple_upn(mem_ctx, krb5_ctx, dom, user,
-                                  user_dom, _upn);
-        if (ret != EOK) {
-            DEBUG(SSSDBG_OP_FAILURE, ("krb5_get_simple_upn failed.\n"));
-            return ret;
+    if (krb5_ctx == NULL || dom == NULL || user == NULL || _upn == NULL) {
+        return EINVAL;
+    }
+
+    if (msg != NULL) {
+        upn = ldb_msg_find_attr_as_string(msg, SYSDB_CANONICAL_UPN, NULL);
+        if (upn != NULL) {
+            ret = EOK;
+            goto done;
         }
-    } else {
+
+        upn = ldb_msg_find_attr_as_string(msg, SYSDB_UPN, NULL);
+        if (upn != NULL) {
+            ret = EOK;
+            goto done;
+        }
+    }
+
+    ret = krb5_get_simple_upn(mem_ctx, krb5_ctx, dom, user,
+                              user_dom, _upn);
+    if (ret != EOK) {
+        DEBUG(SSSDBG_OP_FAILURE, ("krb5_get_simple_upn failed.\n"));
+        return ret;
+    }
+
+done:
+    if (ret == EOK && upn != NULL) {
         *_upn = talloc_strdup(mem_ctx, upn);
         if (*_upn == NULL) {
             DEBUG(SSSDBG_OP_FAILURE, ("talloc_strdup failed.\n"));
@@ -54,7 +72,7 @@ errno_t find_or_guess_upn(TALLOC_CTX *mem_ctx, struct ldb_message *msg,
         }
     }
 
-    return EOK;
+    return ret;
 }
 
 errno_t check_if_cached_upn_needs_update(struct sysdb_ctx *sysdb,
@@ -65,11 +83,12 @@ errno_t check_if_cached_upn_needs_update(struct sysdb_ctx *sysdb,
     TALLOC_CTX *tmp_ctx;
     int ret;
     int sret;
-    const char *attrs[] = {SYSDB_UPN, NULL};
+    const char *attrs[] = {SYSDB_UPN, SYSDB_CANONICAL_UPN, NULL};
     struct sysdb_attrs *new_attrs;
     struct ldb_result *res;
     bool in_transaction = false;
     const char *cached_upn;
+    const char *cached_canonical_upn;
 
     if (sysdb == NULL || user == NULL || upn == NULL) {
         return EINVAL;
@@ -103,8 +122,23 @@ errno_t check_if_cached_upn_needs_update(struct sysdb_ctx *sysdb,
         goto done;
     }
 
-    DEBUG(SSSDBG_TRACE_LIBS, ("Replacing UPN [%s] with [%s] for user [%s].\n",
-                              cached_upn, upn, user));
+    cached_canonical_upn = ldb_msg_find_attr_as_string(res->msgs[0],
+                                                       SYSDB_CANONICAL_UPN,
+                                                       NULL);
+
+    if (cached_canonical_upn != NULL
+            && strcmp(cached_canonical_upn, upn) == 0) {
+        DEBUG(SSSDBG_TRACE_ALL, ("Cached canonical UPN and new one match, "
+                                 "nothing to do.\n"));
+        ret = EOK;
+        goto done;
+    }
+
+    DEBUG(SSSDBG_TRACE_LIBS, ("Replacing canonical UPN [%s] with [%s] " \
+                              "for user [%s].\n",
+                              cached_canonical_upn == NULL ?
+                                                 "empty" : cached_canonical_upn,
+                              upn, user));
 
     new_attrs = sysdb_new_attrs(tmp_ctx);
     if (new_attrs == NULL) {
@@ -113,7 +147,7 @@ errno_t check_if_cached_upn_needs_update(struct sysdb_ctx *sysdb,
         goto done;
     }
 
-    ret = sysdb_attrs_add_string(new_attrs, SYSDB_UPN, upn);
+    ret = sysdb_attrs_add_string(new_attrs, SYSDB_CANONICAL_UPN, upn);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("sysdb_attrs_add_string failed.\n"));
         goto done;
@@ -128,7 +162,8 @@ errno_t check_if_cached_upn_needs_update(struct sysdb_ctx *sysdb,
     in_transaction = true;
 
     ret = sysdb_set_entry_attr(sysdb, res->msgs[0]->dn, new_attrs,
-                               SYSDB_MOD_REP);
+                               cached_canonical_upn == NULL ? SYSDB_MOD_ADD :
+                                                              SYSDB_MOD_REP);
     if (ret != EOK) {
         DEBUG(SSSDBG_OP_FAILURE, ("sysdb_set_entry_attr failed [%d][%s].\n",
                                   ret, strerror(ret)));