Bug 267827 - No TGT from KDC after SA-22:14.heimdal
Summary: No TGT from KDC after SA-22:14.heimdal
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: 13.1-RELEASE
Hardware: amd64 Any
: --- Affects Many People
Assignee: Cy Schubert
URL: https://cgit.freebsd.org/doc/commit/?...
Keywords:
Depends on:
Blocks:
 
Reported: 2022-11-17 09:39 UTC by Peter Much
Modified: 2022-12-01 01:01 UTC (History)
7 users (show)

See Also:


Attachments
Revert part of security patch (1.35 KB, patch)
2022-11-17 16:03 UTC, Cy Schubert
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Much 2022-11-17 09:39:09 UTC
After updating 13.1-RELEASE-p3 to 13.1-RELEASE-p4, the KDC does not provide TGTs when running kinit. The error is 
kinit: krb5_get_init_creds: Clock skew too great

The same error appears with kinit from security/krb5 (MIT), so the issue is with the KDC in base.

On the KDC an error is logged:
kdc[6798]: Too large time skew, client time 1970-01-01T01:00:00 is out by 1668625860 > 300 seconds
Comment 1 Cy Schubert freebsd_committer freebsd_triage 2022-11-17 14:16:26 UTC
What is uname -a on all affected machines?

date -u on all affected machines.

echo $TZ on all affected machines.

zdump /etc/localtime on all affected machines.
Comment 2 M. Voorhis 2022-11-17 14:22:36 UTC
I can attempt to answer Schubert's question, but the number of systems is large.  I use a FreeBSD-heimdal server to do authentication across several dozen FreeBSD and Linux machines of different ages.  There is a good amount of diversity in the operating systems, versions, distros, and the like.

All this stuff worked until I updated the kerberos machine to the latest 12-STABLE.

Regarding the accuracy of time, all these machines are tied to a single pair of NTP servers and I would not expect time to be off more than some very small amount across the whole group.

Sorry for lack of more detail; I am in haste right now...
Comment 3 Cy Schubert freebsd_committer freebsd_triage 2022-11-17 14:40:41 UTC
philip@,

What was your experience on the FreeBSD cluster last week?
Comment 4 Peter Much 2022-11-17 14:41:58 UTC
admin@kerb:501:1~$ uname -a
FreeBSD **** 13.1-RELEASE-p4 FreeBSD 13.1-RELEASE-p4[adf58aa95340=95499c45614c+30] ZE6R13V1 amd64
admin@kerb:502:1~$ date -u
Thu Nov 17 14:37:29 UTC 2022
admin@kerb:503:1~$ echo $TZ

admin@kerb:504:1~$ zdump /etc/localtime
/etc/localtime  Thu Nov 17 15:37:35 2022 CET

admin@disp:528:1~$ uname -a
FreeBSD **** 13.1-RELEASE-p4 FreeBSD 13.1-RELEASE-p4[adf58aa95340=95499c45614c+30] D6R13V1 amd64
admin@disp:529:1~$ date -u
Thu Nov 17 14:36:50 UTC 2022
admin@disp:530:1~$ echo $TZ

admin@disp:531:1~$ zdump /etc/localtime
/etc/localtime  Thu Nov 17 15:36:59 2022 CET
admin@disp:532:1~$
Comment 5 Peter Much 2022-11-17 14:44:00 UTC
I could isolate the offending code, it is crypto/heimdal/lib/asn1/gen_free.c

