From 7cf38425b274c43144a2216accf5330d8ef1fe36 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 20 Jun 2011 17:41:52 +0200 Subject: s4:auth/kerberos: don't ignore return code in kerberos_kinit_password_cc() metze --- source4/auth/kerberos/kerberos.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'source4/auth/kerberos/kerberos.c') diff --git a/source4/auth/kerberos/kerberos.c b/source4/auth/kerberos/kerberos.c index 0db0dd3ced1..8824d748d2a 100644 --- a/source4/auth/kerberos/kerberos.c +++ b/source4/auth/kerberos/kerberos.c @@ -159,6 +159,8 @@ code = krb5_cc_store_cred(ctx, cc, impersonate_creds); krb5_get_creds_opt_free(ctx, options); krb5_free_creds(ctx, impersonate_creds); + + return code; } return 0; -- cgit From b3d49620875d878e2ad39896a6fe9fddb039253e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 20 Jun 2011 18:01:49 +0200 Subject: s4:auth/kerberos: use better variable names in kerberos_kinit_password_cc() This will make the following changes easier to review. metze --- source4/auth/kerberos/kerberos.c | 68 ++++++++++++++++++++++++---------------- 1 file changed, 41 insertions(+), 27 deletions(-) (limited to 'source4/auth/kerberos/kerberos.c') diff --git a/source4/auth/kerberos/kerberos.c b/source4/auth/kerberos/kerberos.c index 8824d748d2a..5e38d878beb 100644 --- a/source4/auth/kerberos/kerberos.c +++ b/source4/auth/kerberos/kerberos.c @@ -85,20 +85,24 @@ */ 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_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_creds init_creds; krb5_get_creds_opt options; + const char *self_service = target_service; /* 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, + if ((code = krb5_get_init_creds_password(ctx, &init_creds, + init_principal, + init_password, NULL, NULL, 0, impersonate_principal ? NULL : target_service, @@ -106,59 +110,69 @@ return code; } - if ((code = krb5_cc_initialize(ctx, cc, principal))) { - krb5_free_cred_contents(ctx, &my_creds); + if ((code = krb5_cc_initialize(ctx, cc, init_principal))) { + krb5_free_cred_contents(ctx, &init_creds); return code; } - if ((code = krb5_cc_store_cred(ctx, cc, &my_creds))) { - krb5_free_cred_contents(ctx, &my_creds); + if ((code = krb5_cc_store_cred(ctx, cc, &init_creds))) { + krb5_free_cred_contents(ctx, &init_creds); return code; } if (expire_time) { - *expire_time = (time_t) my_creds.times.endtime; + *expire_time = (time_t) init_creds.times.endtime; } if (kdc_time) { - *kdc_time = (time_t) my_creds.times.starttime; + *kdc_time = (time_t) init_creds.times.starttime; } - krb5_free_cred_contents(ctx, &my_creds); - + krb5_free_cred_contents(ctx, &init_creds); + if (code == 0 && impersonate_principal) { - krb5_principal target_princ; - if ((code = krb5_get_creds_opt_alloc(ctx, &options))) { + krb5_creds *s4u2self_creds; + krb5_principal self_princ; + const char *self_realm; + + /* + * 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); + + if ((code = krb5_parse_name(ctx, self_service, &self_princ))) { return code; } - if ((code = krb5_get_creds_opt_set_impersonate(ctx, options, impersonate_principal))) { - krb5_get_creds_opt_free(ctx, options); + if ((code = krb5_principal_set_realm(ctx, self_princ, self_realm))) { + krb5_free_principal(ctx, self_princ); return code; } - if ((code = krb5_parse_name(ctx, target_service, &target_princ))) { - krb5_get_creds_opt_free(ctx, options); + if ((code = krb5_get_creds_opt_alloc(ctx, &options))) { + krb5_free_principal(ctx, self_princ); return code; } - if ((code = krb5_principal_set_realm(ctx, target_princ, krb5_principal_get_realm(ctx, principal)))) { + if ((code = krb5_get_creds_opt_set_impersonate(ctx, options, impersonate_principal))) { krb5_get_creds_opt_free(ctx, options); - krb5_free_principal(ctx, target_princ); + krb5_free_principal(ctx, self_princ); return code; } - if ((code = krb5_get_creds(ctx, options, cc, target_princ, &impersonate_creds))) { - krb5_free_principal(ctx, target_princ); + if ((code = krb5_get_creds(ctx, options, cc, self_princ, &s4u2self_creds))) { krb5_get_creds_opt_free(ctx, options); + krb5_free_principal(ctx, self_princ); return code; } - krb5_free_principal(ctx, target_princ); - - 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, self_princ); + + code = krb5_cc_store_cred(ctx, cc, s4u2self_creds); + krb5_free_creds(ctx, s4u2self_creds); return code; } -- cgit From 9c56303f5a56697470ea9f2ee1a428aed2367d75 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 20 Jun 2011 15:27:58 +0200 Subject: 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 --- source4/auth/kerberos/kerberos.c | 100 +++++++++++++++++++++++++++++---------- 1 file changed, 76 insertions(+), 24 deletions(-) (limited to 'source4/auth/kerberos/kerberos.c') diff --git a/source4/auth/kerberos/kerberos.c b/source4/auth/kerberos/kerberos.c index 5e38d878beb..5feb3e62211 100644 --- a/source4/auth/kerberos/kerberos.c +++ b/source4/auth/kerberos/kerberos.c @@ -84,7 +84,7 @@ 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_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, @@ -93,14 +93,15 @@ time_t *expire_time, time_t *kdc_time) { krb5_error_code code = 0; - krb5_creds init_creds; krb5_get_creds_opt options; + krb5_principal store_principal; + krb5_creds store_creds; const char *self_service = target_service; /* 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, &init_creds, + if ((code = krb5_get_init_creds_password(ctx, &store_creds, init_principal, init_password, NULL, NULL, @@ -110,31 +111,43 @@ return code; } - if ((code = krb5_cc_initialize(ctx, cc, init_principal))) { - krb5_free_cred_contents(ctx, &init_creds); - return code; - } - - if ((code = krb5_cc_store_cred(ctx, cc, &init_creds))) { - krb5_free_cred_contents(ctx, &init_creds); - return code; - } - - if (expire_time) { - *expire_time = (time_t) init_creds.times.endtime; - } - - if (kdc_time) { - *kdc_time = (time_t) init_creds.times.starttime; - } - - krb5_free_cred_contents(ctx, &init_creds); + store_principal = init_principal; if (code == 0 && impersonate_principal) { + krb5_ccache tmp_cc; krb5_creds *s4u2self_creds; krb5_principal self_princ; const char *self_realm; + /* + * 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. + */ + if ((code = krb5_cc_new_unique(ctx, NULL, NULL, &tmp_cc))) { + krb5_free_cred_contents(ctx, &store_creds); + return code; + } + + if ((code = krb5_cc_initialize(ctx, tmp_cc, store_creds.client))) { + krb5_cc_destroy(ctx, tmp_cc); + krb5_free_cred_contents(ctx, &store_creds); + return code; + } + + if ((code = krb5_cc_store_cred(ctx, tmp_cc, &store_creds))) { + krb5_cc_destroy(ctx, tmp_cc); + krb5_free_cred_contents(ctx, &store_creds); + return code; + } + + krb5_free_cred_contents(ctx, &store_creds); + /* * For S4U2Self we need our own service principal, * which belongs to our own realm (available on @@ -143,40 +156,79 @@ self_realm = krb5_principal_get_realm(ctx, init_principal); if ((code = krb5_parse_name(ctx, self_service, &self_princ))) { + krb5_cc_destroy(ctx, tmp_cc); return code; } if ((code = krb5_principal_set_realm(ctx, self_princ, self_realm))) { krb5_free_principal(ctx, self_princ); + krb5_cc_destroy(ctx, tmp_cc); return code; } if ((code = krb5_get_creds_opt_alloc(ctx, &options))) { 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); krb5_free_principal(ctx, self_princ); + krb5_cc_destroy(ctx, tmp_cc); return code; } - if ((code = krb5_get_creds(ctx, options, cc, self_princ, &s4u2self_creds))) { + if ((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); return code; } krb5_get_creds_opt_free(ctx, options); krb5_free_principal(ctx, self_princ); + krb5_cc_destroy(ctx, tmp_cc); - code = krb5_cc_store_cred(ctx, cc, s4u2self_creds); + /* + * Now make sure we store the impersonated principal + * and creds instead of the TGT related stuff + * in the krb5_ccache of the caller. + */ + if ((code = krb5_copy_creds_contents(ctx, s4u2self_creds, &store_creds))) { + krb5_free_creds(ctx, s4u2self_creds); + return code; + } krb5_free_creds(ctx, s4u2self_creds); + /* + * 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; + } + + if ((code = krb5_cc_initialize(ctx, store_cc, store_principal))) { + krb5_free_cred_contents(ctx, &store_creds); return code; } + if ((code = krb5_cc_store_cred(ctx, store_cc, &store_creds))) { + 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; } -- cgit From b98428e630cc5a1bbc18bf4260030a24322fdf9e Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 20 Jun 2011 21:09:13 +0200 Subject: s4:auth/kerberos: reformat kerberos_kinit_password_cc() In order to make the following changes easier to review. metze --- source4/auth/kerberos/kerberos.c | 73 ++++++++++++++++++++++------------------ 1 file changed, 41 insertions(+), 32 deletions(-) (limited to 'source4/auth/kerberos/kerberos.c') diff --git a/source4/auth/kerberos/kerberos.c b/source4/auth/kerberos/kerberos.c index 5feb3e62211..1ba6952e2b1 100644 --- a/source4/auth/kerberos/kerberos.c +++ b/source4/auth/kerberos/kerberos.c @@ -98,22 +98,25 @@ krb5_creds store_creds; const char *self_service = target_service; - /* 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, &store_creds, - init_principal, - init_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; } store_principal = init_principal; - if (code == 0 && impersonate_principal) { + if (impersonate_principal) { krb5_ccache tmp_cc; krb5_creds *s4u2self_creds; krb5_principal self_princ; @@ -129,25 +132,26 @@ * We need to avoid that and use a temporary krb5_ccache * in order to pass our TGT to the krb5_get_creds() function. */ - if ((code = krb5_cc_new_unique(ctx, NULL, NULL, &tmp_cc))) { + 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_initialize(ctx, tmp_cc, store_creds.client))) { + 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 ((code = krb5_cc_store_cred(ctx, tmp_cc, &store_creds))) { + code = krb5_cc_store_cred(ctx, tmp_cc, &store_creds); + krb5_free_cred_contents(ctx, &store_creds); + if (code != 0) { krb5_cc_destroy(ctx, tmp_cc); - krb5_free_cred_contents(ctx, &store_creds); return code; } - krb5_free_cred_contents(ctx, &store_creds); - /* * For S4U2Self we need our own service principal, * which belongs to our own realm (available on @@ -155,51 +159,54 @@ */ self_realm = krb5_principal_get_realm(ctx, init_principal); - if ((code = krb5_parse_name(ctx, self_service, &self_princ))) { - krb5_cc_destroy(ctx, tmp_cc); - return code; - } - - if ((code = krb5_principal_set_realm(ctx, self_princ, self_realm))) { - krb5_free_principal(ctx, self_princ); + code = krb5_parse_name(ctx, self_service, &self_princ); + if (code != 0) { krb5_cc_destroy(ctx, tmp_cc); return code; } - if ((code = krb5_get_creds_opt_alloc(ctx, &options))) { + code = krb5_principal_set_realm(ctx, self_princ, self_realm); + if (code != 0) { 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); + code = krb5_get_creds_opt_alloc(ctx, &options); + if (code != 0) { krb5_free_principal(ctx, self_princ); krb5_cc_destroy(ctx, tmp_cc); return code; } - if ((code = krb5_get_creds(ctx, options, tmp_cc, self_princ, &s4u2self_creds))) { + 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, self_princ); krb5_cc_destroy(ctx, tmp_cc); 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) { + 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. */ - if ((code = krb5_copy_creds_contents(ctx, s4u2self_creds, &store_creds))) { - krb5_free_creds(ctx, s4u2self_creds); + code = krb5_copy_creds_contents(ctx, s4u2self_creds, &store_creds); + krb5_free_creds(ctx, s4u2self_creds); + if (code != 0) { return code; } - krb5_free_creds(ctx, s4u2self_creds); /* * It's important to store the principal the KDC @@ -209,12 +216,14 @@ store_principal = store_creds.client; } - if ((code = krb5_cc_initialize(ctx, store_cc, store_principal))) { + code = krb5_cc_initialize(ctx, store_cc, store_principal); + if (code != 0) { krb5_free_cred_contents(ctx, &store_creds); return code; } - if ((code = krb5_cc_store_cred(ctx, store_cc, &store_creds))) { + code = krb5_cc_store_cred(ctx, store_cc, &store_creds); + if (code != 0) { krb5_free_cred_contents(ctx, &store_creds); return code; } -- cgit From e5378e600e507241dd64c1ea7345676076dc8755 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 20 Jun 2011 21:23:45 +0200 Subject: s4:auth/kerberos: remove one indentation level in kerberos_kinit_password_cc() This will make the following changes easier to review. metze --- source4/auth/kerberos/kerberos.c | 193 ++++++++++++++++++++------------------- 1 file changed, 99 insertions(+), 94 deletions(-) (limited to 'source4/auth/kerberos/kerberos.c') diff --git a/source4/auth/kerberos/kerberos.c b/source4/auth/kerberos/kerberos.c index 1ba6952e2b1..4acb54fb335 100644 --- a/source4/auth/kerberos/kerberos.c +++ b/source4/auth/kerberos/kerberos.c @@ -97,6 +97,10 @@ 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; /* * If we are not impersonating, then get this ticket for the @@ -116,106 +120,107 @@ store_principal = init_principal; - if (impersonate_principal) { - krb5_ccache tmp_cc; - krb5_creds *s4u2self_creds; - krb5_principal self_princ; - const char *self_realm; - - /* - * 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; - } - - 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; - } - - code = krb5_cc_store_cred(ctx, tmp_cc, &store_creds); + 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; + } + + 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); - if (code != 0) { - krb5_cc_destroy(ctx, tmp_cc); - return code; - } - - /* - * 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_cc_destroy(ctx, tmp_cc); - return code; - } - - code = krb5_principal_set_realm(ctx, self_princ, self_realm); - if (code != 0) { - krb5_free_principal(ctx, self_princ); - krb5_cc_destroy(ctx, tmp_cc); - return code; - } - - code = krb5_get_creds_opt_alloc(ctx, &options); - if (code != 0) { - krb5_free_principal(ctx, self_princ); - krb5_cc_destroy(ctx, tmp_cc); - 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, self_princ); - krb5_cc_destroy(ctx, tmp_cc); - return code; - } - - code = krb5_get_creds(ctx, options, tmp_cc, - self_princ, &s4u2self_creds); + return code; + } + + code = krb5_cc_store_cred(ctx, tmp_cc, &store_creds); + krb5_free_cred_contents(ctx, &store_creds); + if (code != 0) { + krb5_cc_destroy(ctx, tmp_cc); + return code; + } + + /* + * 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_cc_destroy(ctx, tmp_cc); + return code; + } + + code = krb5_principal_set_realm(ctx, self_princ, self_realm); + if (code != 0) { + krb5_free_principal(ctx, self_princ); + krb5_cc_destroy(ctx, tmp_cc); + return code; + } + + code = krb5_get_creds_opt_alloc(ctx, &options); + if (code != 0) { + krb5_free_principal(ctx, self_princ); + krb5_cc_destroy(ctx, tmp_cc); + 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, self_princ); krb5_cc_destroy(ctx, tmp_cc); - if (code != 0) { - 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) { - 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; + 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) { + 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) { + 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: code = krb5_cc_initialize(ctx, store_cc, store_principal); if (code != 0) { krb5_free_cred_contents(ctx, &store_creds); -- cgit From ede3046b8b9b0576a35626026cb28c31b42da46d Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 21 Jun 2011 01:39:58 +0200 Subject: 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 Autobuild-Date: Wed Jun 22 09:10:55 CEST 2011 on sn-devel-104 --- source4/auth/kerberos/kerberos.c | 48 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) (limited to 'source4/auth/kerberos/kerberos.c') diff --git a/source4/auth/kerberos/kerberos.c b/source4/auth/kerberos/kerberos.c index 4acb54fb335..fa8c64b93e0 100644 --- a/source4/auth/kerberos/kerberos.c +++ b/source4/auth/kerberos/kerberos.c @@ -101,6 +101,7 @@ 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 @@ -150,12 +151,22 @@ } code = krb5_cc_store_cred(ctx, tmp_cc, &store_creds); - krb5_free_cred_contents(ctx, &store_creds); if (code != 0) { + krb5_free_cred_contents(ctx, &store_creds); krb5_cc_destroy(ctx, tmp_cc); return code; } + /* + * 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 @@ -165,12 +176,14 @@ 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; } 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; @@ -178,6 +191,7 @@ 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; @@ -187,6 +201,7 @@ 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; @@ -198,6 +213,7 @@ krb5_free_principal(ctx, self_princ); krb5_cc_destroy(ctx, tmp_cc); if (code != 0) { + krb5_free_principal(ctx, blacklist_principal); return code; } @@ -210,6 +226,7 @@ &store_creds); krb5_free_creds(ctx, s4u2self_creds); if (code != 0) { + krb5_free_principal(ctx, blacklist_principal); return code; } @@ -221,6 +238,35 @@ 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:"", + ip?ip:"")); + + SAFE_FREE(sp); + SAFE_FREE(ip); + + 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); -- cgit From b9e095fdfb684005f9bb5c1d943b2a0705308500 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Mon, 20 Jun 2011 20:28:44 +0200 Subject: s4:auth/kerberos: add S4U2Proxy support to kerberos_kinit_password_cc() For S4U2Proxy we need to use the ticket from the S4U2Self stage and ask the kdc for the delegated ticket for the target service. metze --- source4/auth/kerberos/kerberos.c | 134 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 130 insertions(+), 4 deletions(-) (limited to 'source4/auth/kerberos/kerberos.c') diff --git a/source4/auth/kerberos/kerberos.c b/source4/auth/kerberos/kerberos.c index fa8c64b93e0..d32559ed81e 100644 --- a/source4/auth/kerberos/kerberos.c +++ b/source4/auth/kerberos/kerberos.c @@ -81,13 +81,16 @@ The impersonate_principal is the principal if NULL, or the principal to impersonate - The target_service defaults to the krbtgt if NULL, but could be kpasswd/realm or the local service (if we are doing s4u2self) + The self_service, should be the local service (for S4U2Self if impersonate_principal is given). + + The target_service defaults to the krbtgt if NULL, but could be kpasswd/realm or a remote service (for S4U2Proxy) */ 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 *self_service, const char *target_service, krb5_get_init_creds_opt *krb_options, time_t *expire_time, time_t *kdc_time) @@ -96,13 +99,21 @@ krb5_get_creds_opt options; krb5_principal store_principal; krb5_creds store_creds; - const char *self_service = target_service; krb5_creds *s4u2self_creds; + Ticket s4u2self_ticket; + size_t s4u2self_ticketlen; + krb5_creds *s4u2proxy_creds; krb5_principal self_princ; + bool s4u2proxy; + krb5_principal target_princ; krb5_ccache tmp_cc; const char *self_realm; krb5_principal blacklist_principal = NULL; + if (impersonate_principal && self_service == NULL) { + return EINVAL; + } + /* * If we are not impersonating, then get this ticket for the * target service, otherwise a krbtgt, and get the next ticket @@ -167,6 +178,18 @@ store_creds.client = NULL; krb5_free_cred_contents(ctx, &store_creds); + /* + * Check if we also need S4U2Proxy or if S4U2Self is + * enough in order to get a ticket for the target. + */ + if (target_service == NULL) { + s4u2proxy = false; + } else if (strcmp(target_service, self_service) == 0) { + s4u2proxy = false; + } else { + s4u2proxy = true; + } + /* * For S4U2Self we need our own service principal, * which belongs to our own realm (available on @@ -197,6 +220,14 @@ return code; } + if (s4u2proxy) { + /* + * If we want S4U2Proxy, we need the forwardable flag + * on the S4U2Self ticket. + */ + krb5_get_creds_opt_set_options(ctx, options, KRB5_GC_FORWARDABLE); + } + code = krb5_get_creds_opt_set_impersonate(ctx, options, impersonate_principal); if (code != 0) { @@ -211,6 +242,101 @@ self_princ, &s4u2self_creds); krb5_get_creds_opt_free(ctx, options); krb5_free_principal(ctx, self_princ); + if (code != 0) { + krb5_free_principal(ctx, blacklist_principal); + krb5_cc_destroy(ctx, tmp_cc); + return code; + } + + if (!s4u2proxy) { + krb5_cc_destroy(ctx, tmp_cc); + + /* + * 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) { + 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; + goto store; + } + + /* + * We are trying S4U2Proxy: + * + * We need the ticket from the S4U2Self step + * and our TGT in order to get the delegated ticket. + */ + code = decode_Ticket((const uint8_t *)s4u2self_creds->ticket.data, + s4u2self_creds->ticket.length, + &s4u2self_ticket, + &s4u2self_ticketlen); + krb5_free_creds(ctx, s4u2self_creds); + if (code != 0) { + krb5_free_principal(ctx, blacklist_principal); + krb5_cc_destroy(ctx, tmp_cc); + return code; + } + + /* + * For S4U2Proxy we also got a target service principal, + * which also belongs to our own realm (available on + * our client principal). + */ + code = krb5_parse_name(ctx, target_service, &target_princ); + if (code != 0) { + free_Ticket(&s4u2self_ticket); + krb5_free_principal(ctx, blacklist_principal); + krb5_cc_destroy(ctx, tmp_cc); + return code; + } + + code = krb5_principal_set_realm(ctx, target_princ, self_realm); + if (code != 0) { + free_Ticket(&s4u2self_ticket); + krb5_free_principal(ctx, target_princ); + krb5_free_principal(ctx, blacklist_principal); + krb5_cc_destroy(ctx, tmp_cc); + return code; + } + + code = krb5_get_creds_opt_alloc(ctx, &options); + if (code != 0) { + free_Ticket(&s4u2self_ticket); + krb5_free_principal(ctx, target_princ); + krb5_free_principal(ctx, blacklist_principal); + krb5_cc_destroy(ctx, tmp_cc); + return code; + } + + krb5_get_creds_opt_set_options(ctx, options, KRB5_GC_FORWARDABLE); + krb5_get_creds_opt_set_options(ctx, options, KRB5_GC_CONSTRAINED_DELEGATION); + + code = krb5_get_creds_opt_set_ticket(ctx, options, &s4u2self_ticket); + free_Ticket(&s4u2self_ticket); + if (code != 0) { + krb5_get_creds_opt_free(ctx, options); + krb5_free_principal(ctx, target_princ); + krb5_free_principal(ctx, blacklist_principal); + krb5_cc_destroy(ctx, tmp_cc); + return code; + } + + code = krb5_get_creds(ctx, options, tmp_cc, + target_princ, &s4u2proxy_creds); + krb5_get_creds_opt_free(ctx, options); + krb5_free_principal(ctx, target_princ); krb5_cc_destroy(ctx, tmp_cc); if (code != 0) { krb5_free_principal(ctx, blacklist_principal); @@ -222,9 +348,9 @@ * and creds instead of the TGT related stuff * in the krb5_ccache of the caller. */ - code = krb5_copy_creds_contents(ctx, s4u2self_creds, + code = krb5_copy_creds_contents(ctx, s4u2proxy_creds, &store_creds); - krb5_free_creds(ctx, s4u2self_creds); + krb5_free_creds(ctx, s4u2proxy_creds); if (code != 0) { krb5_free_principal(ctx, blacklist_principal); return code; -- cgit From 033f3376a834c1078b377647069b7e30aef59667 Mon Sep 17 00:00:00 2001 From: Stefan Metzmacher Date: Tue, 21 Jun 2011 11:05:15 +0200 Subject: s4:auth/kerberos: protect kerberos_kinit_password_cc() against old KDCs If the KDC does not support S4U2Proxy, it might return a ticket for the TGT client principal. metze --- source4/auth/kerberos/kerberos.c | 49 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) (limited to 'source4/auth/kerberos/kerberos.c') diff --git a/source4/auth/kerberos/kerberos.c b/source4/auth/kerberos/kerberos.c index d32559ed81e..0fc9d143abd 100644 --- a/source4/auth/kerberos/kerberos.c +++ b/source4/auth/kerberos/kerberos.c @@ -109,6 +109,7 @@ krb5_ccache tmp_cc; const char *self_realm; krb5_principal blacklist_principal = NULL; + krb5_principal whitelist_principal = NULL; if (impersonate_principal && self_service == NULL) { return EINVAL; @@ -282,13 +283,23 @@ s4u2self_creds->ticket.length, &s4u2self_ticket, &s4u2self_ticketlen); - krb5_free_creds(ctx, s4u2self_creds); if (code != 0) { + krb5_free_creds(ctx, s4u2self_creds); krb5_free_principal(ctx, blacklist_principal); krb5_cc_destroy(ctx, tmp_cc); return code; } + /* + * we need to remember the client principal of the + * S4U2Self stage and as it needs to match the one we + * will get for the S4U2Proxy stage. We need this + * in order to detect KDCs which does not support S4U2Proxy. + */ + whitelist_principal = s4u2self_creds->client; + s4u2self_creds->client = NULL; + krb5_free_creds(ctx, s4u2self_creds); + /* * For S4U2Proxy we also got a target service principal, * which also belongs to our own realm (available on @@ -297,6 +308,7 @@ code = krb5_parse_name(ctx, target_service, &target_princ); if (code != 0) { free_Ticket(&s4u2self_ticket); + krb5_free_principal(ctx, whitelist_principal); krb5_free_principal(ctx, blacklist_principal); krb5_cc_destroy(ctx, tmp_cc); return code; @@ -306,6 +318,7 @@ if (code != 0) { free_Ticket(&s4u2self_ticket); krb5_free_principal(ctx, target_princ); + krb5_free_principal(ctx, whitelist_principal); krb5_free_principal(ctx, blacklist_principal); krb5_cc_destroy(ctx, tmp_cc); return code; @@ -315,6 +328,7 @@ if (code != 0) { free_Ticket(&s4u2self_ticket); krb5_free_principal(ctx, target_princ); + krb5_free_principal(ctx, whitelist_principal); krb5_free_principal(ctx, blacklist_principal); krb5_cc_destroy(ctx, tmp_cc); return code; @@ -328,6 +342,7 @@ if (code != 0) { krb5_get_creds_opt_free(ctx, options); krb5_free_principal(ctx, target_princ); + krb5_free_principal(ctx, whitelist_principal); krb5_free_principal(ctx, blacklist_principal); krb5_cc_destroy(ctx, tmp_cc); return code; @@ -339,6 +354,7 @@ krb5_free_principal(ctx, target_princ); krb5_cc_destroy(ctx, tmp_cc); if (code != 0) { + krb5_free_principal(ctx, whitelist_principal); krb5_free_principal(ctx, blacklist_principal); return code; } @@ -352,6 +368,7 @@ &store_creds); krb5_free_creds(ctx, s4u2proxy_creds); if (code != 0) { + krb5_free_principal(ctx, whitelist_principal); krb5_free_principal(ctx, blacklist_principal); return code; } @@ -385,6 +402,7 @@ SAFE_FREE(sp); SAFE_FREE(ip); + krb5_free_principal(ctx, whitelist_principal); krb5_free_principal(ctx, blacklist_principal); krb5_free_cred_contents(ctx, &store_creds); return KRB5_FWD_BAD_PRINCIPAL; @@ -393,6 +411,35 @@ krb5_free_principal(ctx, blacklist_principal); } + if (whitelist_principal && + !krb5_principal_compare(ctx, store_creds.client, whitelist_principal)) { + char *sp = NULL; + char *ep = NULL; + + code = krb5_unparse_name(ctx, store_creds.client, &sp); + if (code != 0) { + sp = NULL; + } + code = krb5_unparse_name(ctx, whitelist_principal, &ep); + if (code != 0) { + ep = NULL; + } + DEBUG(1, ("kerberos_kinit_password_cc: " + "KDC returned wrong principal[%s] we expected [%s]\n", + sp?sp:"", + ep?ep:"")); + + SAFE_FREE(sp); + SAFE_FREE(ep); + + krb5_free_principal(ctx, whitelist_principal); + krb5_free_cred_contents(ctx, &store_creds); + return KRB5_FWD_BAD_PRINCIPAL; + } + if (whitelist_principal) { + krb5_free_principal(ctx, whitelist_principal); + } + code = krb5_cc_initialize(ctx, store_cc, store_principal); if (code != 0) { krb5_free_cred_contents(ctx, &store_creds); -- cgit