rpms/krb5
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Cross-realm s4u fixes for samba (#1836630)

Browse Source
This commit is contained in:
Robbie Harwood 2020-10-21 11:24:24 -04:00
parent da77b5dcf8
commit 7c8b50fca5
6 changed files with 929 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
Adjust-KDC-alias-helper-function-contract.patch Normal file
View File

@ -0,0 +1,80 @@
From 758f5031fe9d6c1e3eb33818bc6d57cf8b4a3a72 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Tue, 22 Sep 2020 01:11:39 +0300
Subject: [PATCH] Adjust KDC alias helper function contract
Change the name of is_client_alias() to is_client_db_alias(), and
change the contract so that the already-canonical principal name comes
from a DB entry (which is less flexible, but clearer since DB entries
always contain canonical principal names). Make the function
available outside of kdc_util.c.
[ghudson@mit.edu: clarified commit message]
(cherry picked from commit 9fb5f572dd6ce808b234cb60a573eac48136d7ca)
---
src/kdc/kdc_util.c | 14 +++++++-------
src/kdc/kdc_util.h | 4 ++++
2 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index dcb2df8dc..6330387d0 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -1463,10 +1463,10 @@ cleanup:
return code;
}
-/* Return true if princ canonicalizes to the same principal as canon. */
-static krb5_boolean
-is_client_alias(krb5_context context, krb5_const_principal canon,
- krb5_const_principal princ)
+/* Return true if princ canonicalizes to the same principal as entry's. */
+krb5_boolean
+is_client_db_alias(krb5_context context, const krb5_db_entry *entry,
+ krb5_const_principal princ)
{
krb5_error_code ret;
krb5_db_entry *self;
@@ -1475,7 +1475,7 @@ is_client_alias(krb5_context context, krb5_const_principal canon,
ret = krb5_db_get_principal(context, princ,
KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY, &self);
if (!ret) {
- is_self = krb5_principal_compare(context, canon, self->princ);
+ is_self = krb5_principal_compare(context, entry->princ, self->princ);
krb5_db_free_principal(context, self);
}
@@ -1535,7 +1535,7 @@ kdc_process_s4u2self_req(kdc_realm_t *kdc_active_realm,
/* If the server is local, check that the request is for self. */
if (!isflagset(c_flags, KRB5_KDB_FLAG_ISSUING_REFERRAL) &&
- !is_client_alias(kdc_context, server->princ, client_princ)) {
+ !is_client_db_alias(kdc_context, server, client_princ)) {
*status = "INVALID_S4U2SELF_REQUEST_SERVER_MISMATCH";
return KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN; /* match Windows error */
}
@@ -1728,7 +1728,7 @@ kdc_process_s4u2proxy_req(kdc_realm_t *kdc_active_realm, unsigned int flags,
}
client_princ = *stkt_authdata_client;
- } else if (!is_client_alias(kdc_context, server->princ, server_princ)) {
+ } else if (!is_client_db_alias(kdc_context, server, server_princ)) {
*status = "EVIDENCE_TICKET_MISMATCH";
return KRB5KDC_ERR_SERVER_NOMATCH;
}
diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h
index 384b21ad2..2c9d8cf69 100644
--- a/src/kdc/kdc_util.h
+++ b/src/kdc/kdc_util.h
@@ -344,6 +344,10 @@ log_tgs_badtrans(krb5_context ctx, krb5_principal cprinc,
void
log_tgs_alt_tgt(krb5_context context, krb5_principal p);
+krb5_boolean
+is_client_db_alias(krb5_context context, const krb5_db_entry *entry,
+ krb5_const_principal princ);
+
/* FAST*/
enum krb5_fast_kdc_flags {
KRB5_FAST_REPLY_KEY_USED = 0x1,

65
Allow-aliases-when-matching-U2U-second-ticket.patch Normal file
View File

@ -0,0 +1,65 @@
From ccc5b9663e229f20421c01836aa5ecb06f1f2a48 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Tue, 22 Sep 2020 01:17:11 +0300
Subject: [PATCH] Allow aliases when matching U2U second ticket
In process_tgs_req() when verifying the user-to-user second ticket,
compare the canonical names of the request server and the second
ticket client.
[ghudson@mit.edu: expanded commit message; trimmed tests]
ticket: 8951 (new)
(cherry picked from commit afc494ef9418e6be7fbb887364efa6606b10034a)
---
src/kdc/do_tgs_req.c | 2 +-
src/tests/t_u2u.py | 25 +++++++++++++++++++++++++
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c
index 463a9c0dd..74cd19e96 100644
--- a/src/kdc/do_tgs_req.c
+++ b/src/kdc/do_tgs_req.c
@@ -666,7 +666,7 @@ process_tgs_req(krb5_kdc_req *request, krb5_data *pkt,
*/
krb5_enc_tkt_part *t2enc = request->second_ticket[st_idx]->enc_part2;
krb5_principal client2 = t2enc->client;
- if (!krb5_principal_compare(kdc_context, request->server, client2)) {
+ if (!is_client_db_alias(kdc_context, server, client2)) {
altcprinc = client2;
errcode = KRB5KDC_ERR_SERVER_NOMATCH;
status = "2ND_TKT_MISMATCH";
diff --git a/src/tests/t_u2u.py b/src/tests/t_u2u.py
index 1ca6ac87e..4b8a82a2f 100644
--- a/src/tests/t_u2u.py
+++ b/src/tests/t_u2u.py
@@ -32,4 +32,29 @@ realm.run([kvno, '--u2u', realm.ccache, realm.user_princ])
realm.run([klist])
+realm.stop()
+
+# Load the test KDB module to test aliases
+testprincs = {'krbtgt/KRBTEST.COM': {'keys': 'aes128-cts'},
+ 'user': {'keys': 'aes128-cts', 'flags': '+preauth'},
+ 'WIN10': {'keys': 'aes128-cts'}}
+kdcconf = {'realms': {'$realm': {'database_module': 'test'}},
+ 'dbmodules': {'test': {'db_library': 'test',
+ 'princs': testprincs,
+ 'alias': {'HOST/win10': 'WIN10'}}}}
+
+realm = K5Realm(kdc_conf=kdcconf, create_kdb=False)
+realm.start_kdc()
+
+# Create a second user principal and get tickets for it.
+u2u_ccache = 'FILE:' + os.path.join(realm.testdir, 'ccu2u')
+realm.extract_keytab('WIN10', realm.keytab)
+realm.kinit('WIN10', None, ['-k', '-c', u2u_ccache])
+
+realm.extract_keytab(realm.user_princ, realm.keytab)
+realm.kinit(realm.user_princ, None, ['-k'])
+
+realm.run([kvno, '--u2u', u2u_ccache, 'HOST/win10'], expected_msg='kvno = 0')
+realm.run([kvno, '--u2u', u2u_ccache, 'WIN10'], expected_msg='kvno = 0')
+
success('user-to-user tests')

124
Improve-KDC-alias-checking-for-S4U-requests.patch Normal file
View File

@ -0,0 +1,124 @@
From ed87237cdd70f72b309960a294a2bed26cef1579 Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris@gmail.com>
Date: Fri, 4 Sep 2020 14:05:50 +0300
Subject: [PATCH] Improve KDC alias checking for S4U requests
When processing an S4U2Self request, check for DB aliases when
matching the TGT client against the request server. When processing
an S4U2Proxy request, check for DB aliases when matching the TGT
client against the evidence ticket server.
[ghudson@mit.edu: minor edits; rewrote commit message]
ticket: 8946 (new)
(cherry picked from commit 05deeebfc096970b5d9aa67a48b14106cf1b9b56)
---
src/kdc/kdc_util.c | 74 ++++++++++++++++------------------------------
1 file changed, 25 insertions(+), 49 deletions(-)
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index e3352f9cc..dcb2df8dc 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -1463,6 +1463,25 @@ cleanup:
return code;
}
+/* Return true if princ canonicalizes to the same principal as canon. */
+static krb5_boolean
+is_client_alias(krb5_context context, krb5_const_principal canon,
+ krb5_const_principal princ)
+{
+ krb5_error_code ret;
+ krb5_db_entry *self;
+ krb5_boolean is_self = FALSE;
+
+ ret = krb5_db_get_principal(context, princ,
+ KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY, &self);
+ if (!ret) {
+ is_self = krb5_principal_compare(context, canon, self->princ);
+ krb5_db_free_principal(context, self);
+ }
+
+ return is_self;
+}
+
/*
* Protocol transition (S4U2Self)
*/
@@ -1481,7 +1500,6 @@ kdc_process_s4u2self_req(kdc_realm_t *kdc_active_realm,
{
krb5_error_code code;
krb5_pa_data *pa_data;
- int flags;
krb5_db_entry *princ;
krb5_s4u_userid *id;
@@ -1515,51 +1533,11 @@ kdc_process_s4u2self_req(kdc_realm_t *kdc_active_realm,
}
id = &(*s4u_x509_user)->user_id;
- /*
- * We need to compare the client name in the TGT with the requested
- * server name. Supporting server name aliases without assuming a
- * global name service makes this difficult to do.
- *
- * The comparison below handles the following cases (note that the
- * term "principal name" below excludes the realm).
- *
- * (1) The requested service is a host-based service with two name
- * components, in which case we assume the principal name to
- * contain sufficient qualifying information. The realm is
- * ignored for the purpose of comparison.
- *
- * (2) The requested service name is an enterprise principal name:
- * the service principal name is compared with the unparsed
- * form of the client name (including its realm).
- *
- * (3) The requested service is some other name type: an exact
- * match is required.
- *
- * An alternative would be to look up the server once again with
- * FLAG_CANONICALIZE | FLAG_CLIENT_REFERRALS_ONLY set, do an exact
- * match between the returned name and client_princ. However, this
- * assumes that the client set FLAG_CANONICALIZE when requesting
- * the TGT and that we have a global name service.
- */
- flags = 0;
- switch (krb5_princ_type(kdc_context, request->server)) {
- case KRB5_NT_SRV_HST: /* (1) */
- if (krb5_princ_size(kdc_context, request->server) == 2)
- flags |= KRB5_PRINCIPAL_COMPARE_IGNORE_REALM;
- break;
- case KRB5_NT_ENTERPRISE_PRINCIPAL: /* (2) */
- flags |= KRB5_PRINCIPAL_COMPARE_ENTERPRISE;
- break;
- default: /* (3) */
- break;
- }
-
- if (!krb5_principal_compare_flags(kdc_context,
- request->server,
- client_princ,
- flags)) {
- *status = "INVALID_S4U2SELF_REQUEST";
- return KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN; /* match Windows error code */
+ /* If the server is local, check that the request is for self. */
+ if (!isflagset(c_flags, KRB5_KDB_FLAG_ISSUING_REFERRAL) &&
+ !is_client_alias(kdc_context, server->princ, client_princ)) {
+ *status = "INVALID_S4U2SELF_REQUEST_SERVER_MISMATCH";
+ return KRB5KDC_ERR_C_PRINCIPAL_UNKNOWN; /* match Windows error */
}
/*
@@ -1750,9 +1728,7 @@ kdc_process_s4u2proxy_req(kdc_realm_t *kdc_active_realm, unsigned int flags,
}
client_princ = *stkt_authdata_client;
- } else if (!krb5_principal_compare(kdc_context,
- server->princ, /* after canon */
- server_princ)) {
+ } else if (!is_client_alias(kdc_context, server->princ, server_princ)) {
*status = "EVIDENCE_TICKET_MISMATCH";
return KRB5KDC_ERR_SERVER_NOMATCH;
}

316
Minimize-usage-of-tgs_server-in-KDC.patch Normal file
View File

@ -0,0 +1,316 @@
From 604135b5ad6bf954491413243eb305b82fec1c06 Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
Date: Fri, 25 Sep 2020 11:12:34 -0400
Subject: [PATCH] Minimize usage of tgs_server in KDC
Where possible, use the realm of the request server principal
(canonicalized via KDB lookup, if available) in preference to
tgs_server. This change facilitates alias realm support and potential
future support for serving multiple realms from the same KDB.
S4U2Self local user testing currently uses the uncanonicalized request
realm after this change, which will require attention for alias realm
support.
FAST armor ticket checking is unaffected by this change (it still
compares against tgs_server). This check poses no issue for realm
aliases, as both tgs_server and the armor ticket server should have
canonical realms, but it will require attention for multi-realm KDB
support.
Remove is_local_principal() as it is no longer used. Add an
is_local_tgs_principal() helper and shorten is_cross_tgs_principal().
Move the header ticket lineage check from kdc_process_tgs_req() to
process_tgs_req(), where we have the canonical request server name and
a more natural indication of whether the request was an S4U2Self
request.
(cherry picked from commit 90fedf8188fc47aa5a476a969af34671555df389)
---
src/kdc/do_as_req.c | 21 ++++++--------
src/kdc/do_tgs_req.c | 16 ++++++++---
src/kdc/kdc_util.c | 68 ++++++++++----------------------------------
src/kdc/kdc_util.h | 3 +-
src/kdc/tgs_policy.c | 16 ++++++-----
5 files changed, 46 insertions(+), 78 deletions(-)
diff --git a/src/kdc/do_as_req.c b/src/kdc/do_as_req.c
index 9ae7b0a5e..e243f50be 100644
--- a/src/kdc/do_as_req.c
+++ b/src/kdc/do_as_req.c
@@ -620,18 +620,6 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt,
}
state->rock.client = state->client;
- /*
- * If the backend returned a principal that is not in the local
- * realm, then we need to refer the client to that realm.
- */
- if (!is_local_principal(kdc_active_realm, state->client->princ)) {
- /* Entry is a referral to another realm */
- state->status = "REFERRAL";
- au_state->cl_realm = &state->client->princ->realm;
- errcode = KRB5KDC_ERR_WRONG_REALM;
- goto errout;
- }
-
au_state->stage = SRVC_PRINC;
s_flags = 0;
@@ -651,6 +639,15 @@ process_as_req(krb5_kdc_req *request, krb5_data *req_pkt,
goto errout;
}
+ /* If the KDB module returned a different realm for the client and server,
+ * we need to issue a client realm referral. */
+ if (!data_eq(state->server->princ->realm, state->client->princ->realm)) {
+ state->status = "REFERRAL";
+ au_state->cl_realm = &state->client->princ->realm;
+ errcode = KRB5KDC_ERR_WRONG_REALM;
+ goto errout;
+ }
+
errcode = get_local_tgt(kdc_context, &state->request->server->realm,
state->server, &state->local_tgt,
&state->local_tgt_storage, &state->local_tgt_key);
diff --git a/src/kdc/do_tgs_req.c b/src/kdc/do_tgs_req.c
index 74cd19e96..72525a462 100644
--- a/src/kdc/do_tgs_req.c
+++ b/src/kdc/do_tgs_req.c
@@ -268,7 +268,7 @@ process_tgs_req(krb5_kdc_req *request, krb5_data *pkt,
goto cleanup;
}
- if (!is_local_principal(kdc_active_realm, header_ticket->server))
+ if (!data_eq(header_server->princ->realm, sprinc->realm))
setflag(c_flags, KRB5_KDB_FLAG_CROSS_REALM);
if (is_referral)
setflag(c_flags, KRB5_KDB_FLAG_ISSUING_REFERRAL);
@@ -295,6 +295,15 @@ process_tgs_req(krb5_kdc_req *request, krb5_data *pkt,
au_state->s4u2self_user = NULL;
}
+ /* Aside from cross-realm S4U2Self requests, do not accept header tickets
+ * for local users issued by foreign realms. */
+ if (s4u_x509_user == NULL && data_eq(cprinc->realm, sprinc->realm) &&
+ isflagset(c_flags, KRB5_KDB_FLAG_CROSS_REALM)) {
+ krb5_klog_syslog(LOG_INFO, _("PROCESS_TGS: failed lineage check"));
+ retval = KRB5KDC_ERR_POLICY;
+ goto cleanup;
+ }
+
if (errcode)
goto cleanup;
@@ -583,13 +592,12 @@ process_tgs_req(krb5_kdc_req *request, krb5_data *pkt,
/*
* Only add the realm of the presented tgt to the transited list if
- * it is different than the local realm (cross-realm) and it is different
+ * it is different than the server realm (cross-realm) and it is different
* than the realm of the client (since the realm of the client is already
* implicitly part of the transited list and should not be explicitly
* listed).
*/
- /* realm compare is like strcmp, but knows how to deal with these args */
- if (krb5_realm_compare(kdc_context, header_ticket->server, tgs_server) ||
+ if (!isflagset(c_flags, KRB5_KDB_FLAG_CROSS_REALM) ||
krb5_realm_compare(kdc_context, header_ticket->server,
enc_tkt_reply.client)) {
/* tgt issued by local realm or issued by realm of client */
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index a4a05b9fa..a631b498d 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -78,12 +78,6 @@ static krb5_error_code find_server_key(krb5_context,
krb5_kvno, krb5_keyblock **,
krb5_kvno *);
-krb5_boolean
-is_local_principal(kdc_realm_t *kdc_active_realm, krb5_const_principal princ1)
-{
- return krb5_realm_compare(kdc_context, princ1, tgs_server);
-}
-
/*
* Returns TRUE if the kerberos principal is the name of a Kerberos ticket
* service.
@@ -104,13 +98,16 @@ krb5_is_tgs_principal(krb5_const_principal principal)
krb5_boolean
is_cross_tgs_principal(krb5_const_principal principal)
{
- if (!krb5_is_tgs_principal(principal))
- return FALSE;
- if (!data_eq(*krb5_princ_component(kdc_context, principal, 1),
- *krb5_princ_realm(kdc_context, principal)))
- return TRUE;
- else
- return FALSE;
+ return krb5_is_tgs_principal(principal) &&
+ !data_eq(principal->data[1], principal->realm);
+}
+
+/* Return true if princ is the name of a local TGS for any realm. */
+krb5_boolean
+is_local_tgs_principal(krb5_const_principal principal)
+{
+ return krb5_is_tgs_principal(principal) &&
+ data_eq(principal->data[1], principal->realm);
}
/*
@@ -143,17 +140,6 @@ comp_cksum(krb5_context kcontext, krb5_data *source, krb5_ticket *ticket,
return(0);
}
-/* Return true if padata contains an entry of either S4U2Self type. */
-static inline krb5_boolean
-has_s4u2self_padata(krb5_pa_data **padata)
-{
- if (krb5int_find_pa_data(NULL, padata, KRB5_PADATA_FOR_USER) != NULL)
- return TRUE;
- if (krb5int_find_pa_data(NULL, padata, KRB5_PADATA_S4U_X509_USER) != NULL)
- return TRUE;
- return FALSE;
-}
-
/* If a header ticket is decrypted, *ticket_out is filled in even on error. */
krb5_error_code
kdc_process_tgs_req(kdc_realm_t *kdc_active_realm,
@@ -170,7 +156,6 @@ kdc_process_tgs_req(kdc_realm_t *kdc_active_realm,
krb5_authdata **authdata = NULL;
krb5_data scratch1;
krb5_data * scratch = NULL;
- krb5_boolean foreign_server = FALSE;
krb5_auth_context auth_context = NULL;
krb5_authenticator * authenticator = NULL;
krb5_checksum * his_cksum = NULL;
@@ -199,19 +184,6 @@ kdc_process_tgs_req(kdc_realm_t *kdc_active_realm,
goto cleanup;
}
- /* If the "server" principal in the ticket is not something
- in the local realm, then we must refuse to service the request
- if the client claims to be from the local realm.
-
- If we don't do this, then some other realm's nasty KDC can
- claim to be authenticating a client from our realm, and we'll
- give out tickets concurring with it!
-
- we set a flag here for checking below.
- */
- foreign_server = !is_local_principal(kdc_active_realm,
- apreq->ticket->server);
-
if ((retval = krb5_auth_con_init(kdc_context, &auth_context)))
goto cleanup;
@@ -265,15 +237,6 @@ kdc_process_tgs_req(kdc_realm_t *kdc_active_realm,
goto cleanup_authenticator;
}
- /* make sure the client is of proper lineage (see above) */
- if (foreign_server && !has_s4u2self_padata(request->padata) &&
- is_local_principal(kdc_active_realm, ticket->enc_part2->client)) {
- /* someone in a foreign realm claiming to be local */
- krb5_klog_syslog(LOG_INFO, _("PROCESS_TGS: failed lineage check"));
- retval = KRB5KDC_ERR_POLICY;
- goto cleanup_authenticator;
- }
-
/*
* Check application checksum vs. tgs request
*
@@ -591,12 +554,12 @@ int
check_anon(kdc_realm_t *kdc_active_realm,
krb5_principal client, krb5_principal server)
{
- /* If restrict_anon is set, reject requests from anonymous to principals
- * other than the local TGT. */
+ /* If restrict_anon is set, reject requests from anonymous clients to
+ * server principals other than local TGTs. */
if (kdc_active_realm->realm_restrict_anon &&
krb5_principal_compare_any_realm(kdc_context, client,
krb5_anonymous_principal()) &&
- !krb5_principal_compare(kdc_context, server, tgs_server))
+ !is_local_tgs_principal(server))
return -1;
return 0;
}
@@ -1527,7 +1490,7 @@ kdc_process_s4u2self_req(kdc_realm_t *kdc_active_realm,
/*
* Do not attempt to lookup principals in foreign realms.
*/
- if (is_local_principal(kdc_active_realm, id->user)) {
+ if (data_eq(server->princ->realm, id->user->realm)) {
krb5_db_entry no_server;
krb5_pa_data **e_data = NULL;
@@ -1663,8 +1626,7 @@ kdc_process_s4u2proxy_req(kdc_realm_t *kdc_active_realm, unsigned int flags,
*/
if (isflagset(flags, KRB5_KDB_FLAG_ISSUING_REFERRAL) ||
!is_cross_tgs_principal(server->princ) ||
- !krb5_principal_compare_any_realm(kdc_context, server->princ,
- tgs_server) ||
+ !data_eq(server->princ->data[1], proxy->princ->realm) ||
!krb5_principal_compare(kdc_context, client_princ, server_princ)) {
*status = "XREALM_EVIDENCE_TICKET_MISMATCH";
return KRB5KDC_ERR_BADOPTION;
diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h
index 42b7ee208..c730409ae 100644
--- a/src/kdc/kdc_util.h
+++ b/src/kdc/kdc_util.h
@@ -37,10 +37,9 @@
#include "reqstate.h"
krb5_error_code check_hot_list (krb5_ticket *);
-krb5_boolean is_local_principal(kdc_realm_t *kdc_active_realm,
- krb5_const_principal princ1);
krb5_boolean krb5_is_tgs_principal (krb5_const_principal);
krb5_boolean is_cross_tgs_principal(krb5_const_principal);
+krb5_boolean is_local_tgs_principal(krb5_const_principal);
krb5_error_code
add_to_transited (krb5_data *,
krb5_data *,
diff --git a/src/kdc/tgs_policy.c b/src/kdc/tgs_policy.c
index 554345ba5..59d60ca25 100644
--- a/src/kdc/tgs_policy.c
+++ b/src/kdc/tgs_policy.c
@@ -251,19 +251,21 @@ check_tgs_s4u2proxy(kdc_realm_t *kdc_active_realm,
}
static int
-check_tgs_u2u(kdc_realm_t *kdc_active_realm,
- krb5_kdc_req *req, const char **status)
+check_tgs_u2u(kdc_realm_t *kdc_active_realm, krb5_kdc_req *req,
+ krb5_const_principal server_princ, const char **status)
{
+ krb5_const_principal second_server_princ;
+
if (req->kdc_options & KDC_OPT_ENC_TKT_IN_SKEY) {
/* Check that second ticket is in request. */
if (!req->second_ticket || !req->second_ticket[0]) {
*status = "NO_2ND_TKT";
return KDC_ERR_BADOPTION;
}
- /* Check that second ticket is a TGT. */
- if (!krb5_principal_compare(kdc_context,
- req->second_ticket[0]->server,
- tgs_server)) {
+ /* Check that second ticket is a TGT to the server realm. */
+ second_server_princ = req->second_ticket[0]->server;
+ if (!is_local_tgs_principal(second_server_princ) ||
+ !data_eq(second_server_princ->data[1], server_princ->realm)) {
*status = "2ND_TKT_NOT_TGS";
return KDC_ERR_POLICY;
}
@@ -352,7 +354,7 @@ validate_tgs_request(kdc_realm_t *kdc_active_realm,
return(KRB_AP_ERR_REPEAT);
}
- errcode = check_tgs_u2u(kdc_active_realm, request, status);
+ errcode = check_tgs_u2u(kdc_active_realm, request, server->princ, status);
if (errcode != 0)
return errcode;

335
Refactor-KDC-authdata-list-management-helpers.patch Normal file
View File

@ -0,0 +1,335 @@
From 9335481c00cd15170adec244ccff0a00a014bbab Mon Sep 17 00:00:00 2001
From: Greg Hudson <ghudson@mit.edu>
Date: Wed, 5 Feb 2020 18:46:11 -0500
Subject: [PATCH] Refactor KDC authdata list management helpers
Remove the unused concat_authorization_data(). Split merge_authdata()
into two helpers, one to destructively merge without filtering and one
to add copied elements while filtering out KDC-only authdata types.
Remove context parameters where they aren't needed (taking advantage
of knowledge that some libkrb5 functions don't use their context
parameters).
(cherry picked from commit b2190fdc253de6024001e0f1ff9fe56c31042bb7)
---
src/kdc/kdc_authdata.c | 138 +++++++++++++++++++----------------------
src/kdc/kdc_util.c | 50 ---------------
src/kdc/kdc_util.h | 5 --
3 files changed, 64 insertions(+), 129 deletions(-)
diff --git a/src/kdc/kdc_authdata.c b/src/kdc/kdc_authdata.c
index 1ebe87246..010922c27 100644
--- a/src/kdc/kdc_authdata.c
+++ b/src/kdc/kdc_authdata.c
@@ -108,7 +108,7 @@ unload_authdata_plugins(krb5_context context)
/* Return true if authdata should be filtered when copying from untrusted
* authdata. If desired_type is non-zero, look only for that type. */
static krb5_boolean
-is_kdc_issued_authdatum(krb5_context context, krb5_authdata *authdata,
+is_kdc_issued_authdatum(krb5_authdata *authdata,
krb5_authdatatype desired_type)
{
krb5_boolean result = FALSE;
@@ -117,7 +117,7 @@ is_kdc_issued_authdatum(krb5_context context, krb5_authdata *authdata,
krb5_authdatatype *ad_types, *containee_types = NULL;
if (authdata->ad_type == KRB5_AUTHDATA_IF_RELEVANT) {
- if (krb5int_get_authdata_containee_types(context, authdata, &count,
+ if (krb5int_get_authdata_containee_types(NULL, authdata, &count,
&containee_types) != 0)
goto cleanup;
ad_types = containee_types;
@@ -152,7 +152,7 @@ cleanup:
/* Return true if authdata contains any elements which should only come from
* the KDC. If desired_type is non-zero, look only for that type. */
static krb5_boolean
-has_kdc_issued_authdata(krb5_context context, krb5_authdata **authdata,
+has_kdc_issued_authdata(krb5_authdata **authdata,
krb5_authdatatype desired_type)
{
int i;
@@ -160,7 +160,7 @@ has_kdc_issued_authdata(krb5_context context, krb5_authdata **authdata,
if (authdata == NULL)
return FALSE;
for (i = 0; authdata[i] != NULL; i++) {
- if (is_kdc_issued_authdatum(context, authdata[i], desired_type))
+ if (is_kdc_issued_authdatum(authdata[i], desired_type))
return TRUE;
}
return FALSE;
@@ -181,66 +181,71 @@ has_mandatory_for_kdc_authdata(krb5_context context, krb5_authdata **authdata)
return FALSE;
}
-/*
- * Add the elements of in_authdata to out_authdata. If copy is false,
- * in_authdata is invalid on successful return. If ignore_kdc_issued is true,
- * KDC-issued authdata is not copied.
- */
+/* Add elements from *new_elements to *existing_list, reallocating as
+ * necessary. On success, release *new_elements and set it to NULL. */
static krb5_error_code
-merge_authdata(krb5_context context, krb5_authdata **in_authdata,
- krb5_authdata ***out_authdata, krb5_boolean copy,
- krb5_boolean ignore_kdc_issued)
+merge_authdata(krb5_authdata ***existing_list, krb5_authdata ***new_elements)
{
- krb5_error_code ret;
- size_t i, j, nadata = 0;
- krb5_authdata **in_copy = NULL, **authdata = *out_authdata;
+ size_t count = 0, ncount = 0;
+ krb5_authdata **list = *existing_list, **nlist = *new_elements;
- if (in_authdata == NULL || in_authdata[0] == NULL)
+ if (nlist == NULL)
return 0;
- if (authdata != NULL) {
- for (nadata = 0; authdata[nadata] != NULL; nadata++)
- ;
- }
+ for (count = 0; list != NULL && list[count] != NULL; count++);
+ for (ncount = 0; nlist[ncount] != NULL; ncount++);
- for (i = 0; in_authdata[i] != NULL; i++)
- ;
-
- if (copy) {
- ret = krb5_copy_authdata(context, in_authdata, &in_copy);
- if (ret)
- return ret;
- in_authdata = in_copy;
- }
-
- authdata = realloc(authdata, (nadata + i + 1) * sizeof(krb5_authdata *));
- if (authdata == NULL) {
- krb5_free_authdata(context, in_copy);
+ list = realloc(list, (count + ncount + 1) * sizeof(*list));
+ if (list == NULL)
return ENOMEM;
+
+ memcpy(list + count, nlist, ncount * sizeof(*nlist));
+ list[count + ncount] = NULL;
+ free(nlist);
+
+ if (list[0] == NULL) {
+ free(list);
+ list = NULL;
}
- for (i = 0, j = 0; in_authdata[i] != NULL; i++) {
- if (ignore_kdc_issued &&
- is_kdc_issued_authdatum(context, in_authdata[i], 0)) {
- free(in_authdata[i]->contents);
- free(in_authdata[i]);
+ *new_elements = NULL;
+ *existing_list = list;
+ return 0;
+}
+
+/* Add a copy of new_elements to *existing_list, omitting KDC-issued
+ * authdata. */
+static krb5_error_code
+add_filtered_authdata(krb5_authdata ***existing_list,
+ krb5_authdata **new_elements)
+{
+ krb5_error_code ret;
+ krb5_authdata **copy;
+ size_t i, j;
+
+ if (new_elements == NULL)
+ return 0;
+
+ ret = krb5_copy_authdata(NULL, new_elements, &copy);
+ if (ret)
+ return ret;
+
+ /* Remove KDC-issued elements from copy. */
+ j = 0;
+ for (i = 0; copy[i] != NULL; i++) {
+ if (is_kdc_issued_authdatum(copy[i], 0)) {
+ free(copy[i]->contents);
+ free(copy[i]);
} else {
- authdata[nadata + j++] = in_authdata[i];
+ copy[j++] = copy[i];
}
}
+ copy[j] = NULL;
- authdata[nadata + j] = NULL;
-
- free(in_authdata);
-
- if (authdata[0] == NULL) {
- free(authdata);
- authdata = NULL;
- }
-
- *out_authdata = authdata;
-
- return 0;
+ /* Destructively merge the filtered copy into existing_list. */
+ ret = merge_authdata(existing_list, &copy);
+ krb5_free_authdata(NULL, copy);
+ return ret;
}
/* Copy TGS-REQ authorization data into the ticket authdata. */
@@ -289,10 +294,7 @@ copy_request_authdata(krb5_context context, krb5_keyblock *client_key,
goto cleanup;
}
- /* Add a copy of the requested authdata to the ticket, ignoring KDC-issued
- * types. */
- ret = merge_authdata(context, req->unenc_authdata, tkt_authdata, TRUE,
- TRUE);
+ ret = add_filtered_authdata(tkt_authdata, req->unenc_authdata);
cleanup:
free(plaintext.data);
@@ -307,9 +309,7 @@ copy_tgt_authdata(krb5_context context, krb5_kdc_req *request,
if (has_mandatory_for_kdc_authdata(context, tgt_authdata))
return KRB5KDC_ERR_POLICY;
- /* Add a copy of the TGT authdata to the ticket, ignoring KDC-issued
- * types. */
- return merge_authdata(context, tgt_authdata, tkt_authdata, TRUE, TRUE);
+ return add_filtered_authdata(tkt_authdata, tgt_authdata);
}
/* Fetch authorization data from KDB module. */
@@ -374,8 +374,7 @@ fetch_kdb_authdata(krb5_context context, unsigned int flags,
/* Put the KDB authdata first in the ticket. A successful merge places the
* combined list in db_authdata and releases the old ticket authdata. */
- ret = merge_authdata(context, enc_tkt_reply->authorization_data,
- &db_authdata, FALSE, FALSE);
+ ret = merge_authdata(&db_authdata, &enc_tkt_reply->authorization_data);
if (ret)
krb5_free_authdata(context, db_authdata);
else
@@ -404,8 +403,7 @@ make_signedpath_data(krb5_context context, krb5_const_principal client,
return ret;
for (i = 0, j = 0; authdata[i] != NULL; i++) {
- if (is_kdc_issued_authdatum(context, authdata[i],
- KRB5_AUTHDATA_SIGNTICKET))
+ if (is_kdc_issued_authdatum(authdata[i], KRB5_AUTHDATA_SIGNTICKET))
continue;
sign_authdata[j++] = authdata[i];
@@ -635,12 +633,8 @@ make_signedpath(krb5_context context, krb5_const_principal for_user_princ,
if (ret)
goto cleanup;
- /* Add the authdata to the ticket, without copying or filtering. */
- ret = merge_authdata(context, if_relevant,
- &enc_tkt_reply->authorization_data, FALSE, FALSE);
- if (ret)
- goto cleanup;
- if_relevant = NULL; /* merge_authdata() freed */
+ /* Add the signedpath authdata to the ticket. */
+ ret = merge_authdata(&enc_tkt_reply->authorization_data, &if_relevant);
cleanup:
free(sp.delegated);
@@ -665,7 +659,7 @@ free_deleg_path(krb5_context context, krb5_principal *deleg_path)
static krb5_boolean
has_pac(krb5_context context, krb5_authdata **authdata)
{
- return has_kdc_issued_authdata(context, authdata, KRB5_AUTHDATA_WIN2K_PAC);
+ return has_kdc_issued_authdata(authdata, KRB5_AUTHDATA_WIN2K_PAC);
}
/* Verify AD-SIGNTICKET authdata if we need to, and insert an AD-SIGNEDPATH
@@ -746,11 +740,7 @@ add_auth_indicators(krb5_context context, krb5_data *const *auth_indicators,
goto cleanup;
/* Add the wrapped authdata to the ticket, without copying or filtering. */
- ret = merge_authdata(context, cammac, &enc_tkt_reply->authorization_data,
- FALSE, FALSE);
- if (ret)
- goto cleanup;
- cammac = NULL; /* merge_authdata() freed */
+ ret = merge_authdata(&enc_tkt_reply->authorization_data, &cammac);
cleanup:
krb5_free_data(context, der_indicators);
diff --git a/src/kdc/kdc_util.c b/src/kdc/kdc_util.c
index 6330387d0..a4a05b9fa 100644
--- a/src/kdc/kdc_util.c
+++ b/src/kdc/kdc_util.c
@@ -78,56 +78,6 @@ static krb5_error_code find_server_key(krb5_context,
krb5_kvno, krb5_keyblock **,
krb5_kvno *);
-/*
- * concatenate first two authdata arrays, returning an allocated replacement.
- * The replacement should be freed with krb5_free_authdata().
- */
-krb5_error_code
-concat_authorization_data(krb5_context context,
- krb5_authdata **first, krb5_authdata **second,
- krb5_authdata ***output)
-{
- int i, j;
- krb5_authdata **ptr, **retdata;
-
- /* count up the entries */
- i = 0;
- if (first)
- for (ptr = first; *ptr; ptr++)
- i++;
- if (second)
- for (ptr = second; *ptr; ptr++)
- i++;
-
- retdata = (krb5_authdata **)malloc((i+1)*sizeof(*retdata));
- if (!retdata)
- return ENOMEM;
- retdata[i] = 0; /* null-terminated array */
- for (i = 0, j = 0, ptr = first; j < 2 ; ptr = second, j++)
- while (ptr && *ptr) {
- /* now walk & copy */
- retdata[i] = (krb5_authdata *)malloc(sizeof(*retdata[i]));
- if (!retdata[i]) {
- krb5_free_authdata(context, retdata);
- return ENOMEM;
- }
- *retdata[i] = **ptr;
- if (!(retdata[i]->contents =
- (krb5_octet *)malloc(retdata[i]->length))) {
- free(retdata[i]);
- retdata[i] = 0;
- krb5_free_authdata(context, retdata);
- return ENOMEM;
- }
- memcpy(retdata[i]->contents, (*ptr)->contents, retdata[i]->length);
-
- ptr++;
- i++;
- }
- *output = retdata;
- return 0;
-}
-
krb5_boolean
is_local_principal(kdc_realm_t *kdc_active_realm, krb5_const_principal princ1)
{
diff --git a/src/kdc/kdc_util.h b/src/kdc/kdc_util.h
index 2c9d8cf69..42b7ee208 100644
--- a/src/kdc/kdc_util.h
+++ b/src/kdc/kdc_util.h
@@ -52,11 +52,6 @@ compress_transited (krb5_data *,
krb5_principal,
krb5_data *);
krb5_error_code
-concat_authorization_data (krb5_context,
- krb5_authdata **,
- krb5_authdata **,
- krb5_authdata ***);
-krb5_error_code
fetch_last_req_info (krb5_db_entry *, krb5_last_req_entry ***);
krb5_error_code

10
krb5.spec
View File

@ -18,7 +18,7 @@ Summary: The Kerberos network authentication system
Name: krb5
Version: 1.18.2
# for prerelease, should be e.g., 0.% {prerelease}.1% { ?dist } (without spaces)
Release: 25%{?dist}
Release: 26%{?dist}
# rharwood has trust path to signing key and verifies on check-in
Source0: https://web.mit.edu/kerberos/dist/krb5/1.18/krb5-%{version}%{prerelease}.tar.gz
@ -74,6 +74,11 @@ Patch35: Fix-leak-in-KERB_AP_OPTIONS_CBT-server-support.patch
Patch36: Fix-input-length-checking-in-SPNEGO-DER-decoding.patch
Patch37: Add-three-kvno-options-from-Heimdal-kgetcred.patch
Patch38: Unify-kvno-option-documentation.patch
Patch39: Improve-KDC-alias-checking-for-S4U-requests.patch
Patch40: Adjust-KDC-alias-helper-function-contract.patch
Patch41: Allow-aliases-when-matching-U2U-second-ticket.patch
Patch42: Refactor-KDC-authdata-list-management-helpers.patch
Patch43: Minimize-usage-of-tgs_server-in-KDC.patch
License: MIT
URL: https://web.mit.edu/kerberos/www/
@ -634,6 +639,9 @@ exit 0
%{_libdir}/libkadm5srv_mit.so.*
%changelog
* Wed Oct 21 2020 Robbie Harwood <rharwood@redhat.com> - 1.18.2-26
- Cross-realm s4u fixes for samba (#1836630)
* Thu Oct 15 2020 Robbie Harwood <rharwood@redhat.com> - 1.18.2-25
- Unify kvno option documentation