[SSSD] [PATCH] Run the ldap_child process as setuid if sssd_be is running as unprivileged user

Michal Židek mzidek at redhat.com
Fri Oct 31 19:15:13 UTC 2014


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 at 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



More information about the sssd-devel mailing list