[SSSD] [PATCH] Fix fallback in validate_tgt()

Sumit Bose sbose at redhat.com
Fri Aug 24 10:32:20 UTC 2012


Hi,

this patch should fix the trusted domain use case by fixing an error
which prevented the intended behaviour in validate_tgt(). Details about
TGT validation should be also explained in the man page, but since there
is already the string freeze for 1.9 I opened
https://fedorahosted.org/sssd/ticket/1499 to track this.

The second patch just replace the remaining old debug levels with the
new ones.

bye,
Sumit
-------------- next part --------------
From a7abacfbb1e9921eb43562449fca45e2a7d9e255 Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 24 Aug 2012 10:56:38 +0200
Subject: [PATCH 1/2] Fix fallback in validate_tgt()

To validate a TGT a keytab entry from the client realm is preferred but
if none ca be found the last entry should be used. But the entry was
freed and zeroed before it could be used.

This should also fix the trusted domain use case mentioned in
https://fedorahosted.org/sssd/ticket/1396
although a different approach then suggested in the ticket is used.
---
 src/providers/krb5/krb5_child.c |   28 ++++++++++++++++++++--------
 1 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 817a3d01ee1e4247e66db4eac512e151f506c077..7ae184cba2774656110c6217fc5e34bd56294788 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -695,6 +695,7 @@ static krb5_error_code validate_tgt(struct krb5_req *kr)
     krb5_kt_cursor cursor;
     krb5_keytab_entry entry;
     krb5_verify_init_creds_opt opt;
+    krb5_principal validation_princ = NULL;
 
     memset(&keytab, 0, sizeof(keytab));
     kerr = krb5_kt_resolve(kr->ctx, kr->keytab, &keytab);
@@ -715,10 +716,15 @@ static krb5_error_code validate_tgt(struct krb5_req *kr)
     /* We look for the first entry from our realm or take the last one */
     memset(&entry, 0, sizeof(entry));
     while ((kt_err = krb5_kt_next_entry(kr->ctx, keytab, &entry, &cursor)) == 0) {
-        if (krb5_realm_compare(kr->ctx, entry.principal, kr->princ)) {
-            DEBUG(SSSDBG_TRACE_INTERNAL,
-                  ("Found keytab entry with the realm of the credential.\n"));
-            break;
+        if (validation_princ != NULL) {
+            krb5_free_principal(kr->ctx, validation_princ);
+            validation_princ = NULL;
+        }
+        kerr = krb5_copy_principal(kr->ctx, entry.principal,
+                                   &validation_princ);
+        if (kerr != 0) {
+            DEBUG(SSSDBG_CRIT_FAILURE, ("krb5_copy_principal failed.\n"));
+            goto done;
         }
 
         kerr = sss_krb5_free_keytab_entry_contents(kr->ctx, &entry);
@@ -726,6 +732,12 @@ static krb5_error_code validate_tgt(struct krb5_req *kr)
             DEBUG(1, ("Failed to free keytab entry.\n"));
         }
         memset(&entry, 0, sizeof(entry));
+
+        if (krb5_realm_compare(kr->ctx, validation_princ, kr->princ)) {
+            DEBUG(SSSDBG_TRACE_INTERNAL,
+                  ("Found keytab entry with the realm of the credential.\n"));
+            break;
+        }
     }
 
     /* Close the keytab here.  Even though we're using cursors, the file
@@ -747,7 +759,7 @@ static krb5_error_code validate_tgt(struct krb5_req *kr)
 
     /* Get the principal to which the key belongs, for logging purposes. */
     principal = NULL;
