From 6cfb9b7193aeb87e372af42e1d589486625ee34e Mon Sep 17 00:00:00 2001 From: Alexander Bokovoy Date: Tue, 28 Sep 2021 10:24:32 +0300 Subject: [PATCH] ipa-kdb: store SID in the principal entry If the principal entry in LDAP has SID associated with it, store it to be able to quickly assess the SID when processing PAC. Also rename string_to_sid to IPA-specific version as it uses different prototype than Samba version. Fixes: https://pagure.io/freeipa/issue/9031 Signed-off-by: Alexander Bokovoy Reviewed-by: Andreas Schneider Reviewed-by: Robert Crittenden Reviewed-By: Rob Crittenden --- daemons/ipa-kdb/ipa_kdb.h | 7 ++++++ daemons/ipa-kdb/ipa_kdb_mspac.c | 31 ++++++++++++++++++------- daemons/ipa-kdb/ipa_kdb_mspac_private.h | 1 - daemons/ipa-kdb/ipa_kdb_principals.c | 25 ++++++++++++++++++++ daemons/ipa-kdb/tests/ipa_kdb_tests.c | 30 ++++++++++++------------ 5 files changed, 69 insertions(+), 25 deletions(-) diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h index 66a1d74f1..884dff950 100644 --- a/daemons/ipa-kdb/ipa_kdb.h +++ b/daemons/ipa-kdb/ipa_kdb.h @@ -79,6 +79,7 @@ #define IPA_USER_AUTH_TYPE "ipaUserAuthType" struct ipadb_mspac; +struct dom_sid; enum ipadb_user_auth { IPADB_USER_AUTH_NONE = 0, @@ -155,6 +156,8 @@ struct ipadb_e_data { bool has_tktpolaux; enum ipadb_user_auth user_auth; struct ipadb_e_pol_limits pol_limits[IPADB_USER_AUTH_IDX_MAX]; + bool has_sid; + struct dom_sid *sid; }; struct ipadb_context *ipadb_get_context(krb5_context kcontext); @@ -366,3 +369,7 @@ int ipadb_get_enc_salt_types(struct ipadb_context *ipactx, LDAPMessage *entry, /* CERTAUTH PLUGIN */ void ipa_certauth_free_moddata(krb5_certauth_moddata *moddata); #endif + +int ipadb_string_to_sid(const char *str, struct dom_sid *sid); +void alloc_sid(struct dom_sid **sid); +void free_sid(struct dom_sid **sid); \ No newline at end of file diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c index 47b12a16f..f3e8657c2 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac.c +++ b/daemons/ipa-kdb/ipa_kdb_mspac.c @@ -80,7 +80,20 @@ static char *memberof_pac_attrs[] = { #define AUTHZ_DATA_TYPE_PAD "PAD" #define AUTHZ_DATA_TYPE_NONE "NONE" -int string_to_sid(const char *str, struct dom_sid *sid) +void alloc_sid(struct dom_sid **sid) +{ + *sid = malloc(sizeof(struct dom_sid)); +} + +void free_sid(struct dom_sid **sid) +{ + if (sid != NULL && *sid != NULL) { + free(*sid); + *sid = NULL; + } +} + +int ipadb_string_to_sid(const char *str, struct dom_sid *sid) { unsigned long val; const char *s; @@ -372,7 +385,7 @@ static krb5_error_code ipadb_add_asserted_identity(struct ipadb_context *ipactx, /* For S4U2Self, add Service Asserted Identity SID * otherwise, add Authentication Authority Asserted Identity SID */ - ret = string_to_sid((flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION) ? + ret = ipadb_string_to_sid((flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION) ? "S-1-18-2" : "S-1-18-1", arr[sidcount].sid); if (ret) { @@ -655,7 +668,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx, /* SID is mandatory */ return ret; } - ret = string_to_sid(strres, &sid); + ret = ipadb_string_to_sid(strres, &sid); free(strres); if (ret) { return ret; @@ -700,7 +713,7 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx, } } if (strcasecmp(dval->type, "ipaNTSecurityIdentifier") == 0) { - ret = string_to_sid((char *)dval->vals[0].bv_val, &gsid); + ret = ipadb_string_to_sid((char *)dval->vals[0].bv_val, &gsid); if (ret) { continue; } @@ -1189,7 +1202,7 @@ static int map_groups(TALLOC_CTX *memctx, krb5_context kcontext, } if (strcasecmp(dval->type, "ipaNTSecurityIdentifier") == 0) { - kerr = string_to_sid((char *)dval->vals[0].bv_val, &sid); + kerr = ipadb_string_to_sid((char *)dval->vals[0].bv_val, &sid); if (kerr != 0) { continue; } @@ -2434,7 +2447,7 @@ ipadb_adtrusts_fill_sid_blacklist(char **source_sid_blacklist, } for (i = 0; i < len; i++) { - (void) string_to_sid(source[i], &sid_blacklist[i]); + (void) ipadb_string_to_sid(source[i], &sid_blacklist[i]); } *result_sids = sid_blacklist; @@ -2594,7 +2607,7 @@ ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx) goto done; } - ret = string_to_sid(t[n].domain_sid, &t[n].domsid); + ret = ipadb_string_to_sid(t[n].domain_sid, &t[n].domsid); if (ret && t[n].domain_sid != NULL) { ret = EINVAL; goto done; @@ -2812,7 +2825,7 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx, bool force_rein goto done; } - ret = string_to_sid(resstr, &ipactx->mspac->domsid); + ret = ipadb_string_to_sid(resstr, &ipactx->mspac->domsid); if (ret) { kerr = ret; free(resstr); @@ -2865,7 +2878,7 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx, bool force_rein goto done; } if (ret == 0) { - ret = string_to_sid(resstr, &gsid); + ret = ipadb_string_to_sid(resstr, &gsid); if (ret) { free(resstr); kerr = ret; diff --git a/daemons/ipa-kdb/ipa_kdb_mspac_private.h b/daemons/ipa-kdb/ipa_kdb_mspac_private.h index 8c8a3a001..3696c3c6c 100644 --- a/daemons/ipa-kdb/ipa_kdb_mspac_private.h +++ b/daemons/ipa-kdb/ipa_kdb_mspac_private.h @@ -51,7 +51,6 @@ struct ipadb_adtrusts { size_t *upn_suffixes_len; }; -int string_to_sid(const char *str, struct dom_sid *sid); char *dom_sid_string(TALLOC_CTX *memctx, const struct dom_sid *dom_sid); krb5_error_code filter_logon_info(krb5_context context, TALLOC_CTX *memctx, krb5_data realm, struct PAC_LOGON_INFO_CTR *info); diff --git a/daemons/ipa-kdb/ipa_kdb_principals.c b/daemons/ipa-kdb/ipa_kdb_principals.c index 0a98ff054..15f3df4fe 100644 --- a/daemons/ipa-kdb/ipa_kdb_principals.c +++ b/daemons/ipa-kdb/ipa_kdb_principals.c @@ -79,6 +79,8 @@ static char *std_principal_attrs[] = { "ipatokenRadiusConfigLink", "krbAuthIndMaxTicketLife", "krbAuthIndMaxRenewableAge", + "ipaNTSecurityIdentifier", + "ipaUniqueID", "objectClass", NULL @@ -594,6 +596,7 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext, char *restring; char *uidstring; char **authz_data_list; + char *princ_sid; krb5_timestamp restime; bool resbool; int result; @@ -963,6 +966,27 @@ static krb5_error_code ipadb_parse_ldap_entry(krb5_context kcontext, ipadb_parse_authind_policies(kcontext, lcontext, lentry, entry, ua); } + /* Add SID if it is associated with the principal account */ + ied->has_sid = false; + ied->sid = NULL; + ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry, + "ipaNTSecurityIdentifier", &princ_sid); + if (ret == 0 && princ_sid != NULL) { + alloc_sid(&ied->sid); + if (ied->sid == NULL) { + kerr = KRB5_KDB_INTERNAL_ERROR; + free(princ_sid); + goto done; + } + ret = ipadb_string_to_sid(princ_sid, ied->sid); + free(princ_sid); + if (ret != 0) { + kerr = ret; + goto done; + } + ied->has_sid = true; + } + kerr = 0; done: @@ -1568,6 +1592,7 @@ void ipadb_free_principal_e_data(krb5_context kcontext, krb5_octet *e_data) } free(ied->authz_data); free(ied->pol); + free_sid(&ied->sid); free(ied); } } diff --git a/daemons/ipa-kdb/tests/ipa_kdb_tests.c b/daemons/ipa-kdb/tests/ipa_kdb_tests.c index 0b51ffb96..d38dfd841 100644 --- a/daemons/ipa-kdb/tests/ipa_kdb_tests.c +++ b/daemons/ipa-kdb/tests/ipa_kdb_tests.c @@ -105,7 +105,7 @@ static int setup(void **state) /* make sure data is not read from LDAP */ ipa_ctx->mspac->last_update = time(NULL) - 1; - ret = string_to_sid(DOM_SID, &ipa_ctx->mspac->domsid); + ret = ipadb_string_to_sid(DOM_SID, &ipa_ctx->mspac->domsid); assert_int_equal(ret, 0); ipa_ctx->mspac->num_trusts = 1; @@ -121,7 +121,7 @@ static int setup(void **state) ipa_ctx->mspac->trusts[0].domain_sid = strdup(DOM_SID_TRUST); assert_non_null(ipa_ctx->mspac->trusts[0].domain_sid); - ret = string_to_sid(DOM_SID_TRUST, &ipa_ctx->mspac->trusts[0].domsid); + ret = ipadb_string_to_sid(DOM_SID_TRUST, &ipa_ctx->mspac->trusts[0].domsid); assert_int_equal(ret, 0); ipa_ctx->mspac->trusts[0].len_sid_blocklist_incoming = 1; @@ -129,7 +129,7 @@ static int setup(void **state) ipa_ctx->mspac->trusts[0].len_sid_blocklist_incoming, sizeof(struct dom_sid)); assert_non_null(ipa_ctx->mspac->trusts[0].sid_blocklist_incoming); - ret = string_to_sid(BLOCKLIST_SID, + ret = ipadb_string_to_sid(BLOCKLIST_SID, &ipa_ctx->mspac->trusts[0].sid_blocklist_incoming[0]); assert_int_equal(ret, 0); @@ -216,7 +216,7 @@ static void test_filter_logon_info(void **state) assert_int_equal(kerr, EINVAL); /* wrong domain SID */ - ret = string_to_sid("S-1-5-21-1-1-1", &dom_sid); + ret = ipadb_string_to_sid("S-1-5-21-1-1-1", &dom_sid); assert_int_equal(ret, 0); info->info->info3.base.domain_sid = &dom_sid; @@ -224,7 +224,7 @@ static void test_filter_logon_info(void **state) assert_int_equal(kerr, EINVAL); /* matching domain SID */ - ret = string_to_sid(DOM_SID_TRUST, &dom_sid); + ret = ipadb_string_to_sid(DOM_SID_TRUST, &dom_sid); assert_int_equal(ret, 0); info->info->info3.base.domain_sid = &dom_sid; @@ -292,7 +292,7 @@ static void test_filter_logon_info(void **state) } for (d = 0; d < info->info->info3.sidcount; d++) { - ret = string_to_sid(test_data[c].sids[d], + ret = ipadb_string_to_sid(test_data[c].sids[d], info->info->info3.sids[d].sid); assert_int_equal(ret, 0); } @@ -434,7 +434,7 @@ static void test_get_authz_data_types(void **state) krb5_free_principal(test_ctx->krb5_ctx, non_nfs_princ); } -static void test_string_to_sid(void **state) +static void test_ipadb_string_to_sid(void **state) { int ret; struct dom_sid sid; @@ -442,25 +442,25 @@ static void test_string_to_sid(void **state) {21, 2127521184, 1604012920, 1887927527, 72713, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; - ret = string_to_sid(NULL, &sid); + ret = ipadb_string_to_sid(NULL, &sid); assert_int_equal(ret, EINVAL); - ret = string_to_sid("abc", &sid); + ret = ipadb_string_to_sid("abc", &sid); assert_int_equal(ret, EINVAL); - ret = string_to_sid("S-", &sid); + ret = ipadb_string_to_sid("S-", &sid); assert_int_equal(ret, EINVAL); - ret = string_to_sid("S-ABC", &sid); + ret = ipadb_string_to_sid("S-ABC", &sid); assert_int_equal(ret, EINVAL); - ret = string_to_sid("S-123", &sid); + ret = ipadb_string_to_sid("S-123", &sid); assert_int_equal(ret, EINVAL); - ret = string_to_sid("S-1-123-1-2-3-4-5-6-7-8-9-0-1-2-3-4-5-6", &sid); + ret = ipadb_string_to_sid("S-1-123-1-2-3-4-5-6-7-8-9-0-1-2-3-4-5-6", &sid); assert_int_equal(ret, EINVAL); - ret = string_to_sid("S-1-5-21-2127521184-1604012920-1887927527-72713", + ret = ipadb_string_to_sid("S-1-5-21-2127521184-1604012920-1887927527-72713", &sid); assert_int_equal(ret, 0); assert_memory_equal(&exp_sid, &sid, sizeof(struct dom_sid)); @@ -531,7 +531,7 @@ int main(int argc, const char *argv[]) setup, teardown), cmocka_unit_test_setup_teardown(test_filter_logon_info, setup, teardown), - cmocka_unit_test(test_string_to_sid), + cmocka_unit_test(test_ipadb_string_to_sid), cmocka_unit_test_setup_teardown(test_dom_sid_string, setup, teardown), cmocka_unit_test_setup_teardown(test_check_trusted_realms,