[SSSD] [PATCHES] Fix offline authentication for the AD provider

Sumit Bose sbose at redhat.com
Fri Sep 20 12:08:24 UTC 2013


Hi,

with the following two patches offline authentication in the AD provider
is working again and https://fedorahosted.org/sssd/ticket/2060 should be
fixed.

I started working on a unit test for find_or_guess_upn() but it is not
finished yet because of the number of internal structs needed which must
be created manually. Since the patches were working well in my tests I
decided to send the functional part first and send the tests later.

bye,
Sumit
-------------- next part --------------
From 821366b9010540956639ff7831ff5ea52bddd291 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 20 Sep 2013 12:12:03 +0200
Subject: [PATCH 1/2] 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
---
 src/db/sysdb.h                      |    1 +
 src/providers/krb5/krb5_access.c    |    5 ++-
 src/providers/krb5/krb5_auth.c      |    5 ++-
 src/providers/krb5/krb5_renew_tgt.c |    2 +-
 src/providers/krb5/krb5_utils.c     |   65 +++++++++++++++++++++++++++--------
 5 files changed, 58 insertions(+), 20 deletions(-)

diff --git a/src/db/sysdb.h b/src/db/sysdb.h
index b959466..f923217 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 8caed7c..479d015 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 fe3e6ab..b373cb4 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 9102f8c..5d5a25b 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 8d10a83..7cf510c 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)));
-- 
1.7.7.6

-------------- next part --------------
From 33ec49eada55fe89a8aeb6be4643a9608e4db2d8 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 20 Sep 2013 12:34:52 +0200
Subject: [PATCH 2/2] krb5: do not expand enterprise principals is offline

Expanding a principle to an enterprise principal only makes sense if
there is a KDC available which can process it. If we are offline the
plain principal should be used, e.g. to create an expired ccache.

Fixes https://fedorahosted.org/sssd/ticket/2060
---
 src/providers/krb5/krb5_child_handler.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/providers/krb5/krb5_child_handler.c b/src/providers/krb5/krb5_child_handler.c
index aa72c07..92dec0d 100644
--- a/src/providers/krb5/krb5_child_handler.c
+++ b/src/providers/krb5/krb5_child_handler.c
@@ -155,7 +155,7 @@ static errno_t create_send_buffer(struct krb5child_req *kr,
             break;
     }
 
-    if (kr->pd->cmd == SSS_CMD_RENEW) {
+    if (kr->pd->cmd == SSS_CMD_RENEW || kr->is_offline) {
         use_enterprise_principal = false;
     } else {
         use_enterprise_principal = dp_opt_get_bool(kr->krb5_ctx->opts,
-- 
1.7.7.6



More information about the sssd-devel mailing list