-    kerr = krb5_unparse_name(kr->ctx, entry.principal, &principal);
+    kerr = krb5_unparse_name(kr->ctx, validation_princ, &principal);
     if (kerr != 0) {
         DEBUG(1, ("internal error parsing principal name, "
                   "not verifying TGT.\n"));
@@ -757,7 +769,7 @@ static krb5_error_code validate_tgt(struct krb5_req *kr)
 
 
     krb5_verify_init_creds_opt_init(&opt);
-    kerr = krb5_verify_init_creds(kr->ctx, kr->creds, entry.principal, keytab,
+    kerr = krb5_verify_init_creds(kr->ctx, kr->creds, validation_princ, keytab,
                                   NULL, &opt);
 
     if (kerr == 0) {
@@ -770,8 +782,8 @@ done:
     if (krb5_kt_close(kr->ctx, keytab) != 0) {
         DEBUG(1, ("krb5_kt_close failed"));
     }
-    if (sss_krb5_free_keytab_entry_contents(kr->ctx, &entry) != 0) {
-        DEBUG(1, ("Failed to free keytab entry.\n"));
+    if (validation_princ != NULL) {
+        krb5_free_principal(kr->ctx, validation_princ);
     }
     if (principal != NULL) {
         sss_krb5_free_unparsed_name(kr->ctx, principal);
-- 
1.7.7.6

-------------- next part --------------
From 975806865051ba49a61a53c9cdec970bdb0d2a5c Mon Sep 17 00:00:00 2001
From: Sumit Bose <sbose at redhat.com>
Date: Fri, 24 Aug 2012 12:09:05 +0200
Subject: [PATCH 2/2] Use new debug levels in validate_tgt()

---
 src/providers/krb5/krb5_child.c |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/src/providers/krb5/krb5_child.c b/src/providers/krb5/krb5_child.c
index 7ae184cba2774656110c6217fc5e34bd56294788..7562bb451c98b81453d9ecd9f39b505ea54e3b50 100644
--- a/src/providers/krb5/krb5_child.c
+++ b/src/providers/krb5/krb5_child.c
@@ -700,16 +700,16 @@ static krb5_error_code validate_tgt(struct krb5_req *kr)
     memset(&keytab, 0, sizeof(keytab));
     kerr = krb5_kt_resolve(kr->ctx, kr->keytab, &keytab);
     if (kerr != 0) {
-        DEBUG(1, ("error resolving keytab [%s], not verifying TGT.\n",
-                  kr->keytab));
+        DEBUG(SSSDBG_CRIT_FAILURE, ("error resolving keytab [%s], " \
+                                    "not verifying TGT.\n", kr->keytab));
         return kerr;
     }
 
     memset(&cursor, 0, sizeof(cursor));
     kerr = krb5_kt_start_seq_get(kr->ctx, keytab, &cursor);
     if (kerr != 0) {
-        DEBUG(1, ("error reading keytab [%s], not verifying TGT.\n",
-                  kr->keytab));
+        DEBUG(SSSDBG_CRIT_FAILURE, ("error reading keytab [%s], " \
+                                    "not verifying TGT.\n", kr->keytab));
         return kerr;
     }
 
@@ -729,7 +729,7 @@ static krb5_error_code validate_tgt(struct krb5_req *kr)
 
         kerr = sss_krb5_free_keytab_entry_contents(kr->ctx, &entry);
         if (kerr != 0) {
-            DEBUG(1, ("Failed to free keytab entry.\n"));
+            DEBUG(SSSDBG_MINOR_FAILURE, ("Failed to free keytab entry.\n"));
         }
         memset(&entry, 0, sizeof(entry));
 
@@ -746,14 +746,15 @@ static krb5_error_code validate_tgt(struct krb5_req *kr)
      * cursor, creating a leak. */
     kerr = krb5_kt_end_seq_get(kr->ctx, keytab, &cursor);
     if (kerr != 0) {
-        DEBUG(1, ("krb5_kt_end_seq_get failed, not verifying TGT.\n"));
+        DEBUG(SSSDBG_CRIT_FAILURE, ("krb5_kt_end_seq_get failed, " \
+                                    "not verifying TGT.\n"));
         goto done;
     }
 
     /* check if we got any errors from krb5_kt_next_entry */
     if (kt_err != 0 && kt_err != KRB5_KT_END) {
-        DEBUG(1, ("error reading keytab [%s], not verifying TGT.\n",
-                  kr->keytab));
+        DEBUG(SSSDBG_CRIT_FAILURE, ("error reading keytab [%s], " \
+                                    "not verifying TGT.\n", kr->keytab));
         goto done;
     }
 
@@ -761,8 +762,8 @@ static krb5_error_code validate_tgt(struct krb5_req *kr)
     principal = NULL;
     kerr = krb5_unparse_name(kr->ctx, validation_princ, &principal);
     if (kerr != 0) {
-        DEBUG(1, ("internal error parsing principal name, "
-                  "not verifying TGT.\n"));
+        DEBUG(SSSDBG_CRIT_FAILURE, ("internal error parsing principal name, "
+                                    "not verifying TGT.\n"));
         KRB5_CHILD_DEBUG(SSSDBG_CRIT_FAILURE, kerr);
         goto done;
     }
@@ -773,14 +774,16 @@ static krb5_error_code validate_tgt(struct krb5_req *kr)
                                   NULL, &opt);
 
     if (kerr == 0) {
-        DEBUG(5, ("TGT verified using key for [%s].\n", principal));
+        DEBUG(SSSDBG_TRACE_FUNC, ("TGT verified using key for [%s].\n",
+                                  principal));
     } else {
-        DEBUG(1 ,("TGT failed verification using key for [%s].\n", principal));
+        DEBUG(SSSDBG_CRIT_FAILURE ,("TGT failed verification using key " \
+                                    "for [%s].\n", principal));
     }
 
 done:
     if (krb5_kt_close(kr->ctx, keytab) != 0) {
-        DEBUG(1, ("krb5_kt_close failed"));
+        DEBUG(SSSDBG_MINOR_FAILURE, ("krb5_kt_close failed"));
     }
     if (validation_princ != NULL) {
         krb5_free_principal(kr->ctx, validation_princ);
-- 
1.7.7.6



More information about the sssd-devel mailing list