URL: https://github.com/freeipa/freeipa/pull/3842 Author: frozencemetery Title: #3842: [KDB] various code hygiene fixes Action: opened
PR body: """ """ To pull the PR as Git branch: git remote add ghfreeipa https://github.com/freeipa/freeipa git fetch ghfreeipa pull/3842/head:pr3842 git checkout pr3842
From f21f7d379f56fa97baec64ea94ba2754424c46d3 Mon Sep 17 00:00:00 2001 From: Robbie Harwood <rharw...@redhat.com> Date: Wed, 31 Jul 2019 18:20:34 -0400 Subject: [PATCH 1/6] [KDB] Make the coding style explicit Signed-off-by: Robbie Harwood <rharw...@redhat.com> --- daemons/ipa-kdb/README | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/daemons/ipa-kdb/README b/daemons/ipa-kdb/README index b0786853bd..4075082ee2 100644 --- a/daemons/ipa-kdb/README +++ b/daemons/ipa-kdb/README @@ -1 +1,19 @@ This is the ipa krb5kdc database backend. + +As the KDB interfaces heavily with krb5, we inherit its code style as well. +However, note the following changes: + +- no modelines (and different file preamble) +- return types don't require their own line +- single-statement blocks may optionally be braced +- /* and */ do not ever get their own line +- C99 for-loops are permitted (and encouraged) +- a restricted set of other C99 features are permitted + +In particular, variable-length arrays, flexible array members, compound +literals, universal character names, and //-style comments are not permitted. + +Use of regular malloc/free is preferred over talloc for new code. + +By and large, existing code mostly conforms to these requirements. New code +must conform to them. From 23bdcbf9dd27a1745b1754adf603748efd7878f0 Mon Sep 17 00:00:00 2001 From: Robbie Harwood <rharw...@redhat.com> Date: Fri, 2 Aug 2019 15:59:12 -0400 Subject: [PATCH 2/6] [KDB] Unify copyright block appearance Signed-off-by: Robbie Harwood <rharw...@redhat.com> --- daemons/ipa-kdb/ipa_kdb_certauth.c | 18 ++++++------ daemons/ipa-kdb/ipa_kdb_mspac_private.h | 5 ++-- daemons/ipa-kdb/tests/ipa_kdb_tests.c | 39 ++++++++++++------------- 3 files changed, 31 insertions(+), 31 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_certauth.c b/daemons/ipa-kdb/ipa_kdb_certauth.c index 47911aa3de..c884254c79 100644 --- a/daemons/ipa-kdb/ipa_kdb_certauth.c +++ b/daemons/ipa-kdb/ipa_kdb_certauth.c @@ -1,7 +1,13 @@ -/** BEGIN COPYRIGHT BLOCK +/* + * ipa-kdb certauth plugin to krb5 + * + * Authors: Sumit Bose <sb...@redhat.com> + * + * Copyright (C) 2017 Red Hat + * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or + * the Free Software Foundation; either version 3 of the License, or * (at your option) any later version. * * This program is distributed in the hope that it will be useful, @@ -29,13 +35,7 @@ * work to be covered by the GPL. Only the copyright holders of this * Program may make changes or additions to the list of Approved * Interfaces. - * - * Authors: - * Sumit Bose <sb...@redhat.com> - * - * Copyright (C) 2017 Red Hat, Inc. - * All rights reserved. - * END COPYRIGHT BLOCK **/ + */ #include <errno.h> //#include <krb5/certauth_plugin.h> diff --git a/daemons/ipa-kdb/ipa_kdb_mspac_private.h b/daemons/ipa-kdb/ipa_kdb_mspac_private.h index 30382d2eef..9e0622ef4d 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac_private.h +++ b/daemons/ipa-kdb/ipa_kdb_mspac_private.h @@ -1,7 +1,8 @@ /* * MIT Kerberos KDC database backend for FreeIPA - * This head file contains private declarations for ipa_kdb_mspac.c and should - * be used only there or in unit-test. + * + * This header file contains private declarations for ipa_kdb_mspac.c and + * should be used only there or in unit-test. * * Authors: Sumit Bose <sb...@redhat.com> * diff --git a/daemons/ipa-kdb/tests/ipa_kdb_tests.c b/daemons/ipa-kdb/tests/ipa_kdb_tests.c index 7f1ae7f66e..e9c1343b9b 100644 --- a/daemons/ipa-kdb/tests/ipa_kdb_tests.c +++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c @@ -1,24 +1,23 @@ /* - Authors: - Sumit Bose <sb...@redhat.com> - - Copyright (C) 2015 Red Hat - - ipa-kdb tests - - This program is free software; you can redistribute it and/or modify - it under the terms of the GNU General Public License as published by - the Free Software Foundation; either version 3 of the License, or - (at your option) any later version. - - This program is distributed in the hope that it will be useful, - but WITHOUT ANY WARRANTY; without even the implied warranty of - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - GNU General Public License for more details. - - You should have received a copy of the GNU General Public License - along with this program. If not, see <http://www.gnu.org/licenses/>. -*/ + * ipa-kdb tests + * + * Authors: Sumit Bose <sb...@redhat.com> + * + * Copyright (C) 2015 Red Hat + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ #include <errno.h> #include <stdarg.h> From 06eb1ed4f3c65f8250e7f47b4625ed9e3017eb1a Mon Sep 17 00:00:00 2001 From: Robbie Harwood <rharw...@redhat.com> Date: Mon, 5 Aug 2019 15:39:34 -0400 Subject: [PATCH 3/6] [KDB] Put macros and struct definitions near file starts Signed-off-by: Robbie Harwood <rharw...@redhat.com> --- daemons/ipa-kdb/ipa_kdb.c | 46 ++++++++++++++-------------- daemons/ipa-kdb/ipa_kdb_common.c | 16 +++++----- daemons/ipa-kdb/ipa_kdb_principals.c | 34 ++++++++++---------- 3 files changed, 48 insertions(+), 48 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c index 612857b389..15a3269c20 100644 --- a/daemons/ipa-kdb/ipa_kdb.c +++ b/daemons/ipa-kdb/ipa_kdb.c @@ -28,6 +28,29 @@ #define IPADB_GLOBAL_CONFIG_CACHE_TIME 60 +#define LDAPI_URI_PREFIX "ldapi://" +#define LDAPI_PATH_PREFIX "%2fslapd-" +#define SOCKET_SUFFIX ".socket" +#define APPEND_PATH_PART(pos, part) \ + do { \ + int partlen = strlen(part); \ + strncpy(pos, part, partlen + 1); \ + p += partlen; \ + } while (0) + +static const struct { + const char *name; + enum ipadb_user_auth flag; +} userauth_table[] = { + { "disabled", IPADB_USER_AUTH_DISABLED }, + { "password", IPADB_USER_AUTH_PASSWORD }, + { "radius", IPADB_USER_AUTH_RADIUS }, + { "otp", IPADB_USER_AUTH_OTP }, + { "pkinit", IPADB_USER_AUTH_PKINIT }, + { "hardened", IPADB_USER_AUTH_HARDENED }, + { } +}; + struct ipadb_context *ipadb_get_context(krb5_context kcontext) { void *db_ctx; @@ -77,16 +100,6 @@ static void ipadb_context_free(krb5_context kcontext, } } -#define LDAPI_URI_PREFIX "ldapi://" -#define LDAPI_PATH_PREFIX "%2fslapd-" -#define SOCKET_SUFFIX ".socket" -#define APPEND_PATH_PART(pos, part) \ - do { \ - int partlen = strlen(part); \ - strncpy(pos, part, partlen + 1); \ - p += partlen; \ - } while (0) - static char *ipadb_realm_to_ldapi_uri(char *realm) { char *uri = NULL; @@ -186,19 +199,6 @@ static char *ipadb_get_base_from_realm(krb5_context kcontext) return base; } -static const struct { - const char *name; - enum ipadb_user_auth flag; -} userauth_table[] = { - { "disabled", IPADB_USER_AUTH_DISABLED }, - { "password", IPADB_USER_AUTH_PASSWORD }, - { "radius", IPADB_USER_AUTH_RADIUS }, - { "otp", IPADB_USER_AUTH_OTP }, - { "pkinit", IPADB_USER_AUTH_PKINIT }, - { "hardened", IPADB_USER_AUTH_HARDENED }, - { } -}; - void ipadb_parse_user_auth(LDAP *lcontext, LDAPMessage *le, enum ipadb_user_auth *userauth) { diff --git a/daemons/ipa-kdb/ipa_kdb_common.c b/daemons/ipa-kdb/ipa_kdb_common.c index d86f4f5307..ccd1735757 100644 --- a/daemons/ipa-kdb/ipa_kdb_common.c +++ b/daemons/ipa-kdb/ipa_kdb_common.c @@ -25,6 +25,14 @@ static struct timeval std_timeout = {30, 0}; +struct ipadb_multires { + LDAP *lcontext; + LDAPMessage **res; + LDAPMessage *next; + ssize_t cursor; + ssize_t count; +}; + char *ipadb_filter_escape(const char *input, bool star) { char *output; @@ -611,14 +619,6 @@ int ipadb_ldap_deref_results(LDAP *lcontext, LDAPMessage *le, return ret; } -struct ipadb_multires { - LDAP *lcontext; - LDAPMessage **res; - LDAPMessage *next; - ssize_t cursor; - ssize_t count; -}; - krb5_error_code ipadb_multires_init(LDAP *lcontext, struct ipadb_multires **r) { *r = malloc(sizeof(struct ipadb_multires)); diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index b208d090a3..4804e6fe7b 100644 --- a/daemons/ipa-kdb/ipa_kdb_principals.c +++ b/daemons/ipa-kdb/ipa_kdb_principals.c @@ -108,6 +108,23 @@ static char *std_principal_obj_classes[] = { #define DEFAULT_TL_DATA_CONTENT "\x00\x00\x00\x00principal@UNINITIALIZED" +#define OSA_ADB_PRINC_VERSION_1 0x12345C01 +/* The XDR encoding of OSA_PRINC_ENC is as follows: + * version: int (signed 32 bit integer) + * name: nullstring (null terminated variable string) + * aux_attributes: long (signed 32 bit integer) + * old_key_next: u_int (unsigned 32 bit integer) + * adm_hist_kvno: u_char (unisgned char) + * old_keys: array of keys, we do not care so always u_int of 0 + */ +#define OSA_PRINC_ENC_BASE_SIZE 20 + +struct ipadb_mods { + LDAPMod **mods; + int alloc_size; + int tip; +}; + static int ipadb_ldap_attr_to_tl_data(LDAP *lcontext, LDAPMessage *le, char *attrname, krb5_tl_data **result, int *num) @@ -350,17 +367,6 @@ static enum ipadb_user_auth ipadb_get_user_auth(struct ipadb_context *ipactx, return ua; } -#define OSA_ADB_PRINC_VERSION_1 0x12345C01 -/* The XDR encoding of OSA_PRINC_ENC is as follows: - version: int (signed 32 bit integer) - name: nullstring (null terminated variable string) - aux_attributes: long (signed 32 bit integer) - old_key_next: u_int (unsigned 32 bit integer) - adm_hist_kvno: u_char (unisgned char) - old_keys: array of keys, we do not care so alway u_int of 0 -*/ -#define OSA_PRINC_ENC_BASE_SIZE 20 - static krb5_error_code ipadb_policydn_to_kdam_tl_data(const char *policydn, krb5_db_entry *entry) { @@ -1385,12 +1391,6 @@ static krb5_error_code ipadb_get_tl_data(krb5_db_entry *entry, return 0; } -struct ipadb_mods { - LDAPMod **mods; - int alloc_size; - int tip; -}; - static int new_ipadb_mods(struct ipadb_mods **imods) { struct ipadb_mods *r; From 2cb22aee2f0e9877a294eed5385fb42b1077db25 Mon Sep 17 00:00:00 2001 From: Robbie Harwood <rharw...@redhat.com> Date: Mon, 5 Aug 2019 16:25:51 -0400 Subject: [PATCH 4/6] [KDB] Fix incorrect and potentially misleading indentations Signed-off-by: Robbie Harwood <rharw...@redhat.com> --- daemons/ipa-kdb/ipa_kdb.c | 2 +- daemons/ipa-kdb/ipa_kdb.h | 24 +-- daemons/ipa-kdb/ipa_kdb_certauth.c | 15 +- daemons/ipa-kdb/ipa_kdb_common.c | 3 +- daemons/ipa-kdb/ipa_kdb_mspac.c | 267 +++++++++++++------------- daemons/ipa-kdb/ipa_kdb_passwords.c | 4 +- daemons/ipa-kdb/tests/ipa_kdb_tests.c | 4 +- 7 files changed, 157 insertions(+), 162 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c index 15a3269c20..898f40beae 100644 --- a/daemons/ipa-kdb/ipa_kdb.c +++ b/daemons/ipa-kdb/ipa_kdb.c @@ -280,7 +280,7 @@ static int ipadb_load_global_config(struct ipadb_context *ipactx) } } - /* Load authz data. */ + /* Load authz data. */ ret = ipadb_ldap_attr_to_strlist(ipactx->lcontext, first, IPA_KRB_AUTHZ_DATA_ATTR, &authz_data_list); if (ret == 0) { diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h index 08c24ae64d..3e1eba1c52 100644 --- a/daemons/ipa-kdb/ipa_kdb.h +++ b/daemons/ipa-kdb/ipa_kdb.h @@ -85,21 +85,21 @@ struct ipadb_mspac; enum ipadb_user_auth { - IPADB_USER_AUTH_NONE = 0, - IPADB_USER_AUTH_DISABLED = 1 << 0, - IPADB_USER_AUTH_PASSWORD = 1 << 1, - IPADB_USER_AUTH_RADIUS = 1 << 2, - IPADB_USER_AUTH_OTP = 1 << 3, - IPADB_USER_AUTH_PKINIT = 1 << 4, - IPADB_USER_AUTH_HARDENED = 1 << 5, + IPADB_USER_AUTH_NONE = 0, + IPADB_USER_AUTH_DISABLED = 1 << 0, + IPADB_USER_AUTH_PASSWORD = 1 << 1, + IPADB_USER_AUTH_RADIUS = 1 << 2, + IPADB_USER_AUTH_OTP = 1 << 3, + IPADB_USER_AUTH_PKINIT = 1 << 4, + IPADB_USER_AUTH_HARDENED = 1 << 5, }; struct ipadb_global_config { - time_t last_update; - bool disable_last_success; - bool disable_lockout; - char **authz_data; - enum ipadb_user_auth user_auth; + time_t last_update; + bool disable_last_success; + bool disable_lockout; + char **authz_data; + enum ipadb_user_auth user_auth; bool disable_preauth_for_spns; }; diff --git a/daemons/ipa-kdb/ipa_kdb_certauth.c b/daemons/ipa-kdb/ipa_kdb_certauth.c index c884254c79..4babf83229 100644 --- a/daemons/ipa-kdb/ipa_kdb_certauth.c +++ b/daemons/ipa-kdb/ipa_kdb_certauth.c @@ -178,7 +178,7 @@ static krb5_error_code ipa_get_init_data(krb5_context kcontext, lc = ipactx->lcontext; for (le = ldap_first_entry(lc, result); le; - le = ldap_next_entry(lc, le)) { + le = ldap_next_entry(lc, le)) { prio = SSS_CERTMAP_MIN_PRIO; ret = ipadb_ldap_attr_to_uint32(lc, le, IPA_CERTMAP_PRIORITY, &prio); @@ -226,7 +226,7 @@ static krb5_error_code ipa_get_init_data(krb5_context kcontext, sss_certmap_free_ctx(ipactx->certauth_moddata->sss_certmap_ctx); ipactx->certauth_moddata->sss_certmap_ctx = ctx; ipactx->certauth_moddata->valid_until = time(NULL) - + DEFAULT_CERTMAP_LIFETIME; + + DEFAULT_CERTMAP_LIFETIME; krb5_klog_syslog(LOG_DEBUG, "Successfully updates certificate mapping rules."); @@ -306,10 +306,9 @@ static krb5_error_code ipa_certauth_authorize(krb5_context context, if (domains != NULL) { if (moddata->local_domain == NULL) { - /* We don't know our own domain name, in general this should not - * happen. But to be fault tolerant we allow matching rule which - * do not have a domain assigned. */ - + /* We don't know our own domain name. In general this should not + * happen, but to be fault tolerant we allow matching a rule which + * do not have a domain assigned. */ ret = KRB5KDC_ERR_CERTIFICATE_MISMATCH; goto done; } @@ -418,8 +417,8 @@ static void ipa_certauth_free_indicator(krb5_context context, krb5_error_code certauth_ipakdb_initvt(krb5_context context, - int maj_ver, int min_ver, - krb5_plugin_vtable vtable) + int maj_ver, int min_ver, + krb5_plugin_vtable vtable) { krb5_certauth_vtable vt; diff --git a/daemons/ipa-kdb/ipa_kdb_common.c b/daemons/ipa-kdb/ipa_kdb_common.c index ccd1735757..d8b5a1ef09 100644 --- a/daemons/ipa-kdb/ipa_kdb_common.c +++ b/daemons/ipa-kdb/ipa_kdb_common.c @@ -339,8 +339,7 @@ krb5_error_code ipadb_deref_search(struct ipadb_context *ipactx, goto done; } - ret = ldap_control_create(LDAP_CONTROL_X_DEREF, - 1, &derefval, 1, &ctrl[0]); + ret = ldap_control_create(LDAP_CONTROL_X_DEREF, 1, &derefval, 1, &ctrl[0]); if (ret != LDAP_SUCCESS) { kerr = ENOMEM; goto done; diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index 615a23e95b..4723259900 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -164,9 +164,8 @@ char *dom_sid_string(TALLOC_CTX *memctx, const struct dom_sid *dom_sid) uint32_t ia; char *buf; - if (dom_sid == NULL - || dom_sid->num_auths < 0 - || dom_sid->num_auths > SID_SUB_AUTHS) { + if (dom_sid == NULL || dom_sid->num_auths < 0 || + dom_sid->num_auths > SID_SUB_AUTHS) { return NULL; } @@ -177,17 +176,15 @@ char *dom_sid_string(TALLOC_CTX *memctx, const struct dom_sid *dom_sid) return NULL; } - ia = (dom_sid->id_auth[5]) + - (dom_sid->id_auth[4] << 8 ) + - (dom_sid->id_auth[3] << 16) + - (dom_sid->id_auth[2] << 24); + ia = (dom_sid->id_auth[5]) + (dom_sid->id_auth[4] << 8 ) + + (dom_sid->id_auth[3] << 16) + (dom_sid->id_auth[2] << 24); - ofs = snprintf(buf, len, "S-%u-%lu", (unsigned int) dom_sid->sid_rev_num, - (unsigned long) ia); + ofs = snprintf(buf, len, "S-%u-%lu", (unsigned int)dom_sid->sid_rev_num, + (unsigned long)ia); for (c = 0; c < dom_sid->num_auths; c++) { ofs += snprintf(buf + ofs, MAX(len - ofs, 0), "-%lu", - (unsigned long) dom_sid->sub_auths[c]); + (unsigned long)dom_sid->sub_auths[c]); } if (ofs >= len) { @@ -942,9 +939,8 @@ static int get_user_and_group_sids(TALLOC_CTX *memctx, } group_sids = talloc_array(memctx, char *, - 3 + - logon_info->info->info3.base.groups.count + - logon_info->info->info3.sidcount); + 3 + logon_info->info->info3.base.groups.count + + logon_info->info->info3.sidcount); if (group_sids == NULL) { krb5_klog_syslog(LOG_ERR, "talloc_array failed"); ret = ENOMEM; @@ -1017,8 +1013,9 @@ static int add_groups(TALLOC_CTX *memctx, } sids = talloc_realloc(memctx, logon_info->info->info3.sids, - struct netr_SidAttr, - logon_info->info->info3.sidcount + ipa_group_sids_count); + struct netr_SidAttr, + logon_info->info->info3.sidcount + + ipa_group_sids_count); if (sids == NULL) { return ENOMEM; } @@ -2803,63 +2800,63 @@ krb5_error_code ipadb_check_transited_realms(krb5_context kcontext, const krb5_data *client_realm, const krb5_data *server_realm) { - struct ipadb_context *ipactx; - bool has_transited_contents, has_client_realm, has_server_realm; - int i; - krb5_error_code ret; + struct ipadb_context *ipactx; + bool has_transited_contents, has_client_realm, has_server_realm; + int i; + krb5_error_code ret; - ipactx = ipadb_get_context(kcontext); - if (!ipactx || !ipactx->mspac) { - return KRB5_KDB_DBNOTINITED; + ipactx = ipadb_get_context(kcontext); + if (!ipactx || !ipactx->mspac) { + return KRB5_KDB_DBNOTINITED; + } + + has_transited_contents = false; + has_client_realm = false; + has_server_realm = false; + + /* First, compare client or server realm with ours */ + if (strncasecmp(client_realm->data, ipactx->realm, client_realm->length) == 0) { + has_client_realm = true; + } + if (strncasecmp(server_realm->data, ipactx->realm, server_realm->length) == 0) { + has_server_realm = true; + } + + if ((tr_contents->length == 0) || (tr_contents->data[0] == '\0')) { + /* For in-realm case allow transition */ + if (has_client_realm && has_server_realm) { + return 0; } + /* Since transited realm is empty, we don't need to check for it, it is a direct trust case */ + has_transited_contents = true; + } + + if (!ipactx->mspac || !ipactx->mspac->trusts) { + return KRB5_PLUGIN_NO_HANDLE; + } - has_transited_contents = false; - has_client_realm = false; - has_server_realm = false; - - /* First, compare client or server realm with ours */ - if (strncasecmp(client_realm->data, ipactx->realm, client_realm->length) == 0) { - has_client_realm = true; - } - if (strncasecmp(server_realm->data, ipactx->realm, server_realm->length) == 0) { - has_server_realm = true; - } - - if ((tr_contents->length == 0) || (tr_contents->data[0] == '\0')) { - /* For in-realm case allow transition */ - if (has_client_realm && has_server_realm) { - return 0; - } - /* Since transited realm is empty, we don't need to check for it, it is a direct trust case */ - has_transited_contents = true; - } - - if (!ipactx->mspac || !ipactx->mspac->trusts) { - return KRB5_PLUGIN_NO_HANDLE; - } - - /* Iterate through list of trusts and check if any of input belongs to any of the trust */ - for(i=0; i < ipactx->mspac->num_trusts ; i++) { - if (!has_transited_contents && - (strncasecmp(tr_contents->data, ipactx->mspac->trusts[i].domain_name, tr_contents->length) == 0)) { - has_transited_contents = true; - } - if (!has_client_realm && - (strncasecmp(client_realm->data, ipactx->mspac->trusts[i].domain_name, client_realm->length) == 0)) { - has_client_realm = true; - } - if (!has_server_realm && - (strncasecmp(server_realm->data, ipactx->mspac->trusts[i].domain_name, server_realm->length) == 0)) { - has_server_realm = true; - } - } - - /* Tell to KDC that we don't handle this transition so that rules in krb5.conf could play its role */ - ret = KRB5_PLUGIN_NO_HANDLE; - if (has_client_realm && has_transited_contents && has_server_realm) { - ret = 0; - } - return ret; + /* Iterate through list of trusts and check if any of input belongs to any of the trust */ + for(i=0; i < ipactx->mspac->num_trusts ; i++) { + if (!has_transited_contents && + (strncasecmp(tr_contents->data, ipactx->mspac->trusts[i].domain_name, tr_contents->length) == 0)) { + has_transited_contents = true; + } + if (!has_client_realm && + (strncasecmp(client_realm->data, ipactx->mspac->trusts[i].domain_name, client_realm->length) == 0)) { + has_client_realm = true; + } + if (!has_server_realm && + (strncasecmp(server_realm->data, ipactx->mspac->trusts[i].domain_name, server_realm->length) == 0)) { + has_server_realm = true; + } + } + + /* Tell to KDC that we don't handle this transition so that rules in krb5.conf could play its role */ + ret = KRB5_PLUGIN_NO_HANDLE; + if (has_client_realm && has_transited_contents && has_server_realm) { + ret = 0; + } + return ret; } /* Checks whether a principal's realm is one of trusted domains' realm or NetBIOS name @@ -2870,70 +2867,70 @@ krb5_error_code ipadb_is_princ_from_trusted_realm(krb5_context kcontext, const char *test_realm, size_t size, char **trusted_realm) { - struct ipadb_context *ipactx; - int i, j, length; - const char *name; - bool result = false; - - if (test_realm == NULL || test_realm[0] == '\0') { - return KRB5_KDB_NOENTRY; - } - - ipactx = ipadb_get_context(kcontext); - if (!ipactx || !ipactx->mspac) { - return KRB5_KDB_DBNOTINITED; - } - - /* First, compare realm with ours, it would not be from a trusted realm then */ - if (strncasecmp(test_realm, ipactx->realm, size) == 0) { - return KRB5_KDB_NOENTRY; - } - - if (!ipactx->mspac || !ipactx->mspac->trusts) { - return KRB5_KDB_NOENTRY; - } - - /* Iterate through list of trusts and check if input realm belongs to any of the trust */ - for(i = 0 ; i < ipactx->mspac->num_trusts ; i++) { - result = strncasecmp(test_realm, - ipactx->mspac->trusts[i].domain_name, - size) == 0; - - if (!result && (ipactx->mspac->trusts[i].flat_name != NULL)) { - result = strncasecmp(test_realm, - ipactx->mspac->trusts[i].flat_name, - size) == 0; - } - - if (!result && (ipactx->mspac->trusts[i].upn_suffixes != NULL)) { - for (j = 0; ipactx->mspac->trusts[i].upn_suffixes[j]; j++) { - result = strncasecmp(test_realm, - ipactx->mspac->trusts[i].upn_suffixes[j], - size) == 0; - if (result) - break; - } - } - - if (result) { - /* return the realm if caller supplied a place for it */ - if (trusted_realm != NULL) { - name = (ipactx->mspac->trusts[i].parent_name != NULL) ? - ipactx->mspac->trusts[i].parent_name : - ipactx->mspac->trusts[i].domain_name; - length = strlen(name) + 1; - *trusted_realm = calloc(1, length); - if (*trusted_realm != NULL) { - for (j = 0; j < length; j++) { - (*trusted_realm)[j] = toupper(name[j]); - } - } else { - return KRB5_KDB_NOENTRY; - } - } - return 0; - } - } - - return KRB5_KDB_NOENTRY; + struct ipadb_context *ipactx; + int i, j, length; + const char *name; + bool result = false; + + if (test_realm == NULL || test_realm[0] == '\0') { + return KRB5_KDB_NOENTRY; + } + + ipactx = ipadb_get_context(kcontext); + if (!ipactx || !ipactx->mspac) { + return KRB5_KDB_DBNOTINITED; + } + + /* First, compare realm with ours, it would not be from a trusted realm then */ + if (strncasecmp(test_realm, ipactx->realm, size) == 0) { + return KRB5_KDB_NOENTRY; + } + + if (!ipactx->mspac || !ipactx->mspac->trusts) { + return KRB5_KDB_NOENTRY; + } + + /* Iterate through list of trusts and check if input realm belongs to any of the trust */ + for(i = 0 ; i < ipactx->mspac->num_trusts ; i++) { + result = strncasecmp(test_realm, + ipactx->mspac->trusts[i].domain_name, + size) == 0; + + if (!result && (ipactx->mspac->trusts[i].flat_name != NULL)) { + result = strncasecmp(test_realm, + ipactx->mspac->trusts[i].flat_name, + size) == 0; + } + + if (!result && (ipactx->mspac->trusts[i].upn_suffixes != NULL)) { + for (j = 0; ipactx->mspac->trusts[i].upn_suffixes[j]; j++) { + result = strncasecmp(test_realm, + ipactx->mspac->trusts[i].upn_suffixes[j], + size) == 0; + if (result) + break; + } + } + + if (result) { + /* return the realm if caller supplied a place for it */ + if (trusted_realm != NULL) { + name = (ipactx->mspac->trusts[i].parent_name != NULL) ? + ipactx->mspac->trusts[i].parent_name : + ipactx->mspac->trusts[i].domain_name; + length = strlen(name) + 1; + *trusted_realm = calloc(1, length); + if (*trusted_realm != NULL) { + for (j = 0; j < length; j++) { + (*trusted_realm)[j] = toupper(name[j]); + } + } else { + return KRB5_KDB_NOENTRY; + } + } + return 0; + } + } + + return KRB5_KDB_NOENTRY; } diff --git a/daemons/ipa-kdb/ipa_kdb_passwords.c b/daemons/ipa-kdb/ipa_kdb_passwords.c index a3d4fe2436..1862c10374 100644 --- a/daemons/ipa-kdb/ipa_kdb_passwords.c +++ b/daemons/ipa-kdb/ipa_kdb_passwords.c @@ -178,8 +178,8 @@ krb5_error_code ipadb_change_pwd(krb5_context context, /* We further filter supported enctypes to restrict to the list * we have in ldap */ kerr = filter_key_salt_tuples(context, ks_tuple, ks_tuple_count, - ipactx->supp_encs, ipactx->n_supp_encs, - &fks, &n_fks); + ipactx->supp_encs, ipactx->n_supp_encs, + &fks, &n_fks); if (kerr) { return kerr; } diff --git a/daemons/ipa-kdb/tests/ipa_kdb_tests.c b/daemons/ipa-kdb/tests/ipa_kdb_tests.c index e9c1343b9b..7ebcb9b965 100644 --- a/daemons/ipa-kdb/tests/ipa_kdb_tests.c +++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c @@ -118,8 +118,8 @@ static int setup(void **state) ipa_ctx->mspac->trusts[0].len_sid_blacklist_incoming = 1; ipa_ctx->mspac->trusts[0].sid_blacklist_incoming = calloc( - ipa_ctx->mspac->trusts[0].len_sid_blacklist_incoming, - sizeof(struct dom_sid)); + ipa_ctx->mspac->trusts[0].len_sid_blacklist_incoming, + sizeof(struct dom_sid)); assert_non_null(ipa_ctx->mspac->trusts[0].sid_blacklist_incoming); ret = string_to_sid(BLACKLIST_SID, &ipa_ctx->mspac->trusts[0].sid_blacklist_incoming[0]); From fc177678456835b37fe4b906c4811e298d2c803c Mon Sep 17 00:00:00 2001 From: Robbie Harwood <rharw...@redhat.com> Date: Mon, 5 Aug 2019 17:47:05 -0400 Subject: [PATCH 5/6] [KDB] Enforce line length limit Signed-off-by: Robbie Harwood <rharw...@redhat.com> --- daemons/ipa-kdb/ipa_kdb.c | 6 +- daemons/ipa-kdb/ipa_kdb.h | 18 +- daemons/ipa-kdb/ipa_kdb_certauth.c | 6 +- daemons/ipa-kdb/ipa_kdb_common.c | 3 +- daemons/ipa-kdb/ipa_kdb_delegation.c | 9 +- daemons/ipa-kdb/ipa_kdb_kdcpolicy.c | 29 ++- daemons/ipa-kdb/ipa_kdb_mspac.c | 347 +++++++++++++++----------- daemons/ipa-kdb/ipa_kdb_passwords.c | 3 +- daemons/ipa-kdb/ipa_kdb_principals.c | 98 ++++---- daemons/ipa-kdb/tests/ipa_kdb_tests.c | 91 ++++--- 10 files changed, 365 insertions(+), 245 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb.c b/daemons/ipa-kdb/ipa_kdb.c index 898f40beae..d9a223fbd2 100644 --- a/daemons/ipa-kdb/ipa_kdb.c +++ b/daemons/ipa-kdb/ipa_kdb.c @@ -282,7 +282,8 @@ static int ipadb_load_global_config(struct ipadb_context *ipactx) /* Load authz data. */ ret = ipadb_ldap_attr_to_strlist(ipactx->lcontext, first, - IPA_KRB_AUTHZ_DATA_ATTR, &authz_data_list); + IPA_KRB_AUTHZ_DATA_ATTR, + &authz_data_list); if (ret == 0) { if (ipactx->config.authz_data != NULL) { for (int i = 0; ipactx->config.authz_data[i]; i++) @@ -686,7 +687,8 @@ kdb_vftabl kdb_function_table = { }; #endif -#if (KRB5_KDB_DAL_MAJOR_VERSION == 6) && !defined(HAVE_KDB_FREEPRINCIPAL_EDATA) +#if (KRB5_KDB_DAL_MAJOR_VERSION == 6) && \ + !defined(HAVE_KDB_FREEPRINCIPAL_EDATA) kdb_vftabl kdb_function_table = { .maj_ver = KRB5_KDB_DAL_MAJOR_VERSION, .min_ver = 0, diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h index 3e1eba1c52..04eb3f9a0a 100644 --- a/daemons/ipa-kdb/ipa_kdb.h +++ b/daemons/ipa-kdb/ipa_kdb.h @@ -189,7 +189,8 @@ int ipadb_ldap_deref_results(LDAP *lcontext, LDAPMessage *le, LDAPDerefRes **results); struct ipadb_multires; -krb5_error_code ipadb_multires_init(LDAP *lcontext, struct ipadb_multires **r); +krb5_error_code ipadb_multires_init(LDAP *lcontext, + struct ipadb_multires **r); void ipadb_multires_free(struct ipadb_multires *r); LDAPMessage *ipadb_multires_next_entry(struct ipadb_multires *r); krb5_error_code ipadb_multibase_search(struct ipadb_context *ipactx, @@ -305,19 +306,22 @@ krb5_error_code ipadb_sign_authdata(krb5_context context, krb5_authdata **tgt_auth_data, krb5_authdata ***signed_auth_data); -krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx, bool force_reinit); +krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx, + bool force_reinit); void ipadb_mspac_struct_free(struct ipadb_mspac **mspac); krb5_error_code ipadb_check_transited_realms(krb5_context kcontext, const krb5_data *tr_contents, const krb5_data *client_realm, const krb5_data *server_realm); -/* Checks whether a principal's realm is one of trusted domains' realm or NetBIOS name - * and returns the realm of the matched trusted domain in 'trusted_domain' - * Returns 0 in case of success and KRB5_KDB_NOENTRY otherwise - * If DAL driver is not initialized, returns KRB5_KDB_DBNOTINITED */ +/* Checks whether a principal's realm is one of trusted domains' realm or + * NetBIOS name and returns the realm of the matched trusted domain in + * 'trusted_domain'. Returns 0 in case of success and KRB5_KDB_NOENTRY + * otherwise. If DAL driver is not initialized, returns + * KRB5_KDB_DBNOTINITED. */ krb5_error_code ipadb_is_princ_from_trusted_realm(krb5_context kcontext, - const char *test_realm, size_t size, + const char *test_realm, + size_t size, char **trusted_realm); /* DELEGATION CHECKS */ diff --git a/daemons/ipa-kdb/ipa_kdb_certauth.c b/daemons/ipa-kdb/ipa_kdb_certauth.c index 4babf83229..84cdb119be 100644 --- a/daemons/ipa-kdb/ipa_kdb_certauth.c +++ b/daemons/ipa-kdb/ipa_kdb_certauth.c @@ -275,7 +275,8 @@ static krb5_error_code ipa_certauth_authorize(krb5_context context, return KRB5_PLUGIN_NO_HANDLE; } - if (moddata->sss_certmap_ctx == NULL || time(NULL) > moddata->valid_until) { + if (moddata->sss_certmap_ctx == NULL || + time(NULL) > moddata->valid_until) { kerr = ipa_get_init_data(context, moddata); if (kerr != 0) { krb5_klog_syslog(LOG_ERR, "Failed to init certmapping data"); @@ -288,7 +289,8 @@ static krb5_error_code ipa_certauth_authorize(krb5_context context, ret = KRB5KDC_ERR_CERTIFICATE_MISMATCH; goto done; } - krb5_klog_syslog(LOG_INFO, "Doing certauth authorize for [%s]", principal); + krb5_klog_syslog(LOG_INFO, "Doing certauth authorize for [%s]", + principal); ret = sss_certmap_get_search_filter(moddata->sss_certmap_ctx, cert, cert_len, diff --git a/daemons/ipa-kdb/ipa_kdb_common.c b/daemons/ipa-kdb/ipa_kdb_common.c index d8b5a1ef09..d65a360ea3 100644 --- a/daemons/ipa-kdb/ipa_kdb_common.c +++ b/daemons/ipa-kdb/ipa_kdb_common.c @@ -339,7 +339,8 @@ krb5_error_code ipadb_deref_search(struct ipadb_context *ipactx, goto done; } - ret = ldap_control_create(LDAP_CONTROL_X_DEREF, 1, &derefval, 1, &ctrl[0]); + ret = ldap_control_create(LDAP_CONTROL_X_DEREF, 1, &derefval, 1, + &ctrl[0]); if (ret != LDAP_SUCCESS) { kerr = ENOMEM; goto done; diff --git a/daemons/ipa-kdb/ipa_kdb_delegation.c b/daemons/ipa-kdb/ipa_kdb_delegation.c index 5ae5e0d9d0..998351702b 100644 --- a/daemons/ipa-kdb/ipa_kdb_delegation.c +++ b/daemons/ipa-kdb/ipa_kdb_delegation.c @@ -141,11 +141,12 @@ static krb5_error_code ipadb_match_acl(krb5_context kcontext, case 0: for (dres = deref_results; dres; dres = dres->next) { if (client_found == false && - strcasecmp(dres->derefAttr, "ipaAllowToImpersonate") == 0) { + strcasecmp(dres->derefAttr, + "ipaAllowToImpersonate") == 0) { /* NOTE: client_missing is used to signal that the - * attribute was completely missing. This signals that - * ANY client is allowed to be impersonated. - * This logic is valid only for clients, not for targets */ + * attribute was completely missing. This signals that ANY + * client is allowed to be impersonated. This logic is + * valid only for clients, not for targets. */ client_missing = false; client_found = ipadb_match_member(client_princ, dres); } diff --git a/daemons/ipa-kdb/ipa_kdb_kdcpolicy.c b/daemons/ipa-kdb/ipa_kdb_kdcpolicy.c index 6b61f162e6..d148edb2d1 100644 --- a/daemons/ipa-kdb/ipa_kdb_kdcpolicy.c +++ b/daemons/ipa-kdb/ipa_kdb_kdcpolicy.c @@ -32,16 +32,21 @@ ipa_kdcpolicy_check_as(krb5_context context, krb5_kdcpolicy_moddata moddata, ied = (struct ipadb_e_data *)client->e_data; if (ied == NULL || ied->magic != IPA_E_DATA_MAGIC) { /* e-data is not availble, getting user auth from LDAP */ - krb5_klog_syslog(LOG_INFO, "IPA kdcpolicy: client e_data not availble. Try fetching..."); - kerr = ipadb_get_principal(context, request->client, KRB5_KDB_FLAG_ALIAS_OK, &client); + krb5_klog_syslog( + LOG_INFO, + "IPA kdcpolicy: client e_data not availble. Try fetching..."); + kerr = ipadb_get_principal(context, request->client, + KRB5_KDB_FLAG_ALIAS_OK, &client); if (kerr != 0) { - krb5_klog_syslog(LOG_ERR, "IPA kdcpolicy: ipadb_find_principal failed."); + krb5_klog_syslog(LOG_ERR, + "IPA kdcpolicy: ipadb_find_principal failed."); return kerr; } ied = (struct ipadb_e_data *)client->e_data; if (ied == NULL && ied->magic != IPA_E_DATA_MAGIC) { - krb5_klog_syslog(LOG_ERR, "IPA kdcpolicy: client e_data fetching failed."); + krb5_klog_syslog(LOG_ERR, + "IPA kdcpolicy: client e_data fetching failed."); return EINVAL; } } @@ -78,20 +83,20 @@ ipa_kdcpolicy_check_as(krb5_context context, krb5_kdcpolicy_moddata moddata, } else if (strcmp(auth_indicator, "hardened") == 0) { valid_auth_indicators++; /* Allow hardened even if only password pre-auth is allowed */ - if (!(ua & (IPADB_USER_AUTH_HARDENED | IPADB_USER_AUTH_PASSWORD))) { + if (!(ua & + (IPADB_USER_AUTH_HARDENED | IPADB_USER_AUTH_PASSWORD))) { *status = "Password pre-authentication not not allowed for this user."; return KRB5KDC_ERR_POLICY; } } } - /* There is no auth indicator assigned for non-hardened password authentication - * so we assume password is used when no supported indicator exists */ - if (!valid_auth_indicators) { - if (!(ua & IPADB_USER_AUTH_PASSWORD)) { - *status = "Non-hardened password authentication not allowed for this user."; - return KRB5KDC_ERR_POLICY; - } + /* There is no auth indicator assigned for non-hardened password + * authentication so we assume password is used when no supported + * indicator exists */ + if (!valid_auth_indicators && !(ua & IPADB_USER_AUTH_PASSWORD)) { + *status = "Non-hardened password authentication not allowed for this user."; + return KRB5KDC_ERR_POLICY; } return 0; diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index 4723259900..30953765b9 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -222,15 +222,15 @@ static struct dom_sid *dom_sid_dup(TALLOC_CTX *memctx, return new_sid; } -/* checks if sid1 is a domain of sid2 or compares them exactly if exact_check is true - * returns - * true -- if sid1 is a domain of sid2 (including full exact match) - * false -- otherwise +/* Checks if sid1 is a domain of sid2 or compares them exactly if exact_check + * is true. Returns true if sid1 is a domain of sid2 (including full exact + * match) and false otherwise. * * dom_sid_check() is supposed to be used with sid1 representing domain SID * and sid2 being either domain or resource SID in the domain */ -static bool dom_sid_check(const struct dom_sid *sid1, const struct dom_sid *sid2, bool exact_check) +static bool dom_sid_check(const struct dom_sid *sid1, + const struct dom_sid *sid2, bool exact_check) { int c, num; @@ -281,7 +281,8 @@ static bool dom_sid_check(const struct dom_sid *sid1, const struct dom_sid *sid2 return true; } -static bool dom_sid_is_prefix(const struct dom_sid *sid1, const struct dom_sid *sid2) +static bool dom_sid_is_prefix(const struct dom_sid *sid1, + const struct dom_sid *sid2) { int c; @@ -445,14 +446,15 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx, return ret; } - /* Currently we only add a PAC to TGTs for IPA servers to allow SSSD in - * ipa_server_mode to access the AD LDAP server */ + /* Currently we only add a PAC to TGTs for IPA servers to allow SSSD + * in ipa_server_mode to access the AD LDAP server */ if (!is_master_host(ipactx, strres)) { free(strres); return ENOENT; } } else if (is_service) { - ret = ipadb_ldap_attr_to_str(lcontext, lentry, "krbPrincipalName", &strres); + ret = ipadb_ldap_attr_to_str(lcontext, lentry, "krbPrincipalName", + &strres); if (ret) { /* krbPrincipalName is mandatory for services */ return ret; @@ -472,8 +474,8 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx, data = krb5_princ_component(ipactx->context, princ, 0); for (i = 0; supported_services[i].service; i++) { - if (0 == memcmp(data->data, supported_services[i].service, - MIN(supported_services[i].length, data->length))) { + if (!memcmp(data->data, supported_services[i].service, + MIN(supported_services[i].length, data->length))) { break; } } @@ -726,7 +728,8 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx, if (info3->base.primary_gid == 0) { if (is_host || is_service) { - info3->base.primary_gid = 515; /* Well known RID for domain computers group */ + /* Well known RID for domain computers group */ + info3->base.primary_gid = 515; } else { if (ipactx->mspac->fallback_rid) { info3->base.primary_gid = ipactx->mspac->fallback_rid; @@ -966,8 +969,9 @@ static int get_user_and_group_sids(TALLOC_CTX *memctx, p++; for (c = 0; c < logon_info->info->info3.base.groups.count; c++) { - group_sids[p] = gen_sid_string(memctx, domain_sid, - logon_info->info->info3.base.groups.rids[c].rid); + group_sids[p] = gen_sid_string( + memctx, domain_sid, + logon_info->info->info3.base.groups.rids[c].rid); if (group_sids[p] == NULL) { krb5_klog_syslog(LOG_ERR, "gen_sid_string 2 failed"); ret = EINVAL; @@ -1006,7 +1010,7 @@ static int add_groups(TALLOC_CTX *memctx, struct dom_sid2 *ipa_group_sids) { size_t c; - struct netr_SidAttr *sids = NULL; + struct netr_SidAttr *sids = NULL, *cursid; if (ipa_group_sids_count == 0) { return 0; @@ -1022,11 +1026,10 @@ static int add_groups(TALLOC_CTX *memctx, for (c = 0; c < ipa_group_sids_count; c++) { - sids[c + logon_info->info->info3.sidcount].sid = &ipa_group_sids[c]; - sids[c + logon_info->info->info3.sidcount].attributes = - SE_GROUP_ENABLED | - SE_GROUP_MANDATORY | - SE_GROUP_ENABLED_BY_DEFAULT; + cursid = &sids[c + logon_info->info->info3.sidcount]; + cursid->sid = &ipa_group_sids[c]; + cursid->attributes = SE_GROUP_ENABLED | SE_GROUP_MANDATORY | + SE_GROUP_ENABLED_BY_DEFAULT; } logon_info->info->info3.sidcount += ipa_group_sids_count; @@ -1064,7 +1067,8 @@ static int map_groups(TALLOC_CTX *memctx, krb5_context kcontext, return KRB5_KDB_DBNOTINITED; } - basedn = talloc_asprintf(memctx, "cn=groups,cn=accounts,%s", ipactx->base); + basedn = talloc_asprintf(memctx, "cn=groups,cn=accounts,%s", + ipactx->base); if (basedn == NULL) { krb5_klog_syslog(LOG_ERR, "talloc_asprintf failed."); kerr = ENOMEM; @@ -1073,8 +1077,10 @@ static int map_groups(TALLOC_CTX *memctx, krb5_context kcontext, for (c = 0; group_sids[c] != NULL; c++) { talloc_free(filter); - filter = talloc_asprintf(memctx, "(&(objectclass=ipaExternalGroup)(ipaExternalMember=%s))", - group_sids[c]); + filter = talloc_asprintf( + memctx, + "(&(objectclass=ipaExternalGroup)(ipaExternalMember=%s))", + group_sids[c]); if (filter == NULL) { krb5_klog_syslog(LOG_ERR, "talloc_asprintf failed."); kerr = ENOMEM; @@ -1097,7 +1103,8 @@ static int map_groups(TALLOC_CTX *memctx, krb5_context kcontext, do { ldap_derefresponse_free(deref_results); - ret = ipadb_ldap_deref_results(ipactx->lcontext, lentry, &deref_results); + ret = ipadb_ldap_deref_results(ipactx->lcontext, lentry, + &deref_results); switch (ret) { case ENOENT: /* No entry found, try next SID */ @@ -1112,7 +1119,8 @@ static int map_groups(TALLOC_CTX *memctx, krb5_context kcontext, count++; } - sids = talloc_realloc(memctx, sids, struct dom_sid, count); + sids = talloc_realloc(memctx, sids, struct dom_sid, + count); if (sids == NULL) { krb5_klog_syslog(LOG_ERR, "talloc_realloc failed."); kerr = ENOMEM; @@ -1127,29 +1135,31 @@ static int map_groups(TALLOC_CTX *memctx, krb5_context kcontext, errno = 0; gid = strtoul((char *)dval->vals[0].bv_val, &endptr,10); - if (gid == 0 || gid >= UINT32_MAX || errno != 0 || - *endptr != '\0') { + if (gid == 0 || gid >= UINT32_MAX || + errno != 0 || *endptr != '\0') { continue; } } if (strcasecmp(dval->type, "ipaNTSecurityIdentifier") == 0) { - kerr = string_to_sid((char *)dval->vals[0].bv_val, &sid); + kerr = string_to_sid( + (char *)dval->vals[0].bv_val, &sid); if (kerr != 0) { continue; } } } if (gid != 0 && sid.sid_rev_num != 0) { - /* TODO: check if gid maps to sid */ + /* TODO: check if gid maps to sid */ if (sid_index >= count) { - krb5_klog_syslog(LOG_ERR, "Index larger than " - "array, this shoould " - "never happen."); + krb5_klog_syslog( + LOG_ERR, + "Index larger than array, this should never happen."); kerr = EFAULT; goto done; } - memcpy(&sids[sid_index], &sid, sizeof(struct dom_sid)); + memcpy(&sids[sid_index], &sid, + sizeof(struct dom_sid)); sid_index++; } } @@ -1282,8 +1292,8 @@ static struct ipadb_adtrusts *get_domain_from_realm(krb5_context context, return NULL; } -static struct ipadb_adtrusts *get_domain_from_realm_update(krb5_context context, - krb5_data realm) +static struct ipadb_adtrusts * +get_domain_from_realm_update(krb5_context context, krb5_data realm) { struct ipadb_context *ipactx; struct ipadb_adtrusts *domain; @@ -1310,29 +1320,34 @@ static void filter_logon_info_log_message(struct dom_sid *sid) domstr = dom_sid_string(NULL, sid); if (domstr) { - krb5_klog_syslog(LOG_ERR, "PAC filtering issue: SID [%s] is not allowed " - "from a trusted source and will be excluded.", domstr); + krb5_klog_syslog( + LOG_ERR, + "PAC filtering issue: SID [%s] is not allowed from a trusted source and will be excluded.", + domstr); talloc_free(domstr); } else { - krb5_klog_syslog(LOG_ERR, "PAC filtering issue: SID is not allowed " - "from a trusted source and will be excluded." - "Unable to allocate memory to display SID."); + krb5_klog_syslog( + LOG_ERR, + "PAC filtering issue: SID is not allowed from a trusted source and will be excluded. Unable to allocate memory to display SID."); } } -static void filter_logon_info_log_message_rid(struct dom_sid *sid, uint32_t rid) +static void filter_logon_info_log_message_rid(struct dom_sid *sid, + uint32_t rid) { char *domstr = NULL; domstr = dom_sid_string(NULL, sid); if (domstr) { - krb5_klog_syslog(LOG_ERR, "PAC filtering issue: SID [%s-%d] is not allowed " - "from a trusted source and will be excluded.", domstr, rid); + krb5_klog_syslog( + LOG_ERR, + "PAC filtering issue: SID [%s-%d] is not allowed from a trusted source and will be excluded.", + domstr, rid); talloc_free(domstr); } else { - krb5_klog_syslog(LOG_ERR, "PAC filtering issue: SID is not allowed " - "from a trusted source and will be excluded." - "Unable to allocate memory to display SID."); + krb5_klog_syslog( + LOG_ERR, + "PAC filtering issue: SID is not allowed from a trusted source and will be excluded. Unable to allocate memory to display SID."); } } @@ -1364,6 +1379,7 @@ krb5_error_code filter_logon_info(krb5_context context, uint32_t rid; bool result; char *domstr = NULL; + struct dom_sid *curbl; domain = get_domain_from_realm_update(context, realm); if (!domain) { @@ -1382,7 +1398,8 @@ krb5_error_code filter_logon_info(krb5_context context, } /* check exact sid */ - result = dom_sid_check(&domain->domsid, info->info->info3.base.domain_sid, true); + result = dom_sid_check(&domain->domsid, info->info->info3.base.domain_sid, + true); if (!result) { domstr = dom_sid_string(NULL, info->info->info3.base.domain_sid); if (!domstr) { @@ -1391,7 +1408,8 @@ krb5_error_code filter_logon_info(krb5_context context, krb5_klog_syslog(LOG_ERR, "PAC Info mismatch: domain = %s, " "expected domain SID = %s, " "found domain SID = %s", - domain->domain_name, domain->domain_sid, domstr); + domain->domain_name, domain->domain_sid, + domstr); talloc_free(domstr); return EINVAL; } @@ -1399,10 +1417,12 @@ krb5_error_code filter_logon_info(krb5_context context, /* Check if this domain has been filtered out by the trust itself*/ if (domain->parent != NULL) { for(k = 0; k < domain->parent->len_sid_blacklist_incoming; k++) { - result = dom_sid_check(info->info->info3.base.domain_sid, - &domain->parent->sid_blacklist_incoming[k], true); + result = dom_sid_check( + info->info->info3.base.domain_sid, + &domain->parent->sid_blacklist_incoming[k], true); if (result) { - filter_logon_info_log_message(info->info->info3.base.domain_sid); + filter_logon_info_log_message( + info->info->info3.base.domain_sid); return KRB5KDC_ERR_POLICY; } } @@ -1410,24 +1430,29 @@ krb5_error_code filter_logon_info(krb5_context context, /* Check if this user's SIDs membership is filtered too */ for(k = 0; k < domain->len_sid_blacklist_incoming; k++) { - /* Short-circuit if there are no RIDs. This may happen if we filtered everything already. - * In normal situation there would be at least primary gid as RID in the RIDs array - * but if we filtered out the primary RID, this MS-PAC is invalid */ + /* Short-circuit if there are no RIDs. This may happen if we filtered + * everything already. In normal situation there would be at least + * primary gid as RID in the RIDs array but if we filtered out the + * primary RID, this MS-PAC is invalid */ count = info->info->info3.base.groups.count; result = dom_sid_is_prefix(info->info->info3.base.domain_sid, &domain->sid_blacklist_incoming[k]); + curbl = &domain->sid_blacklist_incoming[k]; if (result) { i = 0; j = 0; - if (domain->sid_blacklist_incoming[k].num_auths - info->info->info3.base.domain_sid->num_auths != 1) { - krb5_klog_syslog(LOG_ERR, "Incoming SID blacklist element matching domain [%s with SID %s] " - "has more than one RID component. Invalid check skipped.", - domain->domain_name, domain->domain_sid); + if (curbl->num_auths - + info->info->info3.base.domain_sid->num_auths != 1) { + krb5_klog_syslog( + LOG_ERR, + "Incoming SID blacklist element matching domain [%s with SID %s] has more than one RID component. Invalid check skipped.", + domain->domain_name, domain->domain_sid); break; } - rid = domain->sid_blacklist_incoming[k].sub_auths[domain->sid_blacklist_incoming[k].num_auths - 1]; + rid = curbl->sub_auths[curbl->num_auths - 1]; if (rid == info->info->info3.base.rid) { - filter_logon_info_log_message_rid(info->info->info3.base.domain_sid, rid); + filter_logon_info_log_message_rid( + info->info->info3.base.domain_sid, rid); /* Actual user's SID is filtered out */ return KRB5KDC_ERR_POLICY; } @@ -1436,15 +1461,18 @@ krb5_error_code filter_logon_info(krb5_context context, return KRB5KDC_ERR_POLICY; } if (count == 0) { - /* Having checked actual user's SID and primary group SID, and having no other RIDs, - * skip checks below and continue to next blacklist element */ + /* Having checked actual user's SID and primary group SID, and + * having no other RIDs, skip checks below and continue to + * next blacklist element */ continue; } do { if (rid == info->info->info3.base.groups.rids[i].rid) { - filter_logon_info_log_message_rid(info->info->info3.base.domain_sid, rid); - /* If this is just a non-primary RID, we simply remove it from the array of RIDs */ + filter_logon_info_log_message_rid( + info->info->info3.base.domain_sid, rid); + /* If this is just a non-primary RID, we simply remove it + * from the array of RIDs */ l = count - i - j - 1; if (l != 0) { memmove(info->info->info3.base.groups.rids+i, @@ -1460,14 +1488,16 @@ krb5_error_code filter_logon_info(krb5_context context, if (j != 0) { count = count-j; if (count == 0) { - /* All RIDs were filtered out. Unusual but MS-KILE 3.3.5.6.3.1 says SHOULD, not MUST for GroupCount */ + /* All RIDs were filtered out. Unusual but MS-KILE + * 3.3.5.6.3.1 says SHOULD, not MUST for GroupCount */ info->info->info3.base.groups.count = 0; talloc_free(info->info->info3.base.groups.rids); info->info->info3.base.groups.rids = NULL; } else { - info->info->info3.base.groups.rids = talloc_realloc(memctx, - info->info->info3.base.groups.rids, - struct samr_RidWithAttribute, count); + info->info->info3.base.groups.rids = + talloc_realloc(memctx, + info->info->info3.base.groups.rids, + struct samr_RidWithAttribute, count); if (!info->info->info3.base.groups.rids) { info->info->info3.base.groups.count = 0; return ENOMEM; @@ -1478,9 +1508,8 @@ krb5_error_code filter_logon_info(krb5_context context, } } - /* According to MS-KILE 25.0, info->info->info3.sids may be non zero, so check - * should include different possibilities into account - * */ + /* According to MS-KILE 25.0, info->info->info3.sids may be non zero, so + * check should include different possibilities into account. */ if (info->info->info3.sidcount != 0) { ipactx = ipadb_get_context(context); if (!ipactx || !ipactx->mspac) { @@ -1491,22 +1520,30 @@ krb5_error_code filter_logon_info(krb5_context context, j = 0; do { /* Compare SID with our domain without taking RID into account */ - result = dom_sid_check(&ipactx->mspac->domsid, info->info->info3.sids[i].sid, false); + result = dom_sid_check(&ipactx->mspac->domsid, + info->info->info3.sids[i].sid, false); if (result) { filter_logon_info_log_message(info->info->info3.sids[i].sid); } else { /* Go over incoming SID blacklist */ for(k = 0; k < domain->len_sid_blacklist_incoming; k++) { /* if SID is an exact match, filter it out */ - result = dom_sid_check(&domain->sid_blacklist_incoming[k], info->info->info3.sids[i].sid, true); + result = dom_sid_check(&domain->sid_blacklist_incoming[k], + info->info->info3.sids[i].sid, + true); if (result) { - filter_logon_info_log_message(info->info->info3.sids[i].sid); + filter_logon_info_log_message( + info->info->info3.sids[i].sid); break; } - /* if SID is a suffix of the blacklist element, filter it out*/ - result = dom_sid_is_prefix(&domain->sid_blacklist_incoming[k], info->info->info3.sids[i].sid); + /* If SID is a suffix of the blacklist element, filter it + * out. */ + result = dom_sid_is_prefix( + &domain->sid_blacklist_incoming[k], + info->info->info3.sids[i].sid); if (result) { - filter_logon_info_log_message(info->info->info3.sids[i].sid); + filter_logon_info_log_message( + info->info->info3.sids[i].sid); break; } } @@ -1532,9 +1569,9 @@ krb5_error_code filter_logon_info(krb5_context context, talloc_free(info->info->info3.sids); info->info->info3.sids = NULL; } else { - info->info->info3.sids = talloc_realloc(memctx, - info->info->info3.sids, - struct netr_SidAttr, count); + info->info->info3.sids = talloc_realloc( + memctx, info->info->info3.sids, struct netr_SidAttr, + count); if (!info->info->info3.sids) { info->info->info3.sidcount = 0; return ENOMEM; @@ -1810,7 +1847,8 @@ static krb5_error_code ipadb_verify_pac(krb5_context context, goto done; } - kerr = ipadb_check_logon_info(context, client_princ->realm, &pac_blob); + kerr = ipadb_check_logon_info(context, client_princ->realm, + &pac_blob); if (kerr != 0) { goto done; } @@ -2013,9 +2051,9 @@ void get_authz_data_types(krb5_context context, krb5_db_entry *entry, tmp = gcfg->authz_data; } if (ipactx == NULL || tmp == NULL) { - krb5_klog_syslog(LOG_ERR, "No default authorization data types " \ - "available, no authorization data will " \ - "be added."); + krb5_klog_syslog( + LOG_ERR, + "No default authorization data types available, no authorization data will be added."); goto done; } @@ -2031,17 +2069,17 @@ void get_authz_data_types(krb5_context context, krb5_db_entry *entry, sep = strchr(authz_data_list[c], ':'); if (sep != NULL && entry != NULL) { if (entry->princ == NULL) { - krb5_klog_syslog(LOG_ERR, "Missing principal in database " - "entry, no authorization data will " \ - "be added."); + krb5_klog_syslog( + LOG_ERR, + "Missing principal in database entry, no authorization data will be added."); goto done; } service_type = krb5_princ_component(context, entry->princ, 0); if (service_type == NULL) { - krb5_klog_syslog(LOG_ERR, "Missing service type in database " - "entry, no authorization data will " \ - "be added."); + krb5_klog_syslog( + LOG_ERR, + "Missing service type in database entry, no authorization data will be added."); goto done; } @@ -2133,7 +2171,7 @@ krb5_error_code ipadb_sign_authdata(krb5_context context, krb5_db_entry *client_entry = NULL; krb5_boolean is_equal; bool force_reinit_mspac = false; - + krb5_data *pc0, *pc1; is_as_req = ((flags & KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY) != 0); @@ -2145,12 +2183,15 @@ krb5_error_code ipadb_sign_authdata(krb5_context context, if (!is_as_req) { is_equal = false; if ((client != NULL) && (client->princ != NULL)) { - is_equal = krb5_principal_compare(context, client_princ, client->princ); + is_equal = krb5_principal_compare(context, client_princ, + client->princ); } if (!is_equal) { - kerr = ipadb_get_principal(context, client_princ, flags, &client_entry); - /* If we didn't find client_princ in our database, it might be: - * - a principal from another realm, handle it down in ipadb_get/verify_pac() + kerr = ipadb_get_principal(context, client_princ, flags, + &client_entry); + /* If we didn't find client_princ in our database, it might + * be: - a principal from another realm, handle it down in + * ipadb_get/verify_pac() */ if (kerr != 0) { client_entry = NULL; @@ -2198,14 +2239,16 @@ krb5_error_code ipadb_sign_authdata(krb5_context context, * other cases call ipadb_reinit_mspac() with force_reinit_mspac set * to 'false' to make sure the information about trusted domains is * updated on a regular basis for all worker processes. */ - if ((krb5_princ_size(context, ks_client_princ) == 2) && - (strncmp(krb5_princ_component(context, ks_client_princ, 0)->data, "HTTP", - krb5_princ_component(context, ks_client_princ, 0)->length) == 0) && - (ulc_casecmp(krb5_princ_component(context, ks_client_princ, 1)->data, - krb5_princ_component(context, ks_client_princ, 1)->length, - ipactx->kdc_hostname, strlen(ipactx->kdc_hostname), - NULL, NULL, &result) == 0)) { - force_reinit_mspac = true; + + if (krb5_princ_size(context, ks_client_princ) == 2) { + pc0 = krb5_princ_component(context, ks_client_princ, 0); + pc1 = krb5_princ_component(context, ks_client_princ, 1); + if (strncmp(pc0->data, "HTTP", pc0->length) == 0 && + ulc_casecmp(pc1->data, pc1->length, ipactx->kdc_hostname, + strlen(ipactx->kdc_hostname), NULL, NULL, + &result) == 0) { + force_reinit_mspac = true; + } } (void)ipadb_reinit_mspac(ipactx, force_reinit_mspac); @@ -2337,9 +2380,10 @@ void ipadb_mspac_struct_free(struct ipadb_mspac **mspac) *mspac = NULL; } -krb5_error_code ipadb_adtrusts_fill_sid_blacklist(char **source_sid_blacklist, - struct dom_sid **result_sids, - int *result_length) +krb5_error_code +ipadb_adtrusts_fill_sid_blacklist(char **source_sid_blacklist, + struct dom_sid **result_sids, + int *result_length) { int len, i; char **source; @@ -2370,22 +2414,23 @@ krb5_error_code ipadb_adtrusts_fill_sid_blacklist(char **source_sid_blacklist, return 0; } -krb5_error_code ipadb_adtrusts_fill_sid_blacklists(struct ipadb_adtrusts *adtrust, - char **sid_blacklist_incoming, - char **sid_blacklist_outgoing) +krb5_error_code +ipadb_adtrusts_fill_sid_blacklists(struct ipadb_adtrusts *adtrust, + char **sid_blacklist_incoming, + char **sid_blacklist_outgoing) { krb5_error_code kerr; - kerr = ipadb_adtrusts_fill_sid_blacklist(sid_blacklist_incoming, - &adtrust->sid_blacklist_incoming, - &adtrust->len_sid_blacklist_incoming); + kerr = ipadb_adtrusts_fill_sid_blacklist( + sid_blacklist_incoming, &adtrust->sid_blacklist_incoming, + &adtrust->len_sid_blacklist_incoming); if (kerr) { return kerr; } - kerr = ipadb_adtrusts_fill_sid_blacklist(sid_blacklist_outgoing, - &adtrust->sid_blacklist_outgoing, - &adtrust->len_sid_blacklist_outgoing); + kerr = ipadb_adtrusts_fill_sid_blacklist( + sid_blacklist_outgoing, &adtrust->sid_blacklist_outgoing, + &adtrust->len_sid_blacklist_outgoing); if (kerr) { return kerr; } @@ -2393,7 +2438,8 @@ krb5_error_code ipadb_adtrusts_fill_sid_blacklists(struct ipadb_adtrusts *adtrus return 0; } -krb5_error_code ipadb_mspac_check_trusted_domains(struct ipadb_context *ipactx) +krb5_error_code +ipadb_mspac_check_trusted_domains(struct ipadb_context *ipactx) { char *attrs[] = { NULL }; char *filter = "(objectclass=ipaNTTrustedDomain)"; @@ -2417,12 +2463,12 @@ krb5_error_code ipadb_mspac_check_trusted_domains(struct ipadb_context *ipactx) return ret; } -static void ipadb_free_sid_blacklists(char ***sid_blacklist_incoming, char ***sid_blacklist_outgoing) +static void ipadb_free_sid_blacklists(char ***sid_blacklist_incoming, + char ***sid_blacklist_outgoing) { - int i; - if (sid_blacklist_incoming && *sid_blacklist_incoming) { - for (i = 0; *sid_blacklist_incoming && (*sid_blacklist_incoming)[i]; i++) { + for (int i = 0; + *sid_blacklist_incoming && (*sid_blacklist_incoming)[i]; i++) { free((*sid_blacklist_incoming)[i]); } free(*sid_blacklist_incoming); @@ -2430,7 +2476,8 @@ static void ipadb_free_sid_blacklists(char ***sid_blacklist_incoming, char ***si } if (sid_blacklist_outgoing && *sid_blacklist_outgoing) { - for (i = 0; *sid_blacklist_outgoing && (*sid_blacklist_outgoing)[i]; i++) { + for (int i = 0; + *sid_blacklist_outgoing && (*sid_blacklist_outgoing)[i]; i++) { free((*sid_blacklist_outgoing)[i]); } free(*sid_blacklist_outgoing); @@ -2444,7 +2491,8 @@ krb5_error_code ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx) LDAP *lc = ipactx->lcontext; char *attrs[] = { "cn", "ipaNTTrustPartner", "ipaNTFlatName", "ipaNTTrustedDomainSID", "ipaNTSIDBlacklistIncoming", - "ipaNTSIDBlacklistOutgoing", "ipaNTAdditionalSuffixes", NULL }; + "ipaNTSIDBlacklistOutgoing", "ipaNTAdditionalSuffixes", + NULL }; char *filter = "(objectclass=ipaNTTrustedDomain)"; krb5_error_code kerr; LDAPMessage *res = NULL; @@ -2485,8 +2533,9 @@ krb5_error_code ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx) n = ipactx->mspac->num_trusts; ipactx->mspac->num_trusts++; - t = realloc(ipactx->mspac->trusts, - sizeof(struct ipadb_adtrusts) * ipactx->mspac->num_trusts); + t = realloc( + ipactx->mspac->trusts, + sizeof(struct ipadb_adtrusts) * ipactx->mspac->num_trusts); if (!t) { ret = ENOMEM; goto done; @@ -2603,7 +2652,8 @@ krb5_error_code ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx) goto done; } - t[n].parent_name = strndup(rdn[0]->la_value.bv_val, rdn[0]->la_value.bv_len); + t[n].parent_name = strndup(rdn[0]->la_value.bv_val, + rdn[0]->la_value.bv_len); ldap_dnfree(dn); @@ -2637,7 +2687,8 @@ krb5_error_code ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx) return ret; } -krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx, bool force_reinit) +krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx, + bool force_reinit) { char *dom_attrs[] = { "ipaNTFlatName", "ipaNTFallbackPrimaryGroup", @@ -2815,10 +2866,12 @@ krb5_error_code ipadb_check_transited_realms(krb5_context kcontext, has_server_realm = false; /* First, compare client or server realm with ours */ - if (strncasecmp(client_realm->data, ipactx->realm, client_realm->length) == 0) { + if (strncasecmp(client_realm->data, ipactx->realm, + client_realm->length) == 0) { has_client_realm = true; } - if (strncasecmp(server_realm->data, ipactx->realm, server_realm->length) == 0) { + if (strncasecmp(server_realm->data, ipactx->realm, + server_realm->length) == 0) { has_server_realm = true; } @@ -2827,7 +2880,8 @@ krb5_error_code ipadb_check_transited_realms(krb5_context kcontext, if (has_client_realm && has_server_realm) { return 0; } - /* Since transited realm is empty, we don't need to check for it, it is a direct trust case */ + /* Since the transited realm is empty, we don't need to check for it: + * it is a direct trust case. */ has_transited_contents = true; } @@ -2835,23 +2889,31 @@ krb5_error_code ipadb_check_transited_realms(krb5_context kcontext, return KRB5_PLUGIN_NO_HANDLE; } - /* Iterate through list of trusts and check if any of input belongs to any of the trust */ + /* Iterate through list of trusts and check if any of input belongs to any + * of them. */ for(i=0; i < ipactx->mspac->num_trusts ; i++) { if (!has_transited_contents && - (strncasecmp(tr_contents->data, ipactx->mspac->trusts[i].domain_name, tr_contents->length) == 0)) { + strncasecmp(tr_contents->data, + ipactx->mspac->trusts[i].domain_name, + tr_contents->length) == 0) { has_transited_contents = true; } if (!has_client_realm && - (strncasecmp(client_realm->data, ipactx->mspac->trusts[i].domain_name, client_realm->length) == 0)) { + strncasecmp(client_realm->data, + ipactx->mspac->trusts[i].domain_name, + client_realm->length) == 0) { has_client_realm = true; } if (!has_server_realm && - (strncasecmp(server_realm->data, ipactx->mspac->trusts[i].domain_name, server_realm->length) == 0)) { + strncasecmp(server_realm->data, + ipactx->mspac->trusts[i].domain_name, + server_realm->length) == 0) { has_server_realm = true; } } - /* Tell to KDC that we don't handle this transition so that rules in krb5.conf could play its role */ + /* Tell to KDC that we don't handle this transition so that it can use the + * rules in krb5.conf instead. */ ret = KRB5_PLUGIN_NO_HANDLE; if (has_client_realm && has_transited_contents && has_server_realm) { ret = 0; @@ -2859,12 +2921,14 @@ krb5_error_code ipadb_check_transited_realms(krb5_context kcontext, return ret; } -/* Checks whether a principal's realm is one of trusted domains' realm or NetBIOS name - * and returns the realm of the matched trusted domain in 'trusted_domain' - * Returns 0 in case of success and KRB5_KDB_NOENTRY otherwise - * If DAL driver is not initialized, returns KRB5_KDB_DBNOTINITED */ +/* Checks whether a principal's realm is one of trusted domains' realm or + * NetBIOS name and returns the realm of the matched trusted domain in + * 'trusted_domain'. Returns 0 in case of success and KRB5_KDB_NOENTRY + * otherwise. If DAL driver is not initialized, returns + * KRB5_KDB_DBNOTINITED */ krb5_error_code ipadb_is_princ_from_trusted_realm(krb5_context kcontext, - const char *test_realm, size_t size, + const char *test_realm, + size_t size, char **trusted_realm) { struct ipadb_context *ipactx; @@ -2881,7 +2945,7 @@ krb5_error_code ipadb_is_princ_from_trusted_realm(krb5_context kcontext, return KRB5_KDB_DBNOTINITED; } - /* First, compare realm with ours, it would not be from a trusted realm then */ + /* First, compare realm with ours (since then it's not a trusted realm) */ if (strncasecmp(test_realm, ipactx->realm, size) == 0) { return KRB5_KDB_NOENTRY; } @@ -2890,7 +2954,8 @@ krb5_error_code ipadb_is_princ_from_trusted_realm(krb5_context kcontext, return KRB5_KDB_NOENTRY; } - /* Iterate through list of trusts and check if input realm belongs to any of the trust */ + /* Iterate through list of trusts and check if input realm belongs to any + of them. */ for(i = 0 ; i < ipactx->mspac->num_trusts ; i++) { result = strncasecmp(test_realm, ipactx->mspac->trusts[i].domain_name, diff --git a/daemons/ipa-kdb/ipa_kdb_passwords.c b/daemons/ipa-kdb/ipa_kdb_passwords.c index 1862c10374..b40ed09c2b 100644 --- a/daemons/ipa-kdb/ipa_kdb_passwords.c +++ b/daemons/ipa-kdb/ipa_kdb_passwords.c @@ -266,7 +266,8 @@ krb5_error_code ipadb_get_pwd_expiration(krb5_context context, *expire_time = mod_time; } - /* in the case of integer owerflow, set expiration to IPAPWD_END_OF_TIME */ + /* In the case of integer owerflow, set expiration to + * IPAPWD_END_OF_TIME. */ if ((*expire_time) < 0 || (*expire_time) > IPAPWD_END_OF_TIME) { *expire_time = IPAPWD_END_OF_TIME; // 1 Jan 2038, 00:00 GMT } diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index 4804e6fe7b..c775fbd3a3 100644 --- a/daemons/ipa-kdb/ipa_kdb_principals.c +++ b/daemons/ipa-kdb/ipa_kdb_principals.c @@ -28,22 +28,24 @@ * During TGS request search by ipaKrbPrincipalName (case-insensitive) * and krbPrincipalName (case-sensitive) */ -#define PRINC_TGS_SEARCH_FILTER "(&(|(objectclass=krbprincipalaux)" \ - "(objectclass=krbprincipal)" \ - "(objectclass=ipakrbprincipal))" \ - "(|(ipakrbprincipalalias=%s)" \ - "(krbprincipalname:caseIgnoreIA5Match:=%s)))" +#define PRINC_TGS_SEARCH_FILTER \ + "(&(|(objectclass=krbprincipalaux)" \ + "(objectclass=krbprincipal)" \ + "(objectclass=ipakrbprincipal))" \ + "(|(ipakrbprincipalalias=%s)" \ + "(krbprincipalname:caseIgnoreIA5Match:=%s)))" #define PRINC_SEARCH_FILTER "(&(|(objectclass=krbprincipalaux)" \ "(objectclass=krbprincipal))" \ "(krbprincipalname=%s))" -#define PRINC_TGS_SEARCH_FILTER_EXTRA "(&(|(objectclass=krbprincipalaux)" \ - "(objectclass=krbprincipal)" \ - "(objectclass=ipakrbprincipal))" \ - "(|(ipakrbprincipalalias=%s)" \ - "(krbprincipalname:caseIgnoreIA5Match:=%s))" \ - "%s)" +#define PRINC_TGS_SEARCH_FILTER_EXTRA \ + "(&(|(objectclass=krbprincipalaux)" \ + "(objectclass=krbprincipal)" \ + "(objectclass=ipakrbprincipal))" \ + "(|(ipakrbprincipalalias=%s)" \ + "(krbprincipalname:caseIgnoreIA5Match:=%s))" \ + "%s)" #define PRINC_SEARCH_FILTER_EXTRA "(&(|(objectclass=krbprincipalaux)" \ "(objectclass=krbprincipal))" \ @@ -104,7 +106,8 @@ static char *std_principal_obj_classes[] = { NULL }; -#define STD_PRINCIPAL_OBJ_CLASSES_SIZE (sizeof(std_principal_obj_classes) / sizeof(char *) - 1) +#define STD_PRINCIPAL_OBJ_CLASSES_SIZE \ + (sizeof(std_principal_obj_classes) / sizeof(char *) - 1) #define DEFAULT_TL_DATA_CONTENT "\x00\x00\x00\x00principal@UNINITIALIZED" @@ -596,7 +599,8 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext, } ret = ipadb_ldap_attr_to_krb5_timestamp(lcontext, lentry, - "krbPrincipalexpiration", &restime); + "krbPrincipalexpiration", + &restime); switch (ret) { case 0: entry->expiration = restime; @@ -608,7 +612,8 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext, } ret = ipadb_ldap_attr_to_krb5_timestamp(lcontext, lentry, - "krbPasswordExpiration", &restime); + "krbPasswordExpiration", + &restime); switch (ret) { case 0: entry->pw_expiration = restime; @@ -694,10 +699,10 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext, entry->n_tl_data = result; break; case ENOENT: - /* The kadmin utility expects always at least KRB5_TL_MOD_PRINC tl_data - * to be available. So if krbExtraData is missing (may happen when a - * user is created but no password has been set yet) then add a default - * one. */ + /* The kadmin utility expects always at least KRB5_TL_MOD_PRINC + * tl_data to be available. So if krbExtraData is missing (may happen + * when a user is created but no password has been set yet) then add a + * default one. */ kerr = ipadb_set_tl_data(entry, KRB5_TL_MOD_PRINC, sizeof(DEFAULT_TL_DATA_CONTENT), (const krb5_octet *)DEFAULT_TL_DATA_CONTENT); @@ -843,7 +848,8 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext, } ret = ipadb_ldap_attr_to_strlist(lcontext, lentry, - IPA_KRB_AUTHZ_DATA_ATTR, &authz_data_list); + IPA_KRB_AUTHZ_DATA_ATTR, + &authz_data_list); if (ret != 0 && ret != ENOENT) { kerr = KRB5_KDB_INTERNAL_ERROR; goto done; @@ -911,7 +917,8 @@ ipadb_fetch_principals_with_extra_filter(struct ipadb_context *ipactx, ret = asprintf(&src_filter, PRINC_TGS_SEARCH_FILTER, esc_original_princ, esc_original_princ); } else { - ret = asprintf(&src_filter, PRINC_SEARCH_FILTER, esc_original_princ); + ret = asprintf(&src_filter, PRINC_SEARCH_FILTER, + esc_original_princ); } } else { if (flags & KRB5_KDB_FLAG_ALIAS_OK) { @@ -984,8 +991,8 @@ krb5_error_code ipadb_find_principal(krb5_context kcontext, /* we need to check for a strict match as a '*' in the name may have * caused the ldap server to return multiple entries */ for (i = 0; vals[i]; i++) { - /* KDC will accept aliases when doing TGT lookup (ref_tgt_again in do_tgs_req.c */ - /* Use case-insensitive comparison in such cases */ + /* KDC will accept aliases when doing TGT lookup (ref_tgt_again in + do_tgs_req.c. Use case-insensitive comparison in such cases. */ if ((flags & KRB5_KDB_FLAG_ALIAS_OK) != 0) { if (ulc_casecmp(vals[i]->bv_val, vals[i]->bv_len, (*principal), strlen(*principal), @@ -1023,7 +1030,8 @@ krb5_error_code ipadb_find_principal(krb5_context kcontext, continue; } - /* Again, if aliases are accepted by KDC, use case-insensitive comparison */ + /* Again, if aliases are accepted by KDC, use case-insensitive + * comparison. */ if ((flags & KRB5_KDB_FLAG_ALIAS_OK) != 0) { found = true; } else { @@ -1216,9 +1224,9 @@ krb5_error_code ipadb_get_principal(krb5_context kcontext, char *realm; krb5_data *upn; - upn = krb5_princ_component(kcontext, search_for, - krb5_princ_size(kcontext, search_for) - 1); - + upn = krb5_princ_component( + kcontext, search_for, + krb5_princ_size(kcontext, search_for) - 1); if (upn == NULL) { kerr = KRB5_KDB_NOENTRY; goto done; @@ -1230,13 +1238,15 @@ krb5_error_code ipadb_get_principal(krb5_context kcontext, goto done; } - /* skip '@' and use part after '@' as an enterprise realm for comparison */ + /* Skip '@' and usethe part after '@' as an enterprise realm for + * comparison. */ realm++; /* check for our realm */ if (strncasecmp(ipactx->realm, realm, upn->length - (realm - upn->data)) == 0) { - /* it looks like it is ok to use malloc'ed strings as principal */ + /* It looks like it is okay to use malloc'ed strings for + * principal. */ krb5_free_unparsed_name(kcontext, principal); principal = strndup((const char *) upn->data, upn->length); if (principal == NULL) { @@ -1257,11 +1267,9 @@ krb5_error_code ipadb_get_principal(krb5_context kcontext, goto done; } } else { - - kerr = ipadb_is_princ_from_trusted_realm(kcontext, - realm, - upn->length - (realm - upn->data), - &trusted_realm); + kerr = ipadb_is_princ_from_trusted_realm( + kcontext, realm, upn->length - (realm - upn->data), + &trusted_realm); if (kerr == KRB5_KDB_NOENTRY) { /* try to refresh trusted domain data and try again */ kerr = ipadb_reinit_mspac(ipactx, false); @@ -1269,9 +1277,9 @@ krb5_error_code ipadb_get_principal(krb5_context kcontext, kerr = KRB5_KDB_NOENTRY; goto done; } - kerr = ipadb_is_princ_from_trusted_realm(kcontext, realm, - upn->length - (realm - upn->data), - &trusted_realm); + kerr = ipadb_is_princ_from_trusted_realm( + kcontext, realm, upn->length - (realm - upn->data), + &trusted_realm); } if (kerr == 0) { kentry = calloc(1, sizeof(krb5_db_entry)); @@ -1285,7 +1293,8 @@ krb5_error_code ipadb_get_principal(krb5_context kcontext, goto done; } - kerr = krb5_set_principal_realm(kcontext, kentry->princ, trusted_realm); + kerr = krb5_set_principal_realm(kcontext, kentry->princ, + trusted_realm); if (kerr != 0) { goto done; } @@ -1668,7 +1677,8 @@ static krb5_error_code ipadb_get_ldap_mod_extra_data(struct ipadb_mods *imods, goto done; } memcpy(bvs[i]->bv_val, &be_type, 2); - memcpy(&(bvs[i]->bv_val[2]), data->tl_data_contents, data->tl_data_length); + memcpy(&(bvs[i]->bv_val[2]), data->tl_data_contents, + data->tl_data_length); i++; @@ -1927,7 +1937,8 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext, if (entry->pw_expiration == 0) { kerr = ipadb_get_ldap_mod_time(imods, "krbPasswordExpiration", - entry->pw_expiration, LDAP_MOD_DELETE); + entry->pw_expiration, + LDAP_MOD_DELETE); } if (kerr) { goto done; @@ -2117,7 +2128,8 @@ static krb5_error_code ipadb_entry_to_mods(krb5_context kcontext, } /* Do not store auth indicators from the string attribute in - * krbExtraData. Remove require_auth value from the entry temporarily. */ + * krbExtraData. Remove require_auth value from the entry + * temporarily. */ if (req_auth_str != NULL) { kerr = krb5_dbe_set_string(kcontext, entry, "require_auth", NULL); if (kerr) { @@ -2245,7 +2257,8 @@ static krb5_error_code ipadb_entry_default_attrs(struct ipadb_mods *imods) kerr = ENOMEM; goto done; } - m->mod_values = calloc(STD_PRINCIPAL_OBJ_CLASSES_SIZE + 1, sizeof(char *)); + m->mod_values = calloc(STD_PRINCIPAL_OBJ_CLASSES_SIZE + 1, + sizeof(char *)); if (!m->mod_values) { kerr = ENOMEM; goto done; @@ -2522,7 +2535,8 @@ krb5_error_code ipadb_delete_principal(krb5_context kcontext, } flags = KRB5_KDB_FLAG_ALIAS_OK; - kerr = ipadb_find_principal(kcontext, flags, res, &canonicalized, &lentry); + kerr = ipadb_find_principal(kcontext, flags, res, &canonicalized, + &lentry); if (kerr != 0) { goto done; } diff --git a/daemons/ipa-kdb/tests/ipa_kdb_tests.c b/daemons/ipa-kdb/tests/ipa_kdb_tests.c index 7ebcb9b965..8d7216b2ae 100644 --- a/daemons/ipa-kdb/tests/ipa_kdb_tests.c +++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c @@ -237,7 +237,8 @@ void test_filter_logon_info(void **state) } test_data[] = { /* only allowed SIDs */ {3, {DOM_SID_TRUST"-1000", DOM_SID_TRUST"-1001", DOM_SID_TRUST"-1002"}, - 3, {DOM_SID_TRUST"-1000", DOM_SID_TRUST"-1001", DOM_SID_TRUST"-1002"}}, + 3, {DOM_SID_TRUST"-1000", DOM_SID_TRUST"-1001", DOM_SID_TRUST"-1002"} + }, /* last SID filtered */ {3, {DOM_SID_TRUST"-1000", DOM_SID_TRUST"-1001", BLACKLIST_SID"-1002"}, 2, {DOM_SID_TRUST"-1000", DOM_SID_TRUST"-1001"}}, @@ -250,7 +251,7 @@ void test_filter_logon_info(void **state) /* first and last SID filtered */ {3, {BLACKLIST_SID"-1000", DOM_SID_TRUST"-1001", BLACKLIST_SID"-1002"}, 1, {DOM_SID_TRUST"-1001"}}, - /* two SIDs in a rwo filtered */ + /* two SIDs in a row filtered */ {3, {BLACKLIST_SID"-1000", BLACKLIST_SID"-1001", DOM_SID_TRUST"-1002"}, 1, {DOM_SID_TRUST"-1002"}}, /* all SIDs filtered*/ @@ -263,13 +264,13 @@ void test_filter_logon_info(void **state) talloc_free(info->info->info3.sids); info->info->info3.sidcount = test_data[c].sidcount; - info->info->info3.sids = talloc_zero_array(info->info, - struct netr_SidAttr, - info->info->info3.sidcount); + info->info->info3.sids = talloc_zero_array( + info->info, struct netr_SidAttr, + info->info->info3.sidcount); assert_non_null(info->info->info3.sids); for(d = 0; d < info->info->info3.sidcount; d++) { - info->info->info3.sids[d].sid = talloc_zero(info->info->info3.sids, - struct dom_sid2); + info->info->info3.sids[d].sid = talloc_zero( + info->info->info3.sids, struct dom_sid2); assert_non_null(info->info->info3.sids[d].sid); } @@ -281,14 +282,16 @@ void test_filter_logon_info(void **state) kerr = filter_logon_info(test_ctx->krb5_ctx, NULL, realm, info); assert_int_equal(kerr, 0); - assert_int_equal(info->info->info3.sidcount, test_data[c].exp_sidcount); + assert_int_equal(info->info->info3.sidcount, + test_data[c].exp_sidcount); if (test_data[c].exp_sidcount == 0) { assert_null(info->info->info3.sids); } else { for (d = 0; d < test_data[c].exp_sidcount; d++) { - assert_string_equal(test_data[c].exp_sids[d], - dom_sid_string(info->info->info3.sids, - info->info->info3.sids[d].sid)); + assert_string_equal( + test_data[c].exp_sids[d], + dom_sid_string(info->info->info3.sids, + info->info->info3.sids[d].sid)); } } } @@ -368,28 +371,50 @@ void test_get_authz_data_types(void **state) {ad_none_only, NULL, NULL, false, false, "with only NONE in entry"}, {ad_pac_only, NULL, NULL, true, false, "with only MS-PAC in entry"}, {ad_pad_only, NULL, NULL, false, true, "with only PAD in entry"}, - {ad_illegal_only, NULL, NULL, false, false, "with only an invalid value in entry"}, - {ad_pac_and_pad, NULL, NULL, true, true, "with MS-PAC and PAD in entry"}, - {ad_pac_and_none, NULL, NULL, false, false, "with MS-PAC and NONE in entry"}, - {ad_none_and_pad, NULL, NULL, false, false, "with NONE and PAD in entry"}, - {NULL, ad_none_only, NULL, false, false, "with only NONE in global config"}, - {NULL, ad_pac_only, NULL, true, false, "with only MS-PAC in global config"}, - {NULL, ad_pad_only, NULL, false, true, "with only PAD in global config"}, - {NULL, ad_illegal_only, NULL, false, false, "with only an invalid value in global config"}, - {NULL, ad_pac_and_pad, NULL, true, true, "with MS-PAC and PAD in global config"}, - {NULL, ad_pac_and_none, NULL, false, false, "with MS-PAC and NONE in global config"}, - {NULL, ad_none_and_pad, NULL, false, false, "with NONE and PAD in global entry"}, - {NULL, ad_global_pac_nfs_none, NULL, true, false, "with NULL principal and PAC and nfs:NONE in global entry"}, - {NULL, ad_global_pac_nfs_none, nfs_princ, false, false, "with nfs principal and PAC and nfs:NONE in global entry"}, - {NULL, ad_global_pac_nfs_none, non_nfs_princ, true, false, "with non-nfs principal and PAC and nfs:NONE in global entry"}, - {NULL, ad_global_pac_nfs_pad, NULL, true, false, "with NULL principal and PAC and nfs:PAD in global entry"}, - {NULL, ad_global_pac_nfs_pad, nfs_princ, false, true, "with nfs principal and PAC and nfs:PAD in global entry"}, - {NULL, ad_global_pac_nfs_pad, non_nfs_princ, true, false, "with non-nfs principal and PAC and nfs:PAD in global entry"}, - {ad_none_only, ad_pac_only, NULL, false, false, "with NONE overriding PAC in global entry"}, - {ad_pad_only, ad_pac_only, NULL, false, true, "with PAC overriding PAC in global entry"}, - {ad_illegal_only, ad_pac_only, NULL, false, false, "with invalid value overriding PAC in global entry"}, - {ad_pac_and_pad, ad_pac_only, NULL, true, true, "with PAC and PAD overriding PAC in global entry"}, - {ad_none_and_pad, ad_pac_only, NULL, false, false, "with NONE and PAD overriding PAC in global entry"}, + {ad_illegal_only, NULL, NULL, false, false, + "with only an invalid value in entry"}, + {ad_pac_and_pad, NULL, NULL, true, true, + "with MS-PAC and PAD in entry"}, + {ad_pac_and_none, NULL, NULL, false, false, + "with MS-PAC and NONE in entry"}, + {ad_none_and_pad, NULL, NULL, false, false, + "with NONE and PAD in entry"}, + {NULL, ad_none_only, NULL, false, false, + "with only NONE in global config"}, + {NULL, ad_pac_only, NULL, true, false, + "with only MS-PAC in global config"}, + {NULL, ad_pad_only, NULL, false, true, + "with only PAD in global config"}, + {NULL, ad_illegal_only, NULL, false, false, + "with only an invalid value in global config"}, + {NULL, ad_pac_and_pad, NULL, true, true, + "with MS-PAC and PAD in global config"}, + {NULL, ad_pac_and_none, NULL, false, false, + "with MS-PAC and NONE in global config"}, + {NULL, ad_none_and_pad, NULL, false, false, + "with NONE and PAD in global entry"}, + {NULL, ad_global_pac_nfs_none, NULL, true, false, + "with NULL principal and PAC and nfs:NONE in global entry"}, + {NULL, ad_global_pac_nfs_none, nfs_princ, false, false, + "with nfs principal and PAC and nfs:NONE in global entry"}, + {NULL, ad_global_pac_nfs_none, non_nfs_princ, true, false, + "with non-nfs principal and PAC and nfs:NONE in global entry"}, + {NULL, ad_global_pac_nfs_pad, NULL, true, false, + "with NULL principal and PAC and nfs:PAD in global entry"}, + {NULL, ad_global_pac_nfs_pad, nfs_princ, false, true, + "with nfs principal and PAC and nfs:PAD in global entry"}, + {NULL, ad_global_pac_nfs_pad, non_nfs_princ, true, false, + "with non-nfs principal and PAC and nfs:PAD in global entry"}, + {ad_none_only, ad_pac_only, NULL, false, false, + "with NONE overriding PAC in global entry"}, + {ad_pad_only, ad_pac_only, NULL, false, true, + "with PAC overriding PAC in global entry"}, + {ad_illegal_only, ad_pac_only, NULL, false, false, + "with invalid value overriding PAC in global entry"}, + {ad_pac_and_pad, ad_pac_only, NULL, true, true, + "with PAC and PAD overriding PAC in global entry"}, + {ad_none_and_pad, ad_pac_only, NULL, false, false, + "with NONE and PAD overriding PAC in global entry"}, {NULL, NULL, NULL, false, false, NULL} }; From 362dd8a6d49515da8d2c782e006a2701c0cb363c Mon Sep 17 00:00:00 2001 From: Robbie Harwood <rharw...@redhat.com> Date: Fri, 1 Nov 2019 16:48:55 -0400 Subject: [PATCH 6/6] [KDB] Use separate variable for client fetch in kdcpolicy `client` is not intended to be modified as a parameter of the AS check function. Fixes an "incompatible pointer type" compiler warning. Signed-off-by: Robbie Harwood <rharw...@redhat.com> --- daemons/ipa-kdb/ipa_kdb_kdcpolicy.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb_kdcpolicy.c b/daemons/ipa-kdb/ipa_kdb_kdcpolicy.c index d148edb2d1..d06bd0ae91 100644 --- a/daemons/ipa-kdb/ipa_kdb_kdcpolicy.c +++ b/daemons/ipa-kdb/ipa_kdb_kdcpolicy.c @@ -22,6 +22,7 @@ ipa_kdcpolicy_check_as(krb5_context context, krb5_kdcpolicy_moddata moddata, enum ipadb_user_auth ua; struct ipadb_e_data *ied; int valid_auth_indicators = 0; + krb5_db_entry *client_actual = NULL; *status = NULL; *lifetime_out = 0; @@ -36,14 +37,14 @@ ipa_kdcpolicy_check_as(krb5_context context, krb5_kdcpolicy_moddata moddata, LOG_INFO, "IPA kdcpolicy: client e_data not availble. Try fetching..."); kerr = ipadb_get_principal(context, request->client, - KRB5_KDB_FLAG_ALIAS_OK, &client); + KRB5_KDB_FLAG_ALIAS_OK, &client_actual); if (kerr != 0) { krb5_klog_syslog(LOG_ERR, "IPA kdcpolicy: ipadb_find_principal failed."); return kerr; } - ied = (struct ipadb_e_data *)client->e_data; + ied = (struct ipadb_e_data *)client_actual->e_data; if (ied == NULL && ied->magic != IPA_E_DATA_MAGIC) { krb5_klog_syslog(LOG_ERR, "IPA kdcpolicy: client e_data fetching failed.");
_______________________________________________ FreeIPA-devel mailing list -- freeipa-devel@lists.fedorahosted.org To unsubscribe send an email to freeipa-devel-le...@lists.fedorahosted.org Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedorahosted.org/archives/list/freeipa-devel@lists.fedorahosted.org
