[SSSD] [PATCHES] Fix password change for trusted AD users

Sumit Bose sbose at redhat.com
Wed Nov 14 16:27:20 UTC 2012


Hi,

the following three patches make password changes for trusted AD users
work and fix https://fedorahosted.org/sssd/ticket/1615 . The most
important fix, disabling canonicalization for the AS-REQ for the change
password ticket, is in the third patch. The other two fixes issues I
came across which might block password changes under specific
conditions.

bye,
Sumit
-------------- next part --------------
From d8564ea3b360dc1909cc7a33e15cf4832e5c49f8 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 14 Nov 2012 13:56:43 +0100
Subject: [PATCH 1/3] Just use the service name with
 krb5_get_init_creds_password()

Currently we add the realm name to change password principal but
according to the MIT Kerberos docs and the upstream usage the realm name
is just ignored.

Dropping the realm name also does not lead to confusion if the change
password request was received for a user of a trusted domain.
---
 src/providers/krb5/krb5_child.c |   32 +++++++-------------------------
 1 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 8a68f27..92ebebf 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -38,7 +38,11 @@
 #include "providers/krb5/krb5_utils.h"
 #include "sss_cli.h"
 
-#define SSSD_KRB5_CHANGEPW_PRINCIPAL "kadmin/changepw"
+/* SSSD_KRB5_CHANGEPW_PRINCIPAL is used as the 8th argument (in_tkt_service)
+ * of krb5_get_init_creds_password() which is declared as 'char *' in MIT
+ * Kerberos 1.10 or lower. It looks that it will be changed to 'const char *'
+ * in Version 1.11. */
+#define SSSD_KRB5_CHANGEPW_PRINCIPAL discard_const("kadmin/changepw")
 
 struct krb5_child_ctx {
     /* opts taken from kinit */
@@ -1053,7 +1057,6 @@ static errno_t changepw_child(int fd, struct krb5_req *kr)
     char *user_error_message = NULL;
     size_t user_resp_len;
     uint8_t *user_resp;
-    char *changepw_princ = NULL;
     krb5_prompter_fct prompter = sss_krb5_prompter;
     const char *realm_name;
     int realm_length;
@@ -1074,16 +1077,6 @@ static errno_t changepw_child(int fd, struct krb5_req *kr)
         goto sendresponse;
     }
 
-    changepw_princ = talloc_asprintf(kr, "%s@%s", SSSD_KRB5_CHANGEPW_PRINCIPAL,
-                                                  kr->krb5_ctx->realm);
-    if (changepw_princ == NULL) {
-        DEBUG(1, ("talloc_asprintf failed.\n"));
-        kerr = KRB5KRB_ERR_GENERIC;
-        goto sendresponse;
-    }
-    DEBUG(SSSDBG_FUNC_DATA,
-          ("Created a changepw principal [%s]\n", changepw_princ));
-
     if (kr->pd->cmd == SSS_PAM_CHAUTHTOK_PRELIM) {
         /* We do not need a password expiration warning here. */
         prompter = NULL;
@@ -1095,7 +1088,7 @@ static errno_t changepw_child(int fd, struct krb5_req *kr)
           ("Attempting kinit for realm [%s]\n",realm_name));
     kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ,
                                         pass_str, prompter, kr, 0,
-                                        changepw_princ,
+                                        SSSD_KRB5_CHANGEPW_PRINCIPAL,
                                         kr->options);
     if (kerr != 0) {
         pam_status = kerr_handle_error(kerr);
@@ -1202,7 +1195,6 @@ static errno_t tgt_req_child(int fd, struct krb5_req *kr)
     int ret;
     krb5_error_code kerr = 0;
     char *pass_str = NULL;
-    char *changepw_princ = NULL;
     int pam_status = PAM_SYSTEM_ERR;
 
     DEBUG(SSSDBG_TRACE_LIBS, ("Attempting to get a TGT\n"));
@@ -1222,16 +1214,6 @@ static errno_t tgt_req_child(int fd, struct krb5_req *kr)
         goto sendresponse;
     }
 
