Cross realm S4U2Self cont.
Isaac Boukris
iboukris at gmail.com
Mon Aug 20 18:57:06 UTC 2018
Update:
On Mon, Aug 13, 2018 at 12:27 AM, Isaac Boukris <iboukris at gmail.com> wrote:
> Update:
> I've reworked the kdc commit and have updated some others, new patch attached.
> I submitted this to a new branch on gitlab, see:
> https://gitlab.com/samba-team/devel/samba/commits/iboukris_xrealm_s4u2self_v2
I've been doing some tests with MIT KDC and got some new insights.
First and most importantly, I believe I got the cross-realm s4u2self
using enterprise name case - wrong.
In my heimdal tests, I concluded that in this case windows would send
the name only, like user at upnsuffix in PAC_CLIENT_INFO (without realm,
same as none cross realm s4u2self with enterprise), and so have
assumed that I can simply ignore this case (since the
convert-to-enterprise should be no-op).
However now, I clearly see that windows actually sends with realm,
like user at upnsuffix@REALM.
I can see how it worked for me despite my mistake in heimdal when I
tested trust with windows while the service was on samba side, shortly
because heimdal ignores the realm when comparing the names.
But I'm unclear how the other way worked, when the service is on
windows side - I'll have to test more thoroughly and to update the kdc
commit in heimdal.
Meanwhile, attached prove-of-concept patches to make s4u2self and
cross realm s4u2self work with MIT KDC (put aside bug 13571).
With it, both enterprise and none-enterprise work, when the service is
on samba side (still working on the other way).
Note that it looks like we'd need a patch upstream in MIT code (I'll
consult upstream as well).
-------------- next part --------------
From a24ef9111cc8e7f507dce6bc3cc8d55df201539a Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Mon, 20 Aug 2018 16:18:50 +0300
Subject: [PATCH] Let verify pac with realm for cross realm s4u2self
Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
src/include/krb5/krb5.hin | 6 ++++++
src/lib/krb5/krb/authdata.h | 7 +++++++
src/lib/krb5/krb/pac.c | 35 +++++++++++++++++++++++++++++++----
src/lib/krb5/libkrb5.exports | 1 +
4 files changed, 45 insertions(+), 4 deletions(-)
diff --git a/src/include/krb5/krb5.hin b/src/include/krb5/krb5.hin
index 384bb838f..0e75eba64 100644
--- a/src/include/krb5/krb5.hin
+++ b/src/include/krb5/krb5.hin
@@ -8276,6 +8276,12 @@ krb5_pac_verify(krb5_context context, const krb5_pac pac,
krb5_timestamp authtime, krb5_const_principal principal,
const krb5_keyblock *server, const krb5_keyblock *privsvr);
+krb5_error_code KRB5_CALLCONV
+krb5_pac_verify_ex(krb5_context context, const krb5_pac pac,
+ krb5_timestamp authtime, krb5_const_principal principal,
+ const krb5_keyblock *server, const krb5_keyblock *privsvr,
+ krb5_boolean with_realm);
+
/**
* Sign a PAC.
*
diff --git a/src/lib/krb5/krb/authdata.h b/src/lib/krb5/krb/authdata.h
index 1e5c08426..0992f4855 100644
--- a/src/lib/krb5/krb/authdata.h
+++ b/src/lib/krb5/krb/authdata.h
@@ -92,6 +92,13 @@ k5_pac_validate_client(krb5_context context,
krb5_timestamp authtime,
krb5_const_principal principal);
+krb5_error_code
+k5_pac_validate_client_ex(krb5_context context,
+ const krb5_pac pac,
+ krb5_timestamp authtime,
+ krb5_const_principal principal,
+ krb5_boolean with_realm);
+
krb5_error_code
k5_pac_add_buffer(krb5_context context,
krb5_pac pac,
diff --git a/src/lib/krb5/krb/pac.c b/src/lib/krb5/krb/pac.c
index c9b5de30a..460b2bc3a 100644
--- a/src/lib/krb5/krb/pac.c
+++ b/src/lib/krb5/krb/pac.c
@@ -404,6 +404,17 @@ k5_pac_validate_client(krb5_context context,
const krb5_pac pac,
krb5_timestamp authtime,
krb5_const_principal principal)
+{
+ return k5_pac_validate_client_ex(context, pac, authtime,
+ principal, 0);
+}
+
+krb5_error_code
+k5_pac_validate_client_ex(krb5_context context,
+ const krb5_pac pac,
+ krb5_timestamp authtime,
+ krb5_const_principal principal,
+ krb5_boolean with_realm)
{
krb5_error_code ret;
krb5_data client_info;
@@ -413,7 +424,7 @@ k5_pac_validate_client(krb5_context context,
krb5_ui_2 pac_princname_length;
int64_t pac_nt_authtime;
krb5_principal pac_principal;
- int flags;
+ int flags = 0;
ret = k5_pac_locate_buffer(context, pac, KRB5_PAC_CLIENT_INFO,
&client_info);
@@ -443,7 +454,8 @@ k5_pac_validate_client(krb5_context context,
/* Parse the UTF-8 name as an enterprise principal if we are matching
* against one; otherwise parse it as a regular principal with no realm. */
- flags = KRB5_PRINCIPAL_PARSE_NO_REALM;
+ if (!with_realm)
+ flags = KRB5_PRINCIPAL_PARSE_NO_REALM;
if (principal->type == KRB5_NT_ENTERPRISE_PRINCIPAL)
flags |= KRB5_PRINCIPAL_PARSE_ENTERPRISE;
ret = krb5_parse_name_flags(context, pac_princname, flags, &pac_principal);
@@ -458,7 +470,8 @@ k5_pac_validate_client(krb5_context context,
!krb5_principal_compare_flags(context,
pac_principal,
principal,
- KRB5_PRINCIPAL_COMPARE_IGNORE_REALM))
+ with_realm ? 0 :
+ KRB5_PRINCIPAL_COMPARE_IGNORE_REALM))
ret = KRB5KRB_AP_WRONG_PRINC;
krb5_free_principal(context, pac_principal);
@@ -622,6 +635,19 @@ krb5_pac_verify(krb5_context context,
krb5_const_principal principal,
const krb5_keyblock *server,
const krb5_keyblock *privsvr)
+{
+ return krb5_pac_verify_ex(context, pac, authtime, principal,
+ server, privsvr, 0);
+}
+
+krb5_error_code KRB5_CALLCONV
+krb5_pac_verify_ex(krb5_context context,
+ const krb5_pac pac,
+ krb5_timestamp authtime,
+ krb5_const_principal principal,
+ const krb5_keyblock *server,
+ const krb5_keyblock *privsvr,
+ krb5_boolean with_realm)
{
krb5_error_code ret;
@@ -638,7 +664,8 @@ krb5_pac_verify(krb5_context context,
}
if (principal != NULL) {
- ret = k5_pac_validate_client(context, pac, authtime, principal);
+ ret = k5_pac_validate_client_ex(context, pac, authtime,
+ principal, with_realm);
if (ret != 0)
return ret;
}
diff --git a/src/lib/krb5/libkrb5.exports b/src/lib/krb5/libkrb5.exports
index 622bc3673..3d4c6d3ed 100644
--- a/src/lib/krb5/libkrb5.exports
+++ b/src/lib/krb5/libkrb5.exports
@@ -487,6 +487,7 @@ krb5_pac_init
krb5_pac_parse
krb5_pac_sign
krb5_pac_verify
+krb5_pac_verify_ex
krb5_parse_name
krb5_parse_name_flags
krb5_prepend_error_message
--
2.14.3
-------------- next part --------------
From 2140e3cf1da15d3ad18a48f5ccb0d3f3ff2de90c Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Mon, 20 Aug 2018 21:45:09 +0300
Subject: [PATCH] wip: mit-kdc: make s4u2self and cross realm s4u2self work
Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
source4/kdc/mit-kdb/kdb_samba_policies.c | 58 +++++++++++++++++++++++++++-----
1 file changed, 49 insertions(+), 9 deletions(-)
diff --git a/source4/kdc/mit-kdb/kdb_samba_policies.c b/source4/kdc/mit-kdb/kdb_samba_policies.c
index 81ac73582e0..8f295c27192 100644
--- a/source4/kdc/mit-kdb/kdb_samba_policies.c
+++ b/source4/kdc/mit-kdb/kdb_samba_policies.c
@@ -192,7 +192,8 @@ static krb5_error_code ks_verify_pac(krb5_context context,
krb5_keyblock *krbtgt_key,
krb5_timestamp authtime,
krb5_authdata **tgt_auth_data,
- krb5_pac *pac)
+ krb5_pac *pac,
+ krb5_boolean with_realm)
{
struct mit_samba_context *mit_ctx;
krb5_authdata **authdata = NULL;
@@ -249,12 +250,13 @@ static krb5_error_code ks_verify_pac(krb5_context context,
server_key,
krbtgt_key);
} else {
- code = krb5_pac_verify(context,
+ code = krb5_pac_verify_ex(context,
ipac,
authtime,
client_princ,
krbtgt_key,
- NULL);
+ NULL,
+ with_realm);
}
if (code != 0) {
goto done;
@@ -302,17 +304,44 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
krb5_authdata ***signed_auth_data)
{
krb5_const_principal ks_client_princ;
+ krb5_db_entry *check_client;
krb5_authdata **authdata = NULL;
krb5_boolean is_as_req;
+ krb5_boolean with_realm = false;
krb5_error_code code;
krb5_pac pac = NULL;
krb5_data pac_data;
/* Prefer canonicalised name from client entry */
- if (client != NULL) {
- ks_client_princ = client->princ;
- } else {
- ks_client_princ = client_princ;
+ ks_client_princ = client ? client->princ : client_princ;
+ check_client = client;
+
+ /*
+ * In the S4U2Self case, the PAC is of the service if the
+ * impersonated principal is local, otherwise the PAC is
+ * of the impersonated principal. */
+ if (flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION) {
+ /* Cross realm s4u2self include the realm in PAC_CLIENT_INFO */
+ if (client == NULL) {
+ with_realm = true;
+ }
+ else if (server == NULL) {
+ code = KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN;
+ goto done;
+ }
+ /* We need to check the PAC of the TGT client, but we are
+ * only privided with with the impersonated principal.
+ * We can take the client principal from the service since
+ * they are the same principal - not sure it is good enough.
+ *
+ * I wonder whether we really need to check this PAC, since
+ * we aren't doing any authorization based on it.
+ * Skipping the check and discarding the PAC would simplify
+ * greatly the heimdal patch as well. */
+ else {
+ ks_client_princ = server->princ;
+ check_client = server; // ?
+ }
}
is_as_req = ((flags & KRB5_KDB_FLAG_CLIENT_REFERRALS_ONLY) != 0);
@@ -328,17 +357,28 @@ krb5_error_code kdb_samba_db_sign_auth_data(krb5_context context,
code = ks_verify_pac(context,
flags,
ks_client_princ,
- client,
+ check_client,
server,
krbtgt,
server_key,
krbtgt_key,
authtime,
tgt_auth_data,
- &pac);
+ &pac,
+ with_realm);
if (code != 0) {
goto done;
}
+
+ if (flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION) {
+ /* We need a new PAC for our impersonated principal */
+ if (client) {
+ krb5_pac_free(context, pac);
+ pac = NULL;
+ }
+ ks_client_princ = client_princ;
+ }
+
}
if (pac == NULL && client != NULL) {
--
2.14.3
More information about the samba-technical
mailing list