Created attachment 14669 [details]
demonstrator

This demonstration code from Isaac Boukris shows that a MITM (here the send to kdc proxy hook) can replace the S4U2Self data and use only a crc32 checksum.

Patch is against Samba.

See attached patch that demonstrate how the for_user can be overridden
on the wire to administrator.
To test, run:
$ bin/samba4kinit -e des-cbc-crc service@REALM
$ bin/samba4kgetcred --out-cache=out --impersonate=user$REALM service@REALM
$ klist -c out

(In reply to Andrew Bartlett from comment #1)
Note that we later found out that the interception works even with a normal tgt, that is the kinit needs not be DES based. Any kinit would do.

Also, if you want to run the patch, make sure to have a default realm in krb5.conf (or add any realm when parsing the administrator name in the patch).

Created attachment 14810 [details]
Initial patch

Hi Andrew, I worked out a patch meanwhile, with a test based on the demo code. The test ended up being longer than I imagined, hope it looks good. Regards.

Created attachment 14817 [details]
v2 patch

Comment on attachment 14817 [details]
v2 patch

(In reply to Isaac Boukris from comment #4)
Thank you so much.

That looks great.  I'm running some private CI on it just to be sure.

This is a challenge to score, as the attack depends entirely on what happens to the ticket obtained via S4U2Self and potentially S4U2Proxy.

However I'm assuming the worst and so score it as:

CVSS:3.0/AV:N/AC:H/PR:L/UI:N/S:U/C:H/I:H/A:H

I'll write an advisory shortly.

Isaac, I guess you also checked with MIT Kerberos and there is no issue as we don't support S4U* yet?

Created attachment 15094 [details]
a very rough first advisory

Isaac,

Can you confirm any affiliation you want noted and  that you are happy to be publicly credited for all your hard work on this.

Also please see if you can help make this document clearer, specifically we need to introduce s4u2self and s4u2proxy by the other names (Delegation?) that folks might know the features by. 

Thanks

(In reply to Andreas Schneider from comment #7)
Andreas, MIT KDC is not affected as it rejects unkeyed checksums, see:
https://github.com/krb5/krb5/blob/01319a310cf06e4139b65afb12f998dbea636103/src/kdc/kdc_util.c#L1169

Created attachment 15096 [details]
patch for master with knownfails (WIP v3)

This patch for master splits the test from the fix.

I think having the tests in the monster krb5.kdc.canon test is a mistake, it is far to complex in there and it should be pulled out. 

Isaac,

If you could help me with an untangled fix that would be most useful, as it will be much easier to backport.

Created attachment 15097 [details]
remaining failures after fix

By splitting the tests from the fix is is clear that these are some kind of regression from the fix or a bad test interaction.

This needs to be understood better before we can proceed.

Created attachment 15098 [details]
patch for master with knownfails (WIP v4)

This version of the patch skips the problematic combination.  Please check carefully that this is is not masking a genuine regression!

(In reply to Andrew Bartlett from comment #12)
Andrew , thanks for figuring out the mistakes and sorry for the careless testing.
Your changes look okay to me as they are, though I'm trying to work out a simpler alternative.

Also, I think we should grep out line not including mitm-s4u2self in the mitm-s4u2self file.

Created attachment 15100 [details]
Advisory v2

Andrew,
Meanwhile, I added a new version of the advisory with some changes and additions, I might have expanded too much, please take a look.

Comment on attachment 15098 [details]
patch for master with knownfails (WIP v4)

This (v4) patched passed a private GitLab CI on master.

Created attachment 15102 [details]
patch for master v5

Since the remaining failures happen after the mitm-s4u2self testing, I think it is simpler to just bail out once we done testing it. Note that if s4u2self succeeds it saves the ticket in the ccache, while now it may fail and the state won't be the same as after successful s4u2self request.
Please take a look (passes make test TESTS=krb5.kdc).

Comment on attachment 15102 [details]
patch for master v5

Thanks, that looks better.

Comment on attachment 15100 [details]
Advisory v2

Thanks, this looks good.  Really hard to explain this stuff but this makes it clear enough for our users.

Created attachment 15103 [details]
patch for master with CVE (v6)

Created attachment 15104 [details]
patch for 4.9 (v6)

Created attachment 15106 [details]
patch for 4.8 (v6)

Created attachment 15107 [details]
patch for 4.5 (v6)

The patch for master has passed a GitLab CI, I'll add the remaining ticks from myself when they each pass CI.

I believe it is worthwhile mentioning the formal name for the protocol extensions published by Microsoft:

  [MS-SFU]: Kerberos Protocol Extensions: Service for User and Constrained Delegation Protocol

and the Microsoft web site that provides the description of the protocols

  https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-sfu/

The Protocol Examples pages show clearly how the protocols can be used within a single realm or cross-realm.

Although it is true that Microsoft implemented SFU2Self as a method to acquire the PAC for a user, the fact is that what is returned to the requesting service is a valid Kerberos ticket and not just the authenticated user's authorization information.  Even though the MS-SFU protocol extensions are not IETF standardized they have been implemented for many years by non-Microsoft KDCs.  

SFU2Self might be used by a web service authenticating an end user via OAuth, Shibboleth, or other protocols to obtain a Kerberos service ticket for use by any Kerberos service principal the web service has a keytab for.  One example is acquiring an AFS token by requesting an afs/cell@REALM service ticket for a client via SFU2Self.  With this exploit an organization that deploys a KDC built from Heimdal (Heimdal, Samba, macOS, OneIdentity, etc) is vulnerable to privilege escalation attacks.

It is out of scope for Samba to describe all of the possible tool chains that might be vulnerable.  However, it would be useful for the same CVE to cover both Heimdal and Samba.   MS-SFU functionality was introduced in Heimdal-0.8.  All releases of Heimdal through 7.5.0 are vulnerable as are any products that ship a KDC derived from one of those Heimdal releases.

(In reply to Jeffrey Altman from comment #24)
Thanks for the feedback, I'll find a way to include this information in the advisory.

Created attachment 15108 [details]
advisory with CVE (v3)

G'Day Jeffrey,

I've added some of the text and credited you, thank you very much for the suggestions.  Please let me know if that is OK and if you wish to have an affiliation included.

I've not named other vendors, as it isn't my place to make claims about their security status.

I've included the Heimdal versions in the header as requested, which should let you use the same text for your advisory.

Created attachment 15109 [details]
patch for 4.10 (v6)

Created attachment 15110 [details]
Modified version of the advisory.

Andrew, here is a somewhat re-written version of the advisory. I think it makes things clearer. Please let me know if this version works for you. I'm not replacing the old version with this attachment in case you hate it :-).

Jeremy.

Comment on attachment 15110 [details]
Modified version of the advisory.

Bah, uploaded wrong text, sorry.

Created attachment 15111 [details]
CVE-2018-16860-advisory-04

Correct re-written version.

Comment on attachment 15111 [details]
CVE-2018-16860-advisory-04

Much better, thanks!

Comment on attachment 15104 [details]
patch for 4.9 (v6)

version here is 4.10 not 4.9

Created attachment 15112 [details]
patch for 4.9 (v6)

Thanks to Tim for spotting the wrong file uploaded as the 4.9 v6 patch.

Andrew, 

The new advisory text looks good to me.

Thanks.

Jeffrey

Comment on attachment 15106 [details]
patch for 4.8 (v6)

After increasing the runner resources, this (the 4.8 patch) finally passed.  All the patches have passed CI.

Comment on attachment 15107 [details]
patch for 4.5 (v6)

And the patch on Samba 4.5 finally passed CI (important for Debian Stable).

The release date for this issue is set per bug 13911 which you will also have access to. 

NOTE: Please keep an eye on these bugs for confirmation closer to the release in case administrative reasons cause a change in the date.

Samba 4.10.3, 4.9.8 and 4.8.12 have been shipped to address this defect.

Pushed to v4-{10,9,8}-test.

Pushed to autobuild-master.

(In reply to Karolin Seeger from comment #40)
Pushed to master.
Re-assigning to Andrew to decide whether the bug report should be marked public before closing.

This is Microsoft's advisory for the same issue:

https://portal.msrc.microsoft.com/en-US/security-guidance/advisory/CVE-2019-0734

Heimdal uses essentially the same patch as Samba:

https://github.com/heimdal/heimdal/commit/c6257cc2c842c0faaeb4ef34e33890ee88c4cbba

Opening report, this is all quite public now.  

Microsoft's report credits us (as is right) so the link between the multiple vendors here is public.

Removing samba-vendor from this now public bug, feel free to CC yourself if you wish to follow further.

A Debian user reports that Samba 4.5 now segfault (clearly I don'tsee how it is related).

[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
0x00007f9cb561fffa in __GI___waitpid (pid=30937, stat_loc=stat_loc@entry=0x7fff0bf81b10, options=options@entry=0) at ../sysdeps/unix/sysv/linux/waitpid.c:29
#0  0x00007f9cb561fffa in __GI___waitpid (pid=30937, stat_loc=stat_loc@entry=0x7fff0bf81b10, options=options@entry=0) at ../sysdeps/unix/sysv/linux/waitpid.c:29
#1  0x00007f9cb55a70ab in do_system (line=<optimized out>) at ../sysdeps/posix/system.c:148
#2  0x00007f9cb6cd0d31 in smb_panic_s3 () from /usr/lib/x86_64-linux-gnu/libsmbconf.so.0
#3  0x00007f9cb91ed19f in smb_panic () from /usr/lib/x86_64-linux-gnu/libsamba-util.so.0
#4  0x00007f9cb91ed3b6 in ?? () from /usr/lib/x86_64-linux-gnu/libsamba-util.so.0
#5  <signal handler called>
#6  0x00007f9cb8ddf77b in smbXsrv_session_create () from /usr/lib/x86_64-linux-gnu/samba/libsmbd-base.so.0
#7  0x00007f9cb8d73a17 in reply_sesssetup_and_X () from /usr/lib/x86_64-linux-gnu/samba/libsmbd-base.so.0
#8  0x00007f9cb8db1086 in ?? () from /usr/lib/x86_64-linux-gnu/samba/libsmbd-base.so.0
#9  0x00007f9cb8db34ba in ?? () from /usr/lib/x86_64-linux-gnu/samba/libsmbd-base.so.0
#10 0x00007f9cb8db44ac in ?? () from /usr/lib/x86_64-linux-gnu/samba/libsmbd-base.so.0
#11 0x00007f9cb5911ea3 in ?? () from /usr/lib/x86_64-linux-gnu/libtevent.so.0
#12 0x00007f9cb5910277 in ?? () from /usr/lib/x86_64-linux-gnu/libtevent.so.0
#13 0x00007f9cb590c04d in _tevent_loop_once () from /usr/lib/x86_64-linux-gnu/libtevent.so.0
#14 0x00007f9cb590c27b in tevent_common_loop_wait () from /usr/lib/x86_64-linux-gnu/libtevent.so.0
#15 0x00007f9cb5910217 in ?? () from /usr/lib/x86_64-linux-gnu/libtevent.so.0
#16 0x00007f9cb8db57d9 in smbd_process () from /usr/lib/x86_64-linux-gnu/samba/libsmbd-base.so.0
#17 0x0000560c4d00f7b4 in ?? ()
#18 0x00007f9cb5911ea3 in ?? () from /usr/lib/x86_64-linux-gnu/libtevent.so.0
#19 0x00007f9cb5910277 in ?? () from /usr/lib/x86_64-linux-gnu/libtevent.so.0
#20 0x00007f9cb590c04d in _tevent_loop_once () from /usr/lib/x86_64-linux-gnu/libtevent.so.0
#21 0x00007f9cb590c27b in tevent_common_loop_wait () from /usr/lib/x86_64-linux-gnu/libtevent.so.0
#22 0x00007f9cb5910217 in ?? () from /usr/lib/x86_64-linux-gnu/libtevent.so.0
#23 0x0000560c4d00c4f4 in main ()


(see https://bugs.debian.org/929268)

Mathieu: This looks like https://bugzilla.samba.org/show_bug.cgi?id=13315