-    changepw_princ = talloc_asprintf(kr, "%s@%s", SSSD_KRB5_CHANGEPW_PRINCIPAL,
-                                                  kr->krb5_ctx->realm);
-    if (changepw_princ == NULL) {
-        DEBUG(1, ("talloc_asprintf failed.\n"));
-        kerr = KRB5KRB_ERR_GENERIC;
-        goto sendresponse;
-    }
-    DEBUG(SSSDBG_FUNC_DATA,
-          ("Created a changepw principal [%s]\n", changepw_princ));
-
     kerr = get_and_save_tgt(kr, pass_str);
 
     /* If the password is expired the KDC will always return
@@ -1249,7 +1231,7 @@ static errno_t tgt_req_child(int fd, struct krb5_req *kr)
         }
         kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ,
                                             pass_str, sss_krb5_prompter, kr, 0,
-                                            changepw_princ,
+                                            SSSD_KRB5_CHANGEPW_PRINCIPAL,
                                             kr->options);
         krb5_free_cred_contents(kr->ctx, kr->creds);
         if (kerr == 0) {
-- 
1.7.7.6

-------------- next part --------------
From d2ee769d804f74509301a1f6b1183de36d095feb Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 14 Nov 2012 16:29:14 +0100
Subject: [PATCH 2/3] Fix compare_principal_realm() check

In case of a short UPN compare_principal_realm() erroneously returns an
error.
---
 src/providers/krb5/krb5_common.c |   12 +++---------
 src/tests/krb5_utils-tests.c     |    6 ++++++
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/providers/krb5/krb5_common.c b/src/providers/krb5/krb5_common.c
index ee3d725..ed2fffa 100644
--- a/src/providers/krb5/krb5_common.c
+++ b/src/providers/krb5/krb5_common.c
@@ -898,22 +898,16 @@ errno_t krb5_get_simple_upn(TALLOC_CTX *mem_ctx, struct krb5_ctx *krb5_ctx,
 errno_t compare_principal_realm(const char *upn, const char *realm,
                                 bool *different_realm)
 {
-    size_t upn_len;
-    size_t realm_len;
     char *at_sign;
 
-    if (upn == NULL || realm == NULL || different_realm == NULL) {
+    if (upn == NULL || realm == NULL || different_realm == NULL ||
+        *upn == '\0' || *realm == '\0') {
         return EINVAL;
     }
 
-    upn_len = strlen(upn);
-    realm_len = strlen(realm);
     at_sign = strchr(upn, '@');
 
-    /* if coming from the same realm the upn must be at least the size of the
-     * realm plus 1 for the '@' char. */
-    if (upn_len == 0 || realm_len == 0 || upn_len <= realm_len + 1 ||
-        at_sign == NULL) {
+    if (at_sign == NULL) {
         return EINVAL;
     }
 
diff --git a/src/tests/krb5_utils-tests.c b/src/tests/krb5_utils-tests.c
index 77dc27c..bc1890f 100644
--- a/src/tests/krb5_utils-tests.c
+++ b/src/tests/krb5_utils-tests.c
@@ -711,6 +711,12 @@ START_TEST(test_compare_principal_realm)
     fail_unless(ret == EOK, "Failure with different realm");
     fail_unless(different_realm == true, "Different realm but " \
                                           "different_realm is not true.");
+
+    ret = compare_principal_realm("user at ABC", "REALMNAMELONGERTHANUPN",
+                                 &different_realm);
+    fail_unless(ret == EOK, "Failure with long realm name.");
+    fail_unless(different_realm == true, "Realm name longer than UPN but "
+                                         "different_realm is not true.");
 }
 END_TEST
 
-- 
1.7.7.6

-------------- next part --------------
From af5f8815aa57c0e5bdab2e919808103fa4fffdc3 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Wed, 14 Nov 2012 14:56:47 +0100
Subject: [PATCH 3/3] Disable canonicalization during password changes

If canonicalization is enabled Active Directory KDCs return
'krbtgt/AD.DOMAIN' as service name instead of the expected
'kadmin/changepw' which causes a 'KDC reply did not match expectations'
error.

Additionally the forwardable and proxiable flags are disabled, the
renewable lifetime is set to 0 and the lifetime of the ticket is set to
5 minutes as recommended in https://fedorahosted.org/sssd/ticket/1405
and also done by the kpasswd utility.

Fixes: https://fedorahosted.org/sssd/ticket/1405
       https://fedorahosted.org/sssd/ticket/1615