This is apparently a use after free.
Comment 6 Cy Schubert freebsd_committer freebsd_triage 2022-11-17 15:43:48 UTC
(In reply to Peter Much from comment #5)
Good sleuthing.

This patch:

diff --git a/crypto/heimdal/lib/asn1/gen_free.c b/crypto/heimdal/lib/asn1/gen_free.cx b9cae7533b17..74449fe6ca82 100644
--- a/crypto/heimdal/lib/asn1/gen_free.c
+++ b/crypto/heimdal/lib/asn1/gen_free.c
@@ -61,6 +61,13 @@ free_type (const char *name, const Type *t, int preserve)
     case TNull:
     case TGeneralizedTime:
     case TUTCTime:
+        /*
+         * This doesn't do much, but it leaves zeros where garbage might
+         * otherwise have been found.  Gets us closer to having the equivalent
+         * of a memset()-to-zero data structure after calling the free
+         * functions.
+         */
+        fprintf(codefile, "*%s = 0;\n", name);
        break;
     case TBitString:
        if (ASN1_TAILQ_EMPTY(t->members))


Does not appear in the upstream repo but is in the patch that was given to us.

Can you remove this, please, and test?
Comment 7 Cy Schubert freebsd_committer freebsd_triage 2022-11-17 16:03:50 UTC
Created attachment 238141 [details]
Revert part of security patch

Based on Peter's sleuthing this patch reverts part of the security patch.

Looking at the upstream repo I cannot find this patch. A cursory review of the git logs for this module I do not see this patch.

Please try this patch.

philip@, please test this on the cluster.
Comment 8 Peter Much 2022-11-17 16:37:54 UTC
I can confirm, I reverted that part and thus got my things running again.

I found that patch in the heimdal/heimdal repo, but looking closer, it is actually there only for versions from 7.1 onwards.
Comment 9 Cy Schubert freebsd_committer freebsd_triage 2022-11-17 17:20:24 UTC
(In reply to Peter Much from comment #8)
Thank you for testing.

I didn't spend a lot of time searching through their repo before switching the KVM from my FreeBSD laptop to $JOB laptop this morning.
Comment 10 Cy Schubert freebsd_committer freebsd_triage 2022-11-17 17:29:50 UTC
philip@, MFC after 3 days or immediately?
Comment 11 commit-hook freebsd_committer freebsd_triage 2022-11-17 17:36:03 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=f556a05c49261af3d373c599d05fa250f3563b59

commit f556a05c49261af3d373c599d05fa250f3563b59
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2022-11-17 15:43:29 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2022-11-17 17:29:17 +0000

    heimdal: Fix: Too large time skew, client time 1970-01-01T01:00:00

    Part of ed549cb0c53f zeroed out a data structure in the resulting code-file
    when a TUTCTime type was freed. This part of the patch applies to Heimdal
    7.1+ and not our Heimdal 1.5.2.

    PR:             267827
    Reported by:    Peter Much <pmc@citylink.dinoex.sub.org>
    Tested by:      Peter Much <pmc@citylink.dinoex.sub.org>
    Fixes:          ed549cb0c53f
    MFC after:      TBD with philip@

 crypto/heimdal/lib/asn1/gen_free.c | 7 -------
 1 file changed, 7 deletions(-)
Comment 12 Philip Paeps freebsd_committer freebsd_triage 2022-11-18 01:03:13 UTC
(I wonder why this didn't show up in the FreeBSD.org cluster.  It feels like something that should affect every KDC.)

I'm doing a new freebsd.org cluster build with this patch now.  I'll deploy it on our Kerberos servers as soon as it completes.

I think this should be insta-MFC-ed and we should issue an update to the SA.

Let me test a cluster build and report back.
Comment 13 commit-hook freebsd_committer freebsd_triage 2022-11-18 01:10:21 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=b23fe6badebad9a9b2022e95b50451a41c7b4f7a

commit b23fe6badebad9a9b2022e95b50451a41c7b4f7a
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2022-11-17 15:43:29 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2022-11-18 01:09:42 +0000

    heimdal: Fix: Too large time skew, client time 1970-01-01T01:00:00

    Part of ed549cb0c53f zeroed out a data structure in the resulting code-file
    when a TUTCTime type was freed. This part of the patch applies to Heimdal
    7.1+ and not our Heimdal 1.5.2.

    PR:             267827
    Reported by:    Peter Much <pmc@citylink.dinoex.sub.org>
    Tested by:      Peter Much <pmc@citylink.dinoex.sub.org>
    Fixes:          ed549cb0c53f

    (cherry picked from commit f556a05c49261af3d373c599d05fa250f3563b59)

 crypto/heimdal/lib/asn1/gen_free.c | 7 -------
 1 file changed, 7 deletions(-)
Comment 14 commit-hook freebsd_committer freebsd_triage 2022-11-18 01:11:22 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=5afe36c8b79547cda2bdd7297e5e2507a9135945

commit 5afe36c8b79547cda2bdd7297e5e2507a9135945
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2022-11-17 15:43:29 +0000
Commit:     Cy Schubert <cy@FreeBSD.org>
CommitDate: 2022-11-18 01:10:53 +0000

    heimdal: Fix: Too large time skew, client time 1970-01-01T01:00:00

    Part of ed549cb0c53f zeroed out a data structure in the resulting code-file
    when a TUTCTime type was freed. This part of the patch applies to Heimdal
    7.1+ and not our Heimdal 1.5.2.

    PR:             267827
    Reported by:    Peter Much <pmc@citylink.dinoex.sub.org>
    Tested by:      Peter Much <pmc@citylink.dinoex.sub.org>
    Fixes:          ed549cb0c53f

    (cherry picked from commit f556a05c49261af3d373c599d05fa250f3563b59)

 crypto/heimdal/lib/asn1/gen_free.c | 7 -------
 1 file changed, 7 deletions(-)
Comment 15 Cy Schubert freebsd_committer freebsd_triage 2022-11-18 01:12:33 UTC
(In reply to Philip Paeps from comment #12)
Thanks Philip. The patch has been MFC'd.
Comment 16 Peter Much 2022-11-18 02:09:39 UTC
(In reply to Philip Paeps from comment #12)
[on every KDC?]
This is a good question, and I am a fan of root-cause-analysis. I did a test with a rather vanilla installation of a recent 13-stable snapshot, to rule out my bunch of local patches and fancies. And this did reproduce.
Then I tried to figure where exactly this free is called from, and found that it is called 101 times for a single TGT. But before I could then arm a debugger, Cy came up with that this is not even supposed to work on our version, and so I gave up on it.
My high-level conclusion is: the heimdal in base should be upgraded.
Comment 17 Cy Schubert freebsd_committer freebsd_triage 2022-11-18 02:42:50 UTC
(In reply to Peter Much from comment #16)
That (7.7.0) is currently a git branch here: yet another ping from philip@. Unfortunately there are a number of serious regressions. I intend to get back at it once I dig myself out of the current higher priority projects.

Note that once the regressions are resolved, this is just the beginning. An exp-run will need to be run by portmgr. I'm sure there will be a lot of broken ports because of this.
Comment 18 Philip Paeps freebsd_committer freebsd_triage 2022-11-18 03:00:12 UTC
I am also a fan of root cause analysis ... but even more of automated testing!  It's more than likely that this escaped because I didn't test the patches enough in the freebsd.org cluster before they were made public.

I agree that we need to upgrade Heimdal in base.  However, I am not convinced that we need to continue shipping the KDC.  I wonder how much work it would be to make the Heimdal in base "private" similar to how we isolate expat as libbsdxml or unbound as local-unbound.

We absolutely do need to keep shipping the libraries and the user-facing tools.  We need them for NFS.  I would argue that the KDC belongs in ports though.  That's probably a discussion for arch@ rather than here.

Meanwhile, it looks like the cluster build for 13 failed:

Building 13amd64@n253104-37ddf393f85b, log at log.13amd64.n253104-37ddf393f85b
+ time ./build.sh -a amd64 -b 13
+ [ 2 -ne 0 ]
+ echo '*** 13amd64 build failed! ***'
*** 13amd64 build failed! ***

I will check if that's because of the Heimdal patch or something else.
Comment 19 Philip Paeps freebsd_committer freebsd_triage 2022-11-18 03:30:38 UTC
(The build failure on stable/13 does not seem related to Kerberos -- it's in ufs/ffs)
Comment 20 Philip Paeps freebsd_committer freebsd_triage 2022-11-18 06:35:50 UTC
I upgraded krb1.nyi and forced admin.nyi to use it:

root@krb1.nyi:~ # grep build= /etc/motd
role=simplejail build=amd64_13@n252921-ba47974c843
philip@admin.nyi:~ % grep kdc /etc/krb5.conf 
		kdc = udp/krb1.nyi.freebsd.org
philip@admin.nyi:~ % kinit
philip@FREEBSD.ORG's Password: 
philip@admin.nyi:~ % klist
Credentials cache: FILE:/tmp/krb5cc_1003
        Principal: philip@FREEBSD.ORG

  Issued                Expires               Principal
Nov 18 06:29:50 2022  Nov 18 14:29:50 2022  krbtgt/FREEBSD.ORG@FREEBSD.ORG

That looks good.  I'm sure that worked last week too though.  I'll upgrade krb2 and krb-master next and keep a close eye on everything that authenticates with it.

If someone who isn't me can confirm that this fix works, I'll prepare a patch for freebsd-update and an update to SA-22:14.heimdal.
Comment 21 Philip Paeps freebsd_committer freebsd_triage 2022-11-18 07:21:22 UTC
Everything still works with all monkey patches removed from krb1 and krb2 and running completely clean stable/13 builds.  I haven't put a clean build on our krb-master yet, because that's a little disruptive to reboot.  Since krb-master only provides iprop to krb1 and krb2, I believe this environment is sufficiently clean.

root@krb1.nyi:~ # grep build= /etc/motd
role=simplejail build=amd64_13@n253109-c2a74ca2c3d9
root@krb2.nyi:~ # grep build= /etc/motd
role=simplejail build=amd64_13@n253109-c2a74ca2c3d9

I've also tested kinit philip/ldap and using the TGT to get a service ticket for LDAP.  Both the TGT and the service ticket work.

Again: I'd really like someone else to confirm on another system please.
Comment 22 M. Voorhis 2022-11-18 12:36:33 UTC
The patch removing "fprintf(codefile, "*%s = 0;\n", name);" from the switch block for TUTCTime (et.al.), line 70 of src/crypto/heimdal/lib/asn1/gen_free.c restored service in my case, across the many machines.

But I'm running 12-STABLE.  I do not have a FreeBSD 13-STABLE Kerberos server right now, so I can't do the testing you request.
Comment 23 Philip Paeps freebsd_committer freebsd_triage 2022-11-18 13:08:32 UTC
Thanks for testing!  The version of Heimdal in stable/12 is the same as in stable/13.  You've confirmed my result.
Comment 24 Cy Schubert freebsd_committer freebsd_triage 2022-11-22 18:39:29 UTC
fixed.
Comment 25 fillips.grisly-0a 2022-11-22 19:12:03 UTC
Thanks for fixing this. Do you know how the fix will be delivered to users who don’t build the system from source and rely on freebsd-update?
Comment 26 Ed Maste freebsd_committer freebsd_triage 2022-11-22 19:20:35 UTC
(In reply to fillips.grisly-0a from comment #25)
It will be updated in a followup advisory
Comment 27 commit-hook freebsd_committer freebsd_triage 2022-11-29 23:16:19 UTC
A commit in branch releng/13.1 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=10571c04c9ddb65aacdd71d7d86c5a993f506e01

commit 10571c04c9ddb65aacdd71d7d86c5a993f506e01
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2022-11-17 15:43:29 +0000
Commit:     Gordon Tetlow <gordon@FreeBSD.org>
CommitDate: 2022-11-29 23:04:48 +0000

    heimdal: Fix: Too large time skew, client time 1970-01-01T01:00:00

    Part of ed549cb0c53f zeroed out a data structure in the resulting code-file
    when a TUTCTime type was freed. This part of the patch applies to Heimdal
    7.1+ and not our Heimdal 1.5.2.

    PR:             267827
    Reported by:    Peter Much <pmc@citylink.dinoex.sub.org>
    Tested by:      Peter Much <pmc@citylink.dinoex.sub.org>
    Approved by:    so
    Security:       FreeBSD-EN-22:28.heimdal
    Fixes:          ed549cb0c53f

    (cherry picked from commit f556a05c49261af3d373c599d05fa250f3563b59)
    (cherry picked from commit b23fe6badebad9a9b2022e95b50451a41c7b4f7a)

 crypto/heimdal/lib/asn1/gen_free.c | 7 -------
 1 file changed, 7 deletions(-)
Comment 28 commit-hook freebsd_committer freebsd_triage 2022-11-29 23:16:22 UTC
A commit in branch releng/12.3 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=62b8c69d298c08bb443e9e27d26bd5b1ebcd7175

commit 62b8c69d298c08bb443e9e27d26bd5b1ebcd7175
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2022-11-17 15:43:29 +0000
Commit:     Gordon Tetlow <gordon@FreeBSD.org>
CommitDate: 2022-11-29 23:04:04 +0000

    heimdal: Fix: Too large time skew, client time 1970-01-01T01:00:00

    Part of ed549cb0c53f zeroed out a data structure in the resulting code-file
    when a TUTCTime type was freed. This part of the patch applies to Heimdal
    7.1+ and not our Heimdal 1.5.2.

    PR:             267827
    Reported by:    Peter Much <pmc@citylink.dinoex.sub.org>
    Tested by:      Peter Much <pmc@citylink.dinoex.sub.org>
    Approved by:    so
    Security:       FreeBSD-EN-22:28.heimdal
    Fixes:          ed549cb0c53f

    (cherry picked from commit f556a05c49261af3d373c599d05fa250f3563b59)
    (cherry picked from commit 5afe36c8b79547cda2bdd7297e5e2507a9135945)

 crypto/heimdal/lib/asn1/gen_free.c | 7 -------
 1 file changed, 7 deletions(-)
Comment 29 commit-hook freebsd_committer freebsd_triage 2022-11-29 23:18:24 UTC
A commit in branch releng/12.4 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=4ff214b2f2719d001d6497c675b5a0e6776ece23

commit 4ff214b2f2719d001d6497c675b5a0e6776ece23
Author:     Cy Schubert <cy@FreeBSD.org>
AuthorDate: 2022-11-17 15:43:29 +0000
Commit:     Gordon Tetlow <gordon@FreeBSD.org>
CommitDate: 2022-11-29 23:17:39 +0000

    heimdal: Fix: Too large time skew, client time 1970-01-01T01:00:00

    Part of ed549cb0c53f zeroed out a data structure in the resulting code-file
    when a TUTCTime type was freed. This part of the patch applies to Heimdal
    7.1+ and not our Heimdal 1.5.2.

    PR:             267827
    Reported by:    Peter Much <pmc@citylink.dinoex.sub.org>
    Tested by:      Peter Much <pmc@citylink.dinoex.sub.org>
    Approved by:    so, re (implicit)
    Security:       FreeBSD-EN-22:28.heimdal
    Fixes:          ed549cb0c53f

    (cherry picked from commit f556a05c49261af3d373c599d05fa250f3563b59)
    (cherry picked from commit 5afe36c8b79547cda2bdd7297e5e2507a9135945)

 crypto/heimdal/lib/asn1/gen_free.c | 7 -------
 1 file changed, 7 deletions(-)