[Freeipa-devel] [PATCH] 0096 support Windows Server 2012
Alexander Bokovoy
abokovoy at redhat.com
Wed Dec 5 12:16:41 UTC 2012
On Thu, 22 Nov 2012, Simo Sorce wrote:
>On Thu, 2012-11-22 at 17:59 +0200, Alexander Bokovoy wrote:
>> Hi,
>>
>> attached patch attempts to bring us up to MS-KILE version 25.0 support
>> by
>> verifying that if number of additional SIDs in KERB_VALIDATION_INFO
>> structure is equal to one then this SID must be
>>
>> AUTHENTICATION_AUTHORITY_ASSERTED_IDENTITY, S-1-18-1
>>
>> This SID means the client's identity is asserted by an authentication
>> authority based on proof of possession of client credentials.
>>
>> During AD interop event at Microsoft earlier this year Simo found out
>> that this is the case for Windows Server 2012 and we need to relax our
>> check to allow this case.
>>
>> https://fedorahosted.org/freeipa/ticket/3231
>>
>> I haven't tested it against Windows Server 2012 yet but sending the
>> patch out for early check and verification.
>>
>>
>NACK,
>there are 2 SID Windows 2012 may put there, not just S-1-2-18-1 (also -2
>IIRC) and after I checked the docs I really think (As I suggested
>before) that we shouldn't expect a specific SID here, or in a next
>release a Windows server may break us again.
>
>The spec doesn't say they will never add other SIDs like these with new
>meanings.
>What we need to do is to check that NONE of these SIDs is from our own
>domain, or is a builtin SID.
>
>I think the best option for now, is to filter out any SID in there that
>we do not explicitly recognize, but not fail if there is any we do not
>support, just skip.
>
>So if you find S-1-18-1/S-1-18-2 you may decide to leave them in the
>PAC, they are useful indications to services and they can decide whether
>to use them or not. We need to filter out any SID that is not a regular
>domain SID (like wellknown SIDs and Builtin Domain SIDs) and any SID
>that belong to our own domain. Beyond that we should retain other SIDs
>(for example this structure might list an HistrorySID for the incoming
>user and we should give a chance to applications to make use of that
>information.
Attached is a prototype to implement logic above. I haven't added
filtering for anything but our own domain SIDs yet, want to get review
for this part before going further.
--
/ Alexander Bokovoy
-------------- next part --------------
>From 636d0a1dc52cf4059e447c1666477f1633e07c9a Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Thu, 22 Nov 2012 17:45:40 +0200
Subject: [PATCH] ipa-kdb: Support Windows 2012 Server
Windows 2012 Server changed procedure how KERB_VALIDATION_INFO ([MS-PAC] section 2.5)
is populated. Detailed description is available in [MS-KILE] version 25.0 and above.
Refactor KERB_VALIDATION_INFO verification and ensure we filter out extra SIDs in
case they belong to our domain.
https://fedorahosted.org/freeipa/ticket/3231
---
daemons/ipa-kdb/ipa_kdb_mspac.c | 127 ++++++++++++++++++++++++++++++++++------
1 file changed, 110 insertions(+), 17 deletions(-)
diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index ed2c7fb8c8c4975ce942335f4688d32f7c84f937..0609fe3b1ff02e44f715fe9a6645aad7e8cb14eb 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -30,11 +30,13 @@ struct ipadb_adtrusts {
char *domain_name;
char *flat_name;
char *domain_sid;
+ struct dom_sid domsid;
};
struct ipadb_mspac {
char *flat_domain_name;
char *flat_server_name;
+ struct dom_sid domsid;
char *fallback_group;
uint32_t fallback_rid;
@@ -87,6 +89,7 @@ static char *memberof_pac_attrs[] = {
#define SID_ID_AUTHS 6
#define SID_SUB_AUTHS 15
#define MAX(a,b) (((a)>(b))?(a):(b))
+#define SID_AUTHENTICATION_AUTHORITY_ASSERTED_IDENTITY "S-1-18-1"
static int string_to_sid(char *str, struct dom_sid *sid)
{
@@ -213,6 +216,51 @@ static struct dom_sid *dom_sid_dup(TALLOC_CTX *memctx,
return new_sid;
}
+static int dom_sid_cmp(const struct dom_sid *sid1, const struct dom_sid *sid2, bool compare_rids)
+{
+ int c, num;
+
+ if (sid1 == sid2) {
+ return 0;
+ }
+
+ if (sid1 == NULL) {
+ return -1;
+ }
+
+ if (sid2 == NULL) {
+ return 1;
+ }
+
+ /* If SIDs have different revisions, they are different */
+ if (sid1->sid_rev_num != sid2->sid_rev_num)
+ return sid1->sid_rev_num - sid2->sid_rev_num;
+
+ /* When number of authorities is different, sids are different */
+ if (sid1->num_auths != sid2->num_auths)
+ return sid1->num_auths - sid2->num_auths;
+
+ /* Optionally skip RIDs if asked */
+ num = sid1->num_auths - 1;
+ if (!compare_rids) {
+ num--;
+ if (num < 0) return sid1->sub_auths[0] - sid2->sub_auths[0];
+ }
+
+ /* for same size authorities compare them backwards
+ * since RIDs are likely different */
+ for (c = num; c >= 0; --c)
+ if (sid1->sub_auths[c] != sid2->sub_auths[c])
+ return sid1->sub_auths[c] - sid2->sub_auths[c];
+
+ /* Finally, compare Identifier authorities */
+ for (c = 0; c < SID_ID_AUTHS; c++)
+ if (sid1->id_auth[c] != sid2->id_auth[c])
+ return sid1->id_auth[c] - sid2->id_auth[c];
+
+ return 0;
+}
+
static int sid_append_rid(struct dom_sid *sid, uint32_t rid)
{
if (sid->num_auths >= SID_SUB_AUTHS) {
@@ -1070,8 +1118,9 @@ static krb5_error_code filter_logon_info(krb5_context context,
* attempt at getting us to sign fake credentials with the help of a
* compromised trusted realm */
+ struct ipadb_context *ipactx;
struct ipadb_adtrusts *domain;
- char *domsid;
+ int i, j, result, count;
domain = get_domain_from_realm_update(context, realm);
if (!domain) {
@@ -1089,27 +1138,48 @@ static krb5_error_code filter_logon_info(krb5_context context,
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) {
+ /* check exact sid */
+ result = dom_sid_cmp(&domain->domsid, info->info->info3.base.domain_sid, true);
+ if (result != 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);
+ "expected domain SID = %s, ",
+ domain->domain_name, domain->domain_sid);
return EINVAL;
}
- talloc_free(domsid);
- /* According to MS-KILE, info->info->info3.sids must be zero, so check
- * that it is the case here */
+ /* 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) {
- return EINVAL;
+ ipactx = ipadb_get_context(context);
+ if (!ipactx && !ipactx->mspac) {
+ return KRB5_KDB_DBNOTINITED;
+ }
+ count = info->info->info3.sidcount;
+ i = 0;
+ j = 0;
+ do {
+ /* Compare SIDs without taking RID into account */
+ result = dom_sid_cmp(&ipactx->mspac->domsid, info->info->info3.sids[i].sid, false);
+ if (result == 0) {
+ krb5_klog_syslog(LOG_ERR, "PAC Info mismatch: domain = %s, "
+ "extra sid should be not from the domain %s but received RID %d ",
+ domain->domain_name, ipactx->mspac->flat_domain_name,
+ info->info->info3.sids[i].sid->sub_auths[info->info->info3.sids[i].sid->num_auths-1]);
+ j++;
+ memmove(info->info->info3.sids+i, info->info->info3.sids+i+1, count-i-1);
+ }
+ i++;
+ } while (i < count);
+
+ if (j != 0) {
+ info->info->info3.sids = talloc_realloc(memctx, info->info->info3.sids, struct netr_SidAttr, count-j);
+ info->info->info3.sidcount = count-j;
+ if (!info->info->info3.sids) {
+ info->info->info3.sidcount = 0;
+ return ENOMEM;
+ }
+ }
}
/* According to MS-KILE, ResourceGroups must be zero, so check
@@ -1595,6 +1665,12 @@ krb5_error_code ipadb_mspac_get_trusted_domains(struct ipadb_context *ipactx)
ret = EINVAL;
goto done;
}
+
+ ret = string_to_sid(t[n].domain_sid, &t[n].domsid);
+ if (ret) {
+ ret = EINVAL;
+ goto done;
+ }
}
ret = 0;
@@ -1611,6 +1687,7 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx)
{
char *dom_attrs[] = { "ipaNTFlatName",
"ipaNTFallbackPrimaryGroup",
+ "ipaNTSecurityIdentifier",
NULL };
char *grp_attrs[] = { "ipaNTSecurityIdentifier", NULL };
krb5_error_code kerr;
@@ -1664,6 +1741,22 @@ krb5_error_code ipadb_reinit_mspac(struct ipadb_context *ipactx)
goto done;
}
+ ret = ipadb_ldap_attr_to_str(ipactx->lcontext, lentry,
+ "ipaNTSecurityIdentifier",
+ &resstr);
+ if (ret) {
+ kerr = ret;
+ goto done;
+ }
+
+ ret = string_to_sid(resstr, &ipactx->mspac->domsid);
+ if (ret) {
+ kerr = ret;
+ free(resstr);
+ goto done;
+ }
+ free(resstr);
+
free(ipactx->mspac->flat_server_name);
ipactx->mspac->flat_server_name = get_server_netbios_name();
if (!ipactx->mspac->flat_server_name) {
--
1.8.0
More information about the Freeipa-devel
mailing list