---
 src/providers/krb5/krb5_child.c |   45 +++++++++++++++++++++++++++++++++++++-
 1 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 92ebebf..8f8c5b5 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -105,6 +105,28 @@ struct krb5_req {
 static krb5_context krb5_error_ctx;
 #define KRB5_CHILD_DEBUG(level, error) KRB5_DEBUG(level, krb5_error_ctx, error)
 
+static krb5_error_code get_changepw_options(krb5_context ctx,
+                                            krb5_get_init_creds_opt **_options)
+{
+    krb5_get_init_creds_opt *options;
+    krb5_error_code kerr;
+
+    kerr = sss_krb5_get_init_creds_opt_alloc(ctx, &options);
+    if (kerr != 0) {
+        KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);
+        return kerr;
+    }
+
+    sss_krb5_get_init_creds_opt_set_canonicalize(options, 0);
+    krb5_get_init_creds_opt_set_forwardable(options, 0);
+    krb5_get_init_creds_opt_set_proxiable(options, 0);
+    krb5_get_init_creds_opt_set_renew_life(options, 0);
+    krb5_get_init_creds_opt_set_tkt_life(options, 5*60);
+
+    *_options = options;
+
+    return 0;
+}
 
 static errno_t sss_send_pac(krb5_authdata **pac_authdata)
 {
@@ -1060,6 +1082,7 @@ static errno_t changepw_child(int fd, struct krb5_req *kr)
     krb5_prompter_fct prompter = sss_krb5_prompter;
     const char *realm_name;
     int realm_length;
+    krb5_get_init_creds_opt *chagepw_options;
 
     DEBUG(SSSDBG_TRACE_LIBS, ("Password change operation\n"));
 
@@ -1082,6 +1105,12 @@ static errno_t changepw_child(int fd, struct krb5_req *kr)
         prompter = NULL;
     }
 
+    kerr = get_changepw_options(kr->ctx, &chagepw_options);
+    if (kerr != 0) {
+        DEBUG(SSSDBG_OP_FAILURE, ("get_changepw_options failed.\n"));
+        goto sendresponse;
+    }
+
     sss_krb5_princ_realm(kr->ctx, kr->princ, &realm_name, &realm_length);
 
     DEBUG(SSSDBG_TRACE_FUNC,
@@ -1089,7 +1118,8 @@ static errno_t changepw_child(int fd, struct krb5_req *kr)
     kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ,
                                         pass_str, prompter, kr, 0,
                                         SSSD_KRB5_CHANGEPW_PRINCIPAL,
-                                        kr->options);
+                                        chagepw_options);
+    sss_krb5_get_init_creds_opt_free(kr->ctx, chagepw_options);
     if (kerr != 0) {
         pam_status = kerr_handle_error(kerr);
         goto sendresponse;
@@ -1196,6 +1226,7 @@ static errno_t tgt_req_child(int fd, struct krb5_req *kr)
     krb5_error_code kerr = 0;
     char *pass_str = NULL;
     int pam_status = PAM_SYSTEM_ERR;
+    krb5_get_init_creds_opt *chagepw_options;
 
     DEBUG(SSSDBG_TRACE_LIBS, ("Attempting to get a TGT\n"));
 
@@ -1229,10 +1260,20 @@ static errno_t tgt_req_child(int fd, struct krb5_req *kr)
             KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);
             DEBUG(1, ("Failed to unset expire callback, continue ...\n"));
         }
+
+        kerr = get_changepw_options(kr->ctx, &chagepw_options);
+        if (kerr != 0) {
+            DEBUG(SSSDBG_OP_FAILURE, ("get_changepw_options failed.\n"));
+            goto sendresponse;
+        }
+
         kerr = krb5_get_init_creds_password(kr->ctx, kr->creds, kr->princ,
                                             pass_str, sss_krb5_prompter, kr, 0,
                                             SSSD_KRB5_CHANGEPW_PRINCIPAL,
-                                            kr->options);
+                                            chagepw_options);
+
+        sss_krb5_get_init_creds_opt_free(kr->ctx, chagepw_options);
+
         krb5_free_cred_contents(kr->ctx, kr->creds);
         if (kerr == 0) {
             kerr = KRB5KDC_ERR_KEY_EXP;
-- 
1.7.7.6



More information about the sssd-devel mailing list