The branch, master has been updated
       via  ede3046 s4:auth/kerberos: protect kerberos_kinit_password_cc() 
against old KDCs
       via  e5378e6 s4:auth/kerberos: remove one indentation level in 
kerberos_kinit_password_cc()
       via  b98428e s4:auth/kerberos: reformat kerberos_kinit_password_cc()
       via  9c56303 s4:auth/kerberos: don't mix s4u2self creds with machine 
account creds
       via  b3d4962 s4:auth/kerberos: use better variable names in 
kerberos_kinit_password_cc()
       via  7cf3842 s4:auth/kerberos: don't ignore return code in 
kerberos_kinit_password_cc()
      from  9e766f0 samba-tool: added missing GUID component checks to dbcheck

http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit ede3046b8b9b0576a35626026cb28c31b42da46d
Author: Stefan Metzmacher <me...@samba.org>
Date:   Tue Jun 21 01:39:58 2011 +0200

    s4:auth/kerberos: protect kerberos_kinit_password_cc() against old KDCs
    
    Old KDCs may not support S4U2Self (or S4U2Proxy) and return tickets
    which belongs to the client principal of the TGT.
    
    metze
    
    Autobuild-User: Stefan Metzmacher <me...@samba.org>
    Autobuild-Date: Wed Jun 22 09:10:55 CEST 2011 on sn-devel-104

commit e5378e600e507241dd64c1ea7345676076dc8755
Author: Stefan Metzmacher <me...@samba.org>
Date:   Mon Jun 20 21:23:45 2011 +0200

    s4:auth/kerberos: remove one indentation level in 
kerberos_kinit_password_cc()
    
    This will make the following changes easier to review.
    
    metze

commit b98428e630cc5a1bbc18bf4260030a24322fdf9e
Author: Stefan Metzmacher <me...@samba.org>
Date:   Mon Jun 20 21:09:13 2011 +0200

    s4:auth/kerberos: reformat kerberos_kinit_password_cc()
    
    In order to make the following changes easier to review.
    
    metze

commit 9c56303f5a56697470ea9f2ee1a428aed2367d75
Author: Stefan Metzmacher <me...@samba.org>
Date:   Mon Jun 20 15:27:58 2011 +0200

    s4:auth/kerberos: don't mix s4u2self creds with machine account creds
    
    It's important that we don't store the tgt for the machine account
    in the same krb5_ccache as the ticket for the impersonated principal.
    
    We may pass it to some krb5/gssapi functions and they may use them
    in the wrong way, which would grant machine account privileges to
    the client.
    
    metze

commit b3d49620875d878e2ad39896a6fe9fddb039253e
Author: Stefan Metzmacher <me...@samba.org>
Date:   Mon Jun 20 18:01:49 2011 +0200

    s4:auth/kerberos: use better variable names in kerberos_kinit_password_cc()
    
    This will make the following changes easier to review.
    
    metze

commit 7cf38425b274c43144a2216accf5330d8ef1fe36
Author: Stefan Metzmacher <me...@samba.org>
Date:   Mon Jun 20 17:41:52 2011 +0200

    s4:auth/kerberos: don't ignore return code in kerberos_kinit_password_cc()
    
    metze

-----------------------------------------------------------------------

Summary of changes:
 source4/auth/kerberos/kerberos.c |  228 +++++++++++++++++++++++++++++--------
 1 files changed, 178 insertions(+), 50 deletions(-)


Changeset truncated at 500 lines:

diff --git a/source4/auth/kerberos/kerberos.c b/source4/auth/kerberos/kerberos.c
index 0db0dd3..fa8c64b 100644
--- a/source4/auth/kerberos/kerberos.c
+++ b/source4/auth/kerberos/kerberos.c
@@ -84,82 +84,210 @@
   The target_service defaults to the krbtgt if NULL, but could be 
kpasswd/realm or the local service (if we are doing s4u2self)
 
 */
