This patchset is about Ticket #2849
The point is to verify that the PAC information we are getting from a
trusted realm is actually consistent with the information we know about
that trust relationship.
The patchset adds a way to load trust information in the kdb driver
(first 2 patches), reorganizes a bit the code around PAC verification
and adds a filtering function to match realm with AD and SID data.
Tested on my trust environment and seem to work fine.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
>From 30f30901d99fb827016bee6e3f2a7d7dde7a460d Mon Sep 17 00:00:00 2001
From: Simo Sorce <sso...@redhat.com>
Date: Mon, 9 Jul 2012 09:15:51 -0400
Subject: [PATCH 1/5] Move mspac structure to be a private pointer
By keeping it's definition in the mspac file it is easier to modify and make
sure any opertion on it is handled in the same file.
---
daemons/ipa-kdb/ipa_kdb.h | 9 ++-----
daemons/ipa-kdb/ipa_kdb_mspac.c | 49 +++++++++++++++++++++++++--------------
2 files changed, 33 insertions(+), 25 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb.h b/daemons/ipa-kdb/ipa_kdb.h
index c1cc7a7d8ecdf86b10606233078abbb8685f6750..0a179dbcf0e9c17c0eb468638cd7436dc60d31a5 100644
--- a/daemons/ipa-kdb/ipa_kdb.h
+++ b/daemons/ipa-kdb/ipa_kdb.h
@@ -74,12 +74,7 @@
#define IPA_SETUP "ipa-setup-override-restrictions"
-struct ipadb_wincompat {
- char *flat_domain_name;
- char *flat_server_name;
- char *fallback_group;
- uint32_t fallback_rid;
-};
+struct ipadb_mspac;
struct ipadb_context {
char *uri;
@@ -91,7 +86,7 @@ struct ipadb_context {
bool override_restrictions;
krb5_key_salt_tuple *supp_encs;
int n_supp_encs;
- struct ipadb_wincompat wc;
+ struct ipadb_mspac *mspac;
bool disable_last_success;
bool disable_lockout;
};
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 1c7487c3c8f75d02466a2e0746fbef5d36e3d995..44cf522a00e4973077d716a9545f69f325e870ba 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -26,6 +26,13 @@
#include "util/time.h"
#include "gen_ndr/ndr_krb5pac.h"
+struct ipadb_mspac {
+ char *flat_domain_name;
+ char *flat_server_name;
+ char *fallback_group;
+ uint32_t fallback_rid;
+};
+
int krb5_klog_syslog(int, const char *, ...);
@@ -460,8 +467,8 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
}
if (info3->base.primary_gid == 0) {
- if (ipactx->wc.fallback_rid) {
- info3->base.primary_gid = ipactx->wc.fallback_rid;
+ if (ipactx->mspac->fallback_rid) {
+ info3->base.primary_gid = ipactx->mspac->fallback_rid;
} else {
/* can't give a pack without a primary group rid */
return ENOENT;
@@ -474,9 +481,9 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
/* always zero out, not used for Krb, only NTLM */
memset(&info3->base.key, '\0', sizeof(info3->base.key));
- if (ipactx->wc.flat_server_name) {
+ if (ipactx->mspac->flat_server_name) {
info3->base.logon_server.string =
- talloc_strdup(memctx, ipactx->wc.flat_server_name);
+ talloc_strdup(memctx, ipactx->mspac->flat_server_name);
if (!info3->base.logon_server.string) {
return ENOMEM;
}
@@ -485,9 +492,9 @@ static krb5_error_code ipadb_fill_info3(struct ipadb_context *ipactx,
return ENOENT;
}
- if (ipactx->wc.flat_domain_name) {
+ if (ipactx->mspac->flat_domain_name) {
info3->base.logon_domain.string =
- talloc_strdup(memctx, ipactx->wc.flat_domain_name);
+ talloc_strdup(memctx, ipactx->mspac->flat_domain_name);
if (!info3->base.logon_domain.string) {
return ENOMEM;
}
@@ -1318,11 +1325,17 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx)
int ret;
/* clean up in case we had old values around */
- free(ipactx->wc.flat_domain_name);
- ipactx->wc.flat_domain_name = NULL;
- free(ipactx->wc.fallback_group);
- ipactx->wc.fallback_group = NULL;
- ipactx->wc.fallback_rid = 0;
+ if (ipactx->mspac) {
+ free(ipactx->mspac->flat_domain_name);
+ free(ipactx->mspac->fallback_group);
+ free(ipactx->mspac);
+ }
+
+ ipactx->mspac = calloc(1, sizeof(struct ipadb_mspac));
+ if (!ipactx->mspac) {
+ kerr = ENOMEM;
+ goto done;
+ }
kerr = ipadb_simple_search(ipactx, ipactx->base, LDAP_SCOPE_SUBTREE,
"(objectclass=ipaNTDomainAttrs)", dom_attrs,
@@ -1341,22 +1354,22 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx)
ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
"ipaNTFlatName",
- &ipactx->wc.flat_domain_name);
+ &ipactx->mspac->flat_domain_name);
if (ret) {
kerr = ret;
goto done;
}
- free(ipactx->wc.flat_server_name);
- ipactx->wc.flat_server_name = get_server_netbios_name();
- if (!ipactx->wc.flat_server_name) {
+ free(ipactx->mspac->flat_server_name);
+ ipactx->mspac->flat_server_name = get_server_netbios_name();
+ if (!ipactx->mspac->flat_server_name) {
kerr = ENOMEM;
goto done;
}
ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
"ipaNTFallbackPrimaryGroup",
- &ipactx->wc.fallback_group);
+ &ipactx->mspac->fallback_group);
if (ret && ret != ENOENT) {
kerr = ret;
goto done;
@@ -1368,7 +1381,7 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx)
lentry = NULL;
if (ret != ENOENT) {
- kerr = ipadb_simple_search(ipactx, ipactx->wc.fallback_group,
+ kerr = ipadb_simple_search(ipactx, ipactx->mspac->fallback_group,
LDAP_SCOPE_BASE,
"(objectclass=posixGroup)",
grp_attrs, &result);
@@ -1397,7 +1410,7 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx)
kerr = ret;
goto done;
}
- ret = sid_split_rid(&gsid, &ipactx->wc.fallback_rid);
+ ret = sid_split_rid(&gsid, &ipactx->mspac->fallback_rid);
if (ret) {
kerr = ret;
goto done;
--
1.7.10.4
>From 5ef0c638327a82c945de619d1e7771ac0d957a9a Mon Sep 17 00:00:00 2001
From: Simo Sorce <sso...@redhat.com>
Date: Tue, 10 Jul 2012 10:50:14 -0400
Subject: [PATCH 2/5] Load list of trusted domain on connecting to ldap
This list is used to validate data in mspac filtering
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 110 ++++++++++++++++++++++++++++++++++++---
1 file changed, 104 insertions(+), 6 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 44cf522a00e4973077d716a9545f69f325e870ba..2ed093d30a0fea20ef620b8df9858ec4802d1191 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -26,11 +26,20 @@
#include "util/time.h"
#include "gen_ndr/ndr_krb5pac.h"
+struct ipadb_adtrusts {
+ char *domain_name;
+ char *flat_name;
+ char *domain_sid;
+};
+
struct ipadb_mspac {
char *flat_domain_name;
char *flat_server_name;
char *fallback_group;
uint32_t fallback_rid;
+
+ int num_trusts;
+ struct ipadb_adtrusts *trusts;
};
@@ -1311,6 +1320,99 @@ static char *get_server_netbios_name(void)
return strdup(hostname);
}
+void ipadb_mspac_struct_free(struct ipadb_mspac **mspac)
+{
+ int i;
+
+ if (!*mspac) return;
+
+ free((*mspac)->flat_domain_name);
+ free((*mspac)->fallback_group);
+
+ if ((*mspac)->num_trusts) {
+ for (i = 0; i < (*mspac)->num_trusts; i++) {
+ free((*mspac)->trusts[i].domain_name);
+ free((*mspac)->trusts[i].flat_name);
+ free((*mspac)->trusts[i].domain_sid);
+ }
+ }
+
+ *mspac = NULL;
+}
+
+krb5_error_code ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx)
+{
+ struct ipadb_adtrusts *t;
+ LDAP *lc = ipactx->lcontext;
+ char *attrs[] = { "ipaNTTrustPartner", "ipaNTFlatName",
+ "ipaNTTrustedDomainSID", NULL };
+ char *filter = "(objectclass=ipaNTTrustedDomain)";
+ krb5_error_code kerr;
+ LDAPMessage *res = NULL;
+ LDAPMessage *le;
+ char *base = NULL;
+ int ret, n;
+
+ ret = asprintf(&base, "cn=ad,cn=trusts,%s", ipactx->base);
+ if (ret == -1) {
+ ret = ENOMEM;
+ goto done;
+ }
+
+ kerr = ipadb_simple_search(ipactx, base, LDAP_SCOPE_SUBTREE,
+ filter, attrs, &res);
+ if (kerr == KRB5_KDB_NOENTRY) {
+ /* nothing to do, there are no trusts */
+ ret = 0;
+ goto done;
+ } else if (kerr != 0) {
+ ret = EIO;
+ goto done;
+ }
+
+ for (le = ldap_first_entry(lc, res); le; le = ldap_next_entry(lc, le)) {
+ n = ipactx->mspac->num_trusts;
+ ipactx->mspac->num_trusts++;
+ t = realloc(ipactx->mspac->trusts,
+ sizeof(struct ipadb_adtrusts) * ipactx->mspac->num_trusts);
+ if (!t) {
+ ret = ENOMEM;
+ goto done;
+ }
+ ipactx->mspac->trusts = t;
+
+ ret = ipadb_ldap_attr_to_str(lc, le, "ipaNTTrustPartner",
+ &t[n].domain_name);
+ if (ret) {
+ ret = EINVAL;
+ goto done;
+ }
+
+ ret = ipadb_ldap_attr_to_str(lc, le, "ipaNTFlatName",
+ &t[n].flat_name);
+ if (ret) {
+ ret = EINVAL;
+ goto done;
+ }
+
+ ret = ipadb_ldap_attr_to_str(lc, le, "ipaNTTrustedDomainSID",
+ &t[n].domain_sid);
+ if (ret) {
+ ret = EINVAL;
+ goto done;
+ }
+ }
+
+ ret = 0;
+
+done:
+ if (ret != 0) {
+ krb5_klog_syslog(LOG_ERR, "Failed to read list of trusted domains");
+ }
+ free(base);
+ return ret;
+}
+
krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx)
{
char *dom_attrs[] = { "ipaNTFlatName",
@@ -1325,11 +1427,7 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx)
int ret;
/* clean up in case we had old values around */
- if (ipactx->mspac) {
- free(ipactx->mspac->flat_domain_name);
- free(ipactx->mspac->fallback_group);
- free(ipactx->mspac);
- }
+ ipadb_mspac_struct_free(&ipactx->mspac);
ipactx->mspac = calloc(1, sizeof(struct ipadb_mspac));
if (!ipactx->mspac) {
@@ -1419,7 +1517,7 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx)
}
}
- kerr = 0;
+ kerr = ipadb_mspac_get_trusted_domains(ipactx);
done:
ldap_msgfree(result);
--
1.7.10.4
>From 699df40886b2c20abcc5b754b620a686df1314b7 Mon Sep 17 00:00:00 2001
From: Simo Sorce <sso...@redhat.com>
Date: Fri, 13 Jul 2012 12:02:06 -0400
Subject: [PATCH 3/5] Properly name function to add ipa external groups
The function filter_pac was not filtering the pac at all, it was merely
augmenting it with additional data relevant to the IPA server.
Change the name of the function to avoid confusion.
While there I also simplified and cleaed up the code a bit with regard to
variable names and usage.
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 74 +++++++++++++++++++++------------------
1 file changed, 39 insertions(+), 35 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 2ed093d30a0fea20ef620b8df9858ec4802d1191..7e6e71d5b316022cc53438a67dfd3ec4595f0245 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -900,8 +900,8 @@ done:
return kerr;
}
-static krb5_error_code filter_pac(krb5_context context, krb5_data *old_data,
- krb5_data *new_data)
+static krb5_error_code add_local_groups(krb5_context context,
+ krb5_data *pac_blob)
{
DATA_BLOB pac_data;
union PAC_INFO pac_info;
@@ -918,8 +918,8 @@ static krb5_error_code filter_pac(krb5_context context, krb5_data *old_data,
return ENOMEM;
}
- pac_data.length = old_data->length;
- pac_data.data = (uint8_t *) old_data->data;
+ pac_data.length = pac_blob->length;
+ pac_data.data = (uint8_t *)pac_blob->data;
ndr_err = ndr_pull_union_blob(&pac_data, tmpctx, &pac_info,
PAC_TYPE_LOGON_INFO,
@@ -962,14 +962,15 @@ static krb5_error_code filter_pac(krb5_context context, krb5_data *old_data,
goto done;
}
- new_data->magic = KV5M_DATA;
- new_data->data = malloc(pac_data.length);
- if (new_data->data == NULL) {
+ free(pac_blob->data);
+ pac_blob->data = malloc(pac_data.length);
+ if (pac_blob->data == NULL) {
+ pac_blob->length = 0;
kerr = ENOMEM;
goto done;
}
- memcpy(new_data->data, pac_data.data, pac_data.length);
- new_data->length = pac_data.length;
+ memcpy(pac_blob->data, pac_data.data, pac_data.length);
+ pac_blob->length = pac_data.length;
kerr = 0;
@@ -993,12 +994,13 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
krb5_keyblock *srv_key = NULL;
krb5_keyblock *priv_key = NULL;
krb5_error_code kerr;
- krb5_ui_4 *buffer_types = NULL;
+ krb5_ui_4 *types = NULL;
size_t num_buffers;
krb5_pac old_pac = NULL;
krb5_pac new_pac = NULL;
krb5_data data;
- krb5_data filtered_data;
+ krb5_data pac_blob = { 0 , 0, NULL};
+ bool is_cross_realm = false;
size_t i;
kerr = krb5_pac_parse(context,
@@ -1009,7 +1011,6 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
goto done;
}
- memset(&filtered_data, 0, sizeof(filtered_data));
/* for cross realm trusts cases we need to check the right checksum.
* when the PAC is signed by our realm, we can always just check it
* passing our realm krbtgt key as the kdc checksum key (privsvr).
@@ -1018,6 +1019,7 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
* realm krbtgt to check the 'server' checksum instead. */
if (is_cross_realm_krbtgt(krbtgt->princ)) {
/* krbtgt from a trusted realm */
+ is_cross_realm = true;
/* FIXME:
* We must refuse a PAC that comes signed with a cross realm TGT
@@ -1028,15 +1030,6 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
/* TODO: Here is where we need to plug our PAC Filtering, later on */
srv_key = krbtgt_key;
- kerr = krb5_pac_get_buffer(context, old_pac, KRB5_PAC_LOGON_INFO, &data);
- if (kerr != 0) {
- goto done;
- }
-
- kerr = filter_pac(context, &data, &filtered_data);
- if (kerr != 0) {
- goto done;
- }
} else {
/* krbtgt from our own realm */
priv_key = krbtgt_key;
@@ -1048,6 +1041,20 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
goto done;
}
+ /* Now that the PAc is verified augment it with additional info if
+ * it is coming from a different realm */
+ if (is_cross_realm) {
+ kerr = krb5_pac_get_buffer(context, old_pac,
+ KRB5_PAC_LOGON_INFO, &pac_blob);
+ if (kerr != 0) {
+ goto done;
+ }
+
+ kerr = add_local_groups(context, &pac_blob);
+ if (kerr != 0) {
+ goto done;
+ }
+ }
/* extract buffers and rebuilt pac from scratch so that when re-signing
* with a different cksum type does not cause issues due to mismatching
* signature buffer lengths */
@@ -1056,22 +1063,20 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
goto done;
}
- kerr = krb5_pac_get_types(context, old_pac, &num_buffers, &buffer_types);
+ kerr = krb5_pac_get_types(context, old_pac, &num_buffers, &types);
if (kerr) {
goto done;
}
for (i = 0; i < num_buffers; i++) {
- if (buffer_types[i] == KRB5_PAC_SERVER_CHECKSUM ||
- buffer_types[i] == KRB5_PAC_PRIVSVR_CHECKSUM) {
+ if (types[i] == KRB5_PAC_SERVER_CHECKSUM ||
+ types[i] == KRB5_PAC_PRIVSVR_CHECKSUM) {
continue;
}
- if (buffer_types[i] == KRB5_PAC_LOGON_INFO &&
- filtered_data.length != 0) {
- kerr = krb5_pac_add_buffer(context, new_pac,
- buffer_types[i], &filtered_data);
- krb5_free_data_contents(context, &filtered_data);
+ if (types[i] == KRB5_PAC_LOGON_INFO &&
+ pac_blob.length != 0) {
+ kerr = krb5_pac_add_buffer(context, new_pac, types[i], &pac_blob);
if (kerr) {
krb5_pac_free(context, new_pac);
goto done;
@@ -1080,13 +1085,11 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
continue;
}
- kerr = krb5_pac_get_buffer(context, old_pac,
- buffer_types[i], &data);
+ kerr = krb5_pac_get_buffer(context, old_pac, types[i], &data);
if (kerr == 0) {
- kerr = krb5_pac_add_buffer(context, new_pac,
- buffer_types[i], &data);
+ kerr = krb5_pac_add_buffer(context, new_pac, types[i], &data);
+ krb5_free_data_contents(context, &data);
}
- krb5_free_data_contents(context, &data);
if (kerr) {
krb5_pac_free(context, new_pac);
goto done;
@@ -1098,7 +1101,8 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
done:
krb5_free_authdata(context, authdata);
krb5_pac_free(context, old_pac);
- free(buffer_types);
+ krb5_free_data_contents(context, &pac_blob);
+ free(types);
return kerr;
}
--
1.7.10.4
>From 32eb0dd87ac0b49ee4b6b58cb0c5e472eb660bee Mon Sep 17 00:00:00 2001
From: Simo Sorce <sso...@redhat.com>
Date: Fri, 13 Jul 2012 12:42:11 -0400
Subject: [PATCH 4/5] Split out manipulation of logon_info blob
This way multiple functions can manipulate the logon info structure until all
operations we want to do on it are done and then fold it back once.
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 117 ++++++++++++++++++++++++---------------
1 file changed, 73 insertions(+), 44 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 7e6e71d5b316022cc53438a67dfd3ec4595f0245..2a48c4f8ca7cee30d01380fbc12dddb928472963 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -900,83 +900,112 @@ done:
return kerr;
}
+static krb5_error_code get_logon_info(krb5_context context,
+ TALLOC_CTX *memctx,
+ krb5_data *pac_blob,
+ struct PAC_LOGON_INFO_CTR *info)
+{
+ DATA_BLOB pac_data;
+ enum ndr_err_code ndr_err;
+
+ pac_data.length = pac_blob->length;
+ pac_data.data = (uint8_t *)pac_blob->data;
+
+ ndr_err = ndr_pull_union_blob(&pac_data, memctx, info,
+ PAC_TYPE_LOGON_INFO,
+ (ndr_pull_flags_fn_t)ndr_pull_PAC_INFO);
+ if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
+ return KRB5_KDB_INTERNAL_ERROR;
+ }
+
+ return 0;
+}
+
static krb5_error_code add_local_groups(krb5_context context,
- krb5_data *pac_blob)
+ TALLOC_CTX *memctx,
+ struct PAC_LOGON_INFO_CTR *info)
{
- DATA_BLOB pac_data;
- union PAC_INFO pac_info;
- krb5_error_code kerr;
- enum ndr_err_code ndr_err;
- TALLOC_CTX *tmpctx;
int ret;
char **group_sids = NULL;
size_t ipa_group_sids_count = 0;
struct dom_sid *ipa_group_sids = NULL;
- tmpctx = talloc_new(NULL);
- if (!tmpctx) {
- return ENOMEM;
- }
-
- pac_data.length = pac_blob->length;
- pac_data.data = (uint8_t *)pac_blob->data;
-
- ndr_err = ndr_pull_union_blob(&pac_data, tmpctx, &pac_info,
- PAC_TYPE_LOGON_INFO,
- (ndr_pull_flags_fn_t) ndr_pull_PAC_INFO);
- if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
- krb5_klog_syslog(LOG_ERR, "ndr_pull_union_blob failed");
- kerr = KRB5_KDB_INTERNAL_ERROR;
- goto done;
- }
-
- ret = get_group_sids(tmpctx, &pac_info.logon_info, &group_sids);
+ ret = get_group_sids(memctx, info, &group_sids);
if (ret != 0) {
- krb5_klog_syslog(LOG_ERR, "get_group_sids failed");
- kerr = KRB5_KDB_INTERNAL_ERROR;
- goto done;
+ return KRB5_KDB_INTERNAL_ERROR;
}
- ret = map_groups(tmpctx, context, group_sids, &ipa_group_sids_count,
+ ret = map_groups(memctx, context, group_sids, &ipa_group_sids_count,
&ipa_group_sids);
if (ret != 0) {
- krb5_klog_syslog(LOG_ERR, "map_groups failed");
- kerr = KRB5_KDB_INTERNAL_ERROR;
- goto done;
+ return KRB5_KDB_INTERNAL_ERROR;
}
- ret = add_groups(tmpctx, &pac_info.logon_info, ipa_group_sids_count,
- ipa_group_sids);
+ ret = add_groups(memctx, info, ipa_group_sids_count, ipa_group_sids);
if (ret != 0) {
krb5_klog_syslog(LOG_ERR, "add_groups failed");
- kerr = KRB5_KDB_INTERNAL_ERROR;
- goto done;
+ return KRB5_KDB_INTERNAL_ERROR;
}
- ndr_err = ndr_push_union_blob(&pac_data, tmpctx, &pac_info,
+ return 0;
+}
+
+static krb5_error_code save_logon_info(krb5_context context,
+ TALLOC_CTX *memctx,
+ struct PAC_LOGON_INFO_CTR *info,
+ krb5_data *pac_blob)
+{
+ DATA_BLOB pac_data;
+ enum ndr_err_code ndr_err;
+
+ ndr_err = ndr_push_union_blob(&pac_data, memctx, info,
PAC_TYPE_LOGON_INFO,
(ndr_push_flags_fn_t)ndr_push_PAC_INFO);
if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) {
- krb5_klog_syslog(LOG_ERR, "ndr_push_union_blob failed");
- kerr = KRB5_KDB_INTERNAL_ERROR;
- goto done;
+ return KRB5_KDB_INTERNAL_ERROR;
}
free(pac_blob->data);
pac_blob->data = malloc(pac_data.length);
if (pac_blob->data == NULL) {
pac_blob->length = 0;
- kerr = ENOMEM;
- goto done;
+ return ENOMEM;
}
memcpy(pac_blob->data, pac_data.data, pac_data.length);
pac_blob->length = pac_data.length;
- kerr = 0;
+ return 0;
+}
+
+static krb5_error_code ipadb_check_logon_info(krb5_context context,
+ krb5_data *pac_blob)
+{
+ struct PAC_LOGON_INFO_CTR info;
+ krb5_error_code kerr;
+ TALLOC_CTX *tmpctx;
+
+ tmpctx = talloc_new(NULL);
+ if (!tmpctx) {
+ return ENOMEM;
+ }
+
+ kerr = get_logon_info(context, tmpctx, pac_blob, &info);
+ if (kerr) {
+ goto done;
+ }
+
+ kerr = add_local_groups(context, tmpctx, &info);
+ if (kerr) {
+ goto done;
+ }
+
+ kerr = save_logon_info(context, tmpctx, &info, pac_blob);
+ if (kerr) {
+ goto done;
+ }
done:
talloc_free(tmpctx);
-
return kerr;
}
@@ -1050,7 +1079,7 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
goto done;
}
- kerr = add_local_groups(context, &pac_blob);
+ kerr = ipadb_check_logon_info(context, &pac_blob);
if (kerr != 0) {
goto done;
}
--
1.7.10.4
>From 1764e6c2a578191285e4b35d37313cca7ac1be2a Mon Sep 17 00:00:00 2001
From: Simo Sorce <sso...@redhat.com>
Date: Fri, 13 Jul 2012 12:19:03 -0400
Subject: [PATCH 5/5] Add PAC filtering
This check the PAC we receive is consistent.
realm, flat name and domain sid must much our understanding or the trustd
realm and no additional sids beyond the own realm ones must be present.
Ticket #2849
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 108 ++++++++++++++++++++++++++++++++++++---
1 file changed, 100 insertions(+), 8 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 2a48c4f8ca7cee30d01380fbc12dddb928472963..b5346fed1230d02a88c94ab913507112990a1651 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -977,7 +977,101 @@ static krb5_error_code save_logon_info(krb5_context context,
return 0;
}
+static struct ipadb_adtrusts *get_domain_from_realm(krb5_context context,
+ krb5_data realm)
+{
+ struct ipadb_context *ipactx;
+ struct ipadb_adtrusts *domain;
+ int i;
+
+ ipactx = ipadb_get_context(context);
+ if (!ipactx) {
+ return NULL;
+ }
+
+ if (ipactx->mspac == NULL) {
+ return NULL;
+ }
+
+ for (i = 0; i < ipactx->mspac->num_trusts; i++) {
+ domain = &ipactx->mspac->trusts[i];
+ if (strlen(domain->domain_name) != realm.length) {
+ continue;
+ }
+ if (strncasecmp(domain->domain_name, realm.data, realm.length) == 0) {
+ return domain;
+ }
+ }
+
+ return NULL;
+}
+
+static krb5_error_code filter_logon_info(krb5_context context,
+ TALLOC_CTX *memctx,
+ krb5_data realm,
+ struct PAC_LOGON_INFO_CTR *info)
+{
+
+ /* We must refuse a PAC that comes signed with a cross realm TGT
+ * where the client pretends to be from a different realm. It is an
+ * attempt at getting us to sign fake credentials with the help of a
+ * compromised trusted realm */
+
+ struct ipadb_adtrusts *domain;
+ char *domsid;
+
+ domain = get_domain_from_realm(context, realm);
+ if (!domain) {
+ return EINVAL;
+ }
+
+ /* check netbios/flat name */
+ if (strcasecmp(info->info->info3.base.logon_domain.string,
+ domain->flat_name) != 0) {
+ krb5_klog_syslog(LOG_ERR, "PAC Info mismatch: domain = %s, "
+ "expected flat name = %s, "
+ "found logon name = %s",
+ domain->domain_name, domain->flat_name,
+ info->info->info3.base.logon_domain.string);
+ return EINVAL;
+ }
+
+ /* check sid */
+ domsid = dom_sid_string(NULL, info->info->info3.base.domain_sid);
+ if (!domsid) {
+ return EINVAL;
+ }
+
+ if (strcmp(domsid, domain->domain_sid) != 0) {
+ krb5_klog_syslog(LOG_ERR, "PAC Info mismatch: domain = %s, "
+ "expected domain SID = %s, "
+ "found domain SID = %s",
+ domain->domain_name, domain->domain_sid,
+ domsid);
+ talloc_free(domsid);
+ return EINVAL;
+ }
+ talloc_free(domsid);
+
+ /* According to MS-KILE, info->info->info3.sids must be zero, so check
+ * that it is the case here */
+ if (info->info->info3.sidcount != 0) {
+ return EINVAL;
+ }
+
+ /* According to MS-KILE, ResourceGroups must be zero, so check
+ * that it is the case here */
+ if (info->info->res_group_dom_sid != NULL &&
+ info->info->res_groups.count != 0) {
+ return EINVAL;
+ }
+
+ return 0;
+}
+
+
static krb5_error_code ipadb_check_logon_info(krb5_context context,
+ krb5_data origin_realm,
krb5_data *pac_blob)
{
struct PAC_LOGON_INFO_CTR info;
@@ -994,6 +1088,11 @@ static krb5_error_code ipadb_check_logon_info(krb5_context context,
goto done;
}
+ kerr = filter_logon_info(context, tmpctx, origin_realm, &info);
+ if (kerr) {
+ goto done;
+ }
+
kerr = add_local_groups(context, tmpctx, &info);
if (kerr) {
goto done;
@@ -1050,13 +1149,6 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
/* krbtgt from a trusted realm */
is_cross_realm = true;
- /* FIXME:
- * We must refuse a PAC that comes signed with a cross realm TGT
- * where the client pretends to be from our realm. It is an attempt
- * at getting us to sign fake credentials with the help of a
- * compromised trusted realm */
-
- /* TODO: Here is where we need to plug our PAC Filtering, later on */
srv_key = krbtgt_key;
} else {
@@ -1079,7 +1171,7 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
goto done;
}
- kerr = ipadb_check_logon_info(context, &pac_blob);
+ kerr = ipadb_check_logon_info(context, client_princ->realm, &pac_blob);
if (kerr != 0) {
goto done;
}
--
1.7.10.4
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
