On 10/29/2014 11:03 AM, Pavel Reichl wrote:
Just code style nitpicks, feel free to ignore.
On 10/27/2014 04:17 PM, Jakub Hrozek wrote:
> 0002-LDAP-Move-sss_krb5_verify_keytab_ex-to-ldap_child.patch
>
>
> From 154a897b09136c4499fc024c9110096d34d2b6d5 Mon Sep 17 00:00:00 2001
> From: Jakub Hrozek<jhrozek(a)redhat.com>
> Date: Sat, 11 Oct 2014 17:39:21 +0200
> Subject: [PATCH 2/4] LDAP: Move sss_krb5_verify_keytab_ex to ldap_child
>
> The function was called from one place only, so it makes no sense to
> keep it in a shared module. Moreover, the function should only be
> called from code that runs as root.
> ---
> src/providers/ldap/ldap_child.c | 80 ++++++++++++++++++++++++++++++++++++++++-
> src/util/sss_krb5.c | 76 ---------------------------------------
> src/util/sss_krb5.h | 3 --
> 3 files changed, 79 insertions(+), 80 deletions(-)
>
> diff --git a/src/providers/ldap/ldap_child.c b/src/providers/ldap/ldap_child.c
> index
e5779b70906d90ab855677f04a154e179f2163c6..faa6f027ba52f55985c63e8055292e616b5bb6bf 100644
> --- a/src/providers/ldap/ldap_child.c
> +++ b/src/providers/ldap/ldap_child.c
> @@ -160,6 +160,84 @@ set_child_debugging(krb5_context ctx)
> return EOK;
> }
>
> +static int lc_verify_keytab_ex(const char *principal,
> + const char *keytab_name,
> + krb5_context context,
> + krb5_keytab keytab)
> +{
> + bool found;
> + char *kt_principal;
> + krb5_error_code krberr;
> + krb5_kt_cursor cursor;
> + krb5_keytab_entry entry;
> +
> + krberr = krb5_kt_start_seq_get(context, keytab, &cursor);
> + if (krberr) {
> + DEBUG(SSSDBG_FATAL_FAILURE,
> + "Cannot read keytab [%s].\n", KEYTAB_CLEAN_NAME);
> +
> + sss_log(SSS_LOG_ERR, "Error reading keytab file [%s]: [%d][%s]. "
> + "Unable to create GSSAPI-encrypted LDAP "
> + "connection.",
> + KEYTAB_CLEAN_NAME, krberr,
> + sss_krb5_get_error_message(context, krberr));
> +
> + return EIO;
> + }
> +
> + found = false;
> + while((krb5_kt_next_entry(context, keytab, &entry, &cursor)) == 0){
missing space after while and before {
> + krberr = krb5_unparse_name(context, entry.principal, &kt_principal);
> + if (krberr) {
> + DEBUG(SSSDBG_FATAL_FAILURE,
> + "Could not parse keytab entry\n");
> + sss_log(SSS_LOG_ERR, "Could not parse keytab entry\n");
> + return EIO;
> + }
> +
> + if (strcmp(principal, kt_principal) == 0) {
> + found = true;
> + }
> + free(kt_principal);
> + krberr = sss_krb5_free_keytab_entry_contents(context, &entry);
> + if (krberr) {
> + /* This should never happen. The API docs for this function
> + * specify only success for this function
> + */
> + DEBUG(SSSDBG_CRIT_FAILURE,"Could not free keytab entry
contents\n");
> + /* This is non-fatal, so we'll continue here */
> + }
> +
> + if (found) {
> + break;
> + }
> + }
> +
> + krberr = krb5_kt_end_seq_get(context, keytab, &cursor);
> + if (krberr) {
> + DEBUG(SSSDBG_FATAL_FAILURE, "Could not close keytab.\n");
> + sss_log(SSS_LOG_ERR, "Could not close keytab file [%s].",
> + KEYTAB_CLEAN_NAME);
> + return EIO;
> + }
> +
> + if (!found) {
> + DEBUG(SSSDBG_FATAL_FAILURE,
> + "Principal [%s] not found in keytab [%s]\n",
> + principal,
> + KEYTAB_CLEAN_NAME);
> + sss_log(SSS_LOG_ERR, "Error processing keytab file [%s]: "
> + "Principal [%s] was not found. "
> + "Unable to create GSSAPI-encrypted LDAP
connection.",
> + KEYTAB_CLEAN_NAME, principal);
> +
> + return EFAULT;
> + }
> +
> + return EOK;
> +}
> +
> +
One empty line too many.
These patches look good to me. If the nitpicks that Pavel found
are fixed, I'll ack.
Michal