Some patches for import-lorikeet-heimdal branch

[Isaac Boukris]

On Fri, Nov 2, 2018 at 7:13 PM Andrew Bartlett <abartlet at samba.org> wrote:
>
> On Fri, 2018-11-02 at 14:13 +0100, Isaac Boukris via samba-technical
> wrote:
> > On Wed, Oct 31, 2018 at 10:17 PM Isaac Boukris <iboukris at gmail.com> wrote:
> > > I've made a patch for this, along with some other stuff I found on the
> > > way. Please take a look at:
> > > https://gitlab.com/samba-team/devel/samba/commits/iboukris_lorikeet_import_round_one
> > >
> > > $ make test TESTS=samba4.krb5.kdc SAMBA_OPTIONS="-d3" >out.log 2>err.log
> > >
> > > $ grep failure before_patch.log |wc -l
> > > 1750
> > >
> > > $ grep failure after_patch.log |wc -l
> > > 1694
> > >
> > > $ grep -A1 failure before_patch.log | grep Exception | cut -d ' ' -f4 | sort -u
> > > ../source4/torture/krb5/kdc-canon-heimdal.c:1783:
> > > ../source4/torture/krb5/kdc-canon-heimdal.c:1950:
> > > ../source4/torture/krb5/kdc-canon-heimdal.c:2085:
> > > ../source4/torture/krb5/kdc-canon-heimdal.c:2153:
> > > ../source4/torture/krb5/kdc-heimdal.c:812:
> > > ../source4/torture/krb5/kdc-heimdal.c:824:
> > >
> > > $ grep -A1 failure after_patch.log | grep Exception | cut -d ' ' -f4 | sort -u
> > > ../source4/torture/krb5/kdc-canon-heimdal.c:1783:
> > > ../source4/torture/krb5/kdc-canon-heimdal.c:1950:
> > > ../source4/torture/krb5/kdc-canon-heimdal.c:1998:
> > > ../source4/torture/krb5/kdc-canon-heimdal.c:2377:
> > > ../source4/torture/krb5/kdc-heimdal.c:812:
> > > ../source4/torture/krb5/kdc-heimdal.c:824:
> >
> > I found another change of behavior in the new client library code, and
> > added a new commit to the branch with a temporary fix.
> > This lowers the number of failures to 1444, and the assertion points
> > down by one!
> >
> > $ grep -A1 failure new.log | grep Exception | cut -d ' ' -f4 | sort -u
> > ../source4/torture/krb5/kdc-canon-heimdal.c:1795:
> > ../source4/torture/krb5/kdc-canon-heimdal.c:1962:
> > ../source4/torture/krb5/kdc-canon-heimdal.c:2026:
> > ../source4/torture/krb5/kdc-heimdal.c:812:
> > ../source4/torture/krb5/kdc-heimdal.c:824:
>
> w00t!
>
> Seriously,


Thanks!

Note that I added another patch which seem correct, but has revealed
more assertion failure points. I think some of these failures are
still related to the of order order between capath and referrals and I
need to look more carefully on all TGS callbacks and expectations.

Meanwhile, I ported one of the patches to the master branch, since the
bug is actually in samba - see attached.
Pipeline: https://gitlab.com/samba-team/devel/samba/pipelines/35330420
-------------- next part --------------
From c01464809fe9bac21975ce1d974a202eaf4f19fa Mon Sep 17 00:00:00 2001
From: Isaac Boukris <iboukris at gmail.com>
Date: Wed, 31 Oct 2018 22:09:58 +0200
Subject: [PATCH] sdb: always canonicalize TGS server realm

even for enterprise server principal, same as windows.

We have a torture test for this, which only works due
 due to another bug in tgs_build_reply() (which got fixed
in upstream) heimdal, where we'd miss a goto after setting
KRB5KRB_AP_ERR_NOT_US, so fix that too.

Signed-off-by: Isaac Boukris <iboukris at gmail.com>
---
 source4/heimdal/kdc/krb5tgs.c | 1 +
 source4/kdc/db-glue.c         | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/source4/heimdal/kdc/krb5tgs.c b/source4/heimdal/kdc/krb5tgs.c
index a888788bb6f..a5feee906d9 100644
--- a/source4/heimdal/kdc/krb5tgs.c
+++ b/source4/heimdal/kdc/krb5tgs.c
@@ -1827,6 +1827,7 @@ server_lookup:
 	if(ret == 0)
 	    free(ktpn);
 	ret = KRB5KRB_AP_ERR_NOT_US;
+	goto out;
     }
 
     ret = hdb_enctype2key(context, &krbtgt_out->entry,
diff --git a/source4/kdc/db-glue.c b/source4/kdc/db-glue.c
index acd24ec0c83..dd0855e6a9c 100644
--- a/source4/kdc/db-glue.c
+++ b/source4/kdc/db-glue.c
@@ -932,7 +932,9 @@ static krb5_error_code samba_kdc_message2entry(krb5_context context,
 			goto out;
 		}
 
-		if (smb_krb5_principal_get_type(context, principal) != KRB5_NT_ENTERPRISE_PRINCIPAL) {
+                /* Can we get rid of the if ? */
+		if (smb_krb5_principal_get_type(context, principal) != KRB5_NT_ENTERPRISE_PRINCIPAL ||
+                    (flags & SDB_F_FOR_TGS_REQ)) {
 			/* While we have copied the client principal, tests
 			 * show that Win2k3 returns the 'corrected' realm, not
 			 * the client-specified realm.  This code attempts to
-- 
2.14.4