Re: [Freeipa-devel] [PATCH] #2169 Fix PAC Validation

2012-01-11 Thread Rob Crittenden

Simo Sorce wrote:


PAC Validation was not correct for all cases.

In order to test this patch you must use the krb5 packages in the
ipa-devel repo as they contains some patches that were necessary to fix
various issues with s4u2proxy cases including PAC verification.

Fixes 2169

Simo.


ACK

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] #2169 Fix PAC Validation

2012-01-11 Thread Simo Sorce
On Wed, 2012-01-11 at 17:29 -0500, Rob Crittenden wrote:
 Simo Sorce wrote:
 
  PAC Validation was not correct for all cases.
 
  In order to test this patch you must use the krb5 packages in the
  ipa-devel repo as they contains some patches that were necessary to fix
  various issues with s4u2proxy cases including PAC verification.
 
  Fixes 2169
 
  Simo.
 
 ACK

Pushed to master.
Also pushed to ipa-2-2 to maintain the code in sync even though this
code will be disabled there.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


[Freeipa-devel] [PATCH] #2169 Fix PAC Validation

2011-12-07 Thread Simo Sorce

PAC Validation was not correct for all cases.

In order to test this patch you must use the krb5 packages in the
ipa-devel repo as they contains some patches that were necessary to fix
various issues with s4u2proxy cases including PAC verification.

Fixes 2169

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
From b1dcc5df2dea94540fab7ce7dc210ab5014eed25 Mon Sep 17 00:00:00 2001
From: Simo Sorce sso...@redhat.com
Date: Tue, 6 Dec 2011 18:07:59 -0500
Subject: [PATCH 1/2] ipa-kdb: Verify the correct checksum in PAC validation

This patch requires a forthcoming change in MIT libraries which allows to pass
NULL for the server_key to the krb5_pac_verify() function.

In most cases we should always only check the KDC checksum to verify the PAC
validity.

The only exception is when we are releasing a ticket to a client from another
realm. In this case the only signature we can check is the server checksum, and
we use the cross-realm key to validate in this case.

The previous code was working for normal cases because the kdc uses the same
key to create the server and the kdc checksum for a TGT, but that is not true
for evidence tickets (s4u2proxy) or cross-realm TGTs.

Fixes: https://fedorahosted.org/freeipa/ticket/2169
---
 daemons/ipa-kdb/ipa_kdb_mspac.c |   50 +++
 1 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 7973e8c89a66c25394c61878514315620a0e7521..a22044c2413da57a54479eedaa390e9a2725cdcb 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -554,10 +554,27 @@ done:
 return kerr;
 }
 
+static bool is_cross_realm_krbtgt(krb5_const_principal princ)
+{
+if ((princ-length != 2) ||
+(princ-data[0].length != 6) ||
+(strncasecmp(princ-data[0].data, krbtgt, 6) != 0)) {
+return false;
+}
+if (princ-data[1].length == princ-realm.length 
+strncasecmp(princ-data[1].data,
+princ-realm.data, princ-realm.length) == 0) {
+return false;
+}
+
+return true;
+}
+
 static krb5_error_code ipadb_verify_pac(krb5_context context,
 unsigned int flags,
 krb5_const_principal client_princ,
-krb5_db_entry *client,
+krb5_db_entry *server,
+krb5_db_entry *krbtgt,
 krb5_keyblock *server_key,
 krb5_keyblock *krbtgt_key,
 krb5_timestamp authtime,
@@ -565,6 +582,8 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
 krb5_pac *pac)
 {
 krb5_authdata **authdata = NULL;
+krb5_keyblock *srv_key = NULL;
+krb5_keyblock *priv_key = NULL;
 krb5_error_code kerr;
 krb5_ui_4 *buffer_types = NULL;
 size_t num_buffers;
@@ -598,8 +617,30 @@ static krb5_error_code ipadb_verify_pac(krb5_context context,
 goto done;
 }
 
+/* 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).
+ * But when a trusted realm passes us a PAC the kdc checksum is
+ * generated with that realm krbtgt key, so we need to use the cross
+ * realm krbtgt to check the 'server' checksum instead. */
+if (is_cross_realm_krbtgt(krbtgt-princ)) {
+/* krbtgt from a trusted realm */
+
+/* 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 {
+/* krbtgt from our own realm */
+priv_key = krbtgt_key;
+}
+
 kerr = krb5_pac_verify(context, old_pac, authtime,
-client_princ, krbtgt_key, NULL);
+client_princ, srv_key, priv_key);
 if (kerr) {
 goto done;
 }
@@ -684,9 +725,8 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
 }
 
 if (!is_as_req) {
-kerr = ipadb_verify_pac(context, flags,
-ks_client_princ, client,
-server_key, krbtgt_key,
+kerr = ipadb_verify_pac(context, flags, ks_client_princ,
+server, krbtgt, server_key, krbtgt_key,
 authtime, tgt_auth_data, pac);
 if (kerr != 0) {
 goto done;
-- 
1.7.7.1

___
Freeipa-devel