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

5 files changed, 58 insertions, 20 deletions

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index b95946647..f92321762 100644
--- a/src/db/sysdb.h
+++ b/src/db/sysdb.h
@@ -108,6 +108,7 @@
 #define SYSDB_PRIMARY_GROUP_GIDNUM "origPrimaryGroupGidNumber"
 #define SYSDB_SID_STR "objectSIDString"
 #define SYSDB_UPN "userPrincipalName"
+#define SYSDB_CANONICAL_UPN "canonicalUserPrincipalName"
 #define SYSDB_CCACHE_FILE "ccacheFile"
 
 #define SYSDB_ORIG_DN "originalDN"
diff --git a/src/providers/krb5/krb5_access.c b/src/providers/krb5/krb5_access.c
index 8caed7c69..479d01517 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 fe3e6aba7..b373cb4c3 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 9102f8ca3..5d5a25b87 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 8d10a8346..7cf510ca9 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)));