- krb5_error_code kerberos_kinit_password_cc(krb5_context ctx, krb5_ccache cc, 
-                                           krb5_principal principal, const 
char *password,
-                                           krb5_principal 
impersonate_principal, const char *target_service,
+ krb5_error_code kerberos_kinit_password_cc(krb5_context ctx, krb5_ccache 
store_cc,
+                                           krb5_principal init_principal,
+                                           const char *init_password,
+                                           krb5_principal 
impersonate_principal,
+                                           const char *target_service,
                                            krb5_get_init_creds_opt 
*krb_options,
                                            time_t *expire_time, time_t 
*kdc_time)
 {
        krb5_error_code code = 0;
-       krb5_creds my_creds;
-       krb5_creds *impersonate_creds;
        krb5_get_creds_opt options;
+       krb5_principal store_principal;
+       krb5_creds store_creds;
+       const char *self_service = target_service;
+       krb5_creds *s4u2self_creds;
+       krb5_principal self_princ;
+       krb5_ccache tmp_cc;
+       const char *self_realm;
+       krb5_principal blacklist_principal = NULL;
 
-       /* If we are not impersonating, then get this ticket for the
+       /*
+        * If we are not impersonating, then get this ticket for the
         * target service, otherwise a krbtgt, and get the next ticket
-        * for the target */
-       if ((code = krb5_get_init_creds_password(ctx, &my_creds, principal, 
password, 
-                                                NULL, NULL,
-                                                0,
-                                                impersonate_principal ? NULL : 
target_service,
-                                                krb_options))) {
+        * for the target
+        */
+       code = krb5_get_init_creds_password(ctx, &store_creds,
+                                           init_principal,
+                                           init_password,
+                                           NULL, NULL,
+                                           0,
+                                           impersonate_principal ? NULL : 
target_service,
+                                           krb_options);
+       if (code != 0) {
                return code;
        }
 
-       if ((code = krb5_cc_initialize(ctx, cc, principal))) {
-               krb5_free_cred_contents(ctx, &my_creds);
+       store_principal = init_principal;
+
+       if (impersonate_principal == NULL) {
+               goto store;
+       }
+
+       /*
+        * We are trying S4U2Self now:
+        *
+        * As we do not want to expose our TGT in the
+        * krb5_ccache, which is also holds the impersonated creds.
+        *
+        * Some low level krb5/gssapi function might use the TGT
+        * identity and let the client act as our machine account.
+        *
+        * We need to avoid that and use a temporary krb5_ccache
+        * in order to pass our TGT to the krb5_get_creds() function.
+        */
+       code = krb5_cc_new_unique(ctx, NULL, NULL, &tmp_cc);
+       if (code != 0) {
+               krb5_free_cred_contents(ctx, &store_creds);
                return code;
        }
-       
-       if ((code = krb5_cc_store_cred(ctx, cc, &my_creds))) {
-               krb5_free_cred_contents(ctx, &my_creds);
+
+       code = krb5_cc_initialize(ctx, tmp_cc, store_creds.client);
+       if (code != 0) {
+               krb5_cc_destroy(ctx, tmp_cc);
+               krb5_free_cred_contents(ctx, &store_creds);
                return code;
        }
-       
-       if (expire_time) {
-               *expire_time = (time_t) my_creds.times.endtime;
+
+       code = krb5_cc_store_cred(ctx, tmp_cc, &store_creds);
+       if (code != 0) {
+               krb5_free_cred_contents(ctx, &store_creds);
+               krb5_cc_destroy(ctx, tmp_cc);
+               return code;
        }
 
-       if (kdc_time) {
-               *kdc_time = (time_t) my_creds.times.starttime;
+       /*
+        * we need to remember the client principal of our
+        * TGT and make sure the KDC does not return this
+        * in the impersonated tickets. This can happen
+        * if the KDC does not support S4U2Self and S4U2Proxy.
+        */
+       blacklist_principal = store_creds.client;
+       store_creds.client = NULL;
+       krb5_free_cred_contents(ctx, &store_creds);
+
+       /*
+        * For S4U2Self we need our own service principal,
+        * which belongs to our own realm (available on
+        * our client principal).
+        */
+       self_realm = krb5_principal_get_realm(ctx, init_principal);
+
+       code = krb5_parse_name(ctx, self_service, &self_princ);
+       if (code != 0) {
+               krb5_free_principal(ctx, blacklist_principal);
+               krb5_cc_destroy(ctx, tmp_cc);
+               return code;
        }
 
-       krb5_free_cred_contents(ctx, &my_creds);
-       
-       if (code == 0 && impersonate_principal) {
-               krb5_principal target_princ;
-               if ((code = krb5_get_creds_opt_alloc(ctx, &options))) {
-                       return code;
-               }
+       code = krb5_principal_set_realm(ctx, self_princ, self_realm);
+       if (code != 0) {
+               krb5_free_principal(ctx, blacklist_principal);
+               krb5_free_principal(ctx, self_princ);
+               krb5_cc_destroy(ctx, tmp_cc);
+               return code;
+       }
 
-               if ((code = krb5_get_creds_opt_set_impersonate(ctx, options, 
impersonate_principal))) {
-                       krb5_get_creds_opt_free(ctx, options);
-                       return code;
-               }
+       code = krb5_get_creds_opt_alloc(ctx, &options);
+       if (code != 0) {
+               krb5_free_principal(ctx, blacklist_principal);
+               krb5_free_principal(ctx, self_princ);
+               krb5_cc_destroy(ctx, tmp_cc);
+               return code;
+       }
 
-               if ((code = krb5_parse_name(ctx, target_service, 
&target_princ))) {
-                       krb5_get_creds_opt_free(ctx, options);
-                       return code;
-               }
+       code = krb5_get_creds_opt_set_impersonate(ctx, options,
+                                                 impersonate_principal);
+       if (code != 0) {
+               krb5_get_creds_opt_free(ctx, options);
+               krb5_free_principal(ctx, blacklist_principal);
+               krb5_free_principal(ctx, self_princ);
+               krb5_cc_destroy(ctx, tmp_cc);
+               return code;
+       }
 
-               if ((code = krb5_principal_set_realm(ctx, target_princ, 
krb5_principal_get_realm(ctx, principal)))) {
-                       krb5_get_creds_opt_free(ctx, options);
-                       krb5_free_principal(ctx, target_princ);
-                       return code;
-               }
+       code = krb5_get_creds(ctx, options, tmp_cc,
+                             self_princ, &s4u2self_creds);
+       krb5_get_creds_opt_free(ctx, options);
+       krb5_free_principal(ctx, self_princ);
+       krb5_cc_destroy(ctx, tmp_cc);
+       if (code != 0) {
+               krb5_free_principal(ctx, blacklist_principal);
+               return code;
+       }
 
-               if ((code = krb5_get_creds(ctx, options, cc, target_princ, 
&impersonate_creds))) {
-                       krb5_free_principal(ctx, target_princ);
-                       krb5_get_creds_opt_free(ctx, options);
-                       return code;
+       /*
+        * Now make sure we store the impersonated principal
+        * and creds instead of the TGT related stuff
+        * in the krb5_ccache of the caller.
+        */
+       code = krb5_copy_creds_contents(ctx, s4u2self_creds,
+                                       &store_creds);
+       krb5_free_creds(ctx, s4u2self_creds);
+       if (code != 0) {
+               krb5_free_principal(ctx, blacklist_principal);
+               return code;
+       }
+
+       /*
+        * It's important to store the principal the KDC
+        * returned, as otherwise the caller would not find
+        * the S4U2Self ticket in the krb5_ccache lookup.
+        */
+       store_principal = store_creds.client;
+
+ store:
+       if (blacklist_principal &&
+           krb5_principal_compare(ctx, store_creds.client, 
blacklist_principal)) {
+               char *sp = NULL;
+               char *ip = NULL;
+
+               code = krb5_unparse_name(ctx, blacklist_principal, &sp);
+               if (code != 0) {
+                       sp = NULL;
+               }
+               code = krb5_unparse_name(ctx, impersonate_principal, &ip);
+               if (code != 0) {
+                       ip = NULL;
                }
+               DEBUG(1, ("kerberos_kinit_password_cc: "
+                         "KDC returned self principal[%s] while impersonating 
[%s]\n",
+                         sp?sp:"<no memory>",
+                         ip?ip:"<no memory>"));
 
-               krb5_free_principal(ctx, target_princ);
+               SAFE_FREE(sp);
+               SAFE_FREE(ip);
 
-               code = krb5_cc_store_cred(ctx, cc, impersonate_creds);
-               krb5_get_creds_opt_free(ctx, options);
-               krb5_free_creds(ctx, impersonate_creds);
+               krb5_free_principal(ctx, blacklist_principal);
+               krb5_free_cred_contents(ctx, &store_creds);
+               return KRB5_FWD_BAD_PRINCIPAL;
        }
+       if (blacklist_principal) {
+               krb5_free_principal(ctx, blacklist_principal);
+       }
+
+       code = krb5_cc_initialize(ctx, store_cc, store_principal);
+       if (code != 0) {
+               krb5_free_cred_contents(ctx, &store_creds);
+               return code;
+       }
+
+       code = krb5_cc_store_cred(ctx, store_cc, &store_creds);
+       if (code != 0) {
+               krb5_free_cred_contents(ctx, &store_creds);
+               return code;
+       }
+
+       if (expire_time) {
+               *expire_time = (time_t) store_creds.times.endtime;
+       }
+
+       if (kdc_time) {
+               *kdc_time = (time_t) store_creds.times.starttime;
+       }
+
+       krb5_free_cred_contents(ctx, &store_creds);
 
        return 0;
 }


-- 
Samba Shared Repository

Reply via email to