Bug 14233 - Follow-up to bug 14187: DelegationNotAllowed on server account
Summary: Follow-up to bug 14187: DelegationNotAllowed on server account
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Andrew Bartlett
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-01-13 12:24 UTC by Isaac Boukris
Modified: 2020-06-26 13:41 UTC (History)
6 users (show)

See Also:


Attachments
wip patch for heimdal (957 bytes, patch)
2020-01-13 22:58 UTC, Isaac Boukris
no flags Details
full patch (2.94 KB, patch)
2020-01-14 12:53 UTC, Isaac Boukris
no flags Details
patch for master (6.41 KB, patch)
2020-01-19 15:55 UTC, Isaac Boukris
iboukris: review+
metze: review-
iboukris: ci-passed+
Details
basic patch (5.99 KB, patch)
2020-01-23 12:21 UTC, Isaac Boukris
iboukris: review+
iboukris: review? (metze)
iboukris: review? (abartlet)
iboukris: ci-passed+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Isaac Boukris 2020-01-13 12:24:37 UTC

    
Comment 1 Isaac Boukris 2020-01-13 12:42:12 UTC
This needs more research, but I think the fix in bug 14187 is incomplete as we also need to apply disallow-forwardable in TGS request, if the flag is set on the requested server principal. I think it may have a meaning on trust accounts for instance.

I tested on Windows that adding DelegationNotAllowed on the client has
no impact on TGS request (if he already had a forwardable tgt), but if the server has the flag Windows does strip the forwardable flag in TGS request, while in heimdal we do not (we now have the same problem in our MIT KDB module due to my hack, but the fix is the module, not upstream like in heimdal).

On Windows if I add DelegationNotAllowed to a server principal I get:
$ echo -n pwd | kinit administrator@ACME.COM && kvno apache@ACME.COM &&
klist -f
Password for administrator@ACME.COM: 
apache@ACME.COM: kvno = 2
Default principal: administrator@ACME.COM
Valid starting       Expires              Service principal
01/11/2020 15:46:57  01/12/2020 01:46:57  krbtgt/ACME.COM@ACME.COM
        renew until 01/18/2020 15:46:57, Flags: FRIA
01/11/2020 15:46:57  01/12/2020 01:46:57  apache@ACME.COM
        renew until 01/18/2020 15:46:57, Flags: RA


On Samba heimdal I get:
$ echo -n pwd | kinit administrator@SMB.NET && kvno apache@SMB.NET &&
klist -f
Password for administrator@SMB.NET: 
apache@MIT.NET: kvno = 2
Default principal: administrator@SMB.NET
Valid starting       Expires              Service principal
01/11/2020 15:20:48  01/12/2020 01:20:48  krbtgt/SMB.NET@SMB.NET
        renew until 01/18/2020 15:20:48, Flags: FRIA
01/11/2020 15:20:48  01/12/2020 01:20:48  apache@SMB.NET
        renew until 01/18/2020 15:20:48, Flags: FRAT
Comment 2 Isaac Boukris 2020-01-13 22:19:27 UTC
Quote from MS-KILE (note, AS or TGS):
3.3.5.1 Request Flag Ticket-issuing Behavior Kerberos V5 specifies Kerberos ticket-issuing behavior defined by the kdc-options ([RFC4120] section 5.4.1) that are passed to the KDC during the AS or TGS exchange.
Kerberos V5 specifies Kerberos TicketFlags ([RFC4120] Section 5.3) that can be set by the KDC on tickets.
KILE KDCs use the following account variables to enforce TicketFlags:
- If DelegationNotAllowed is set to TRUE on the principal (or if domainControllerFunctionality returns a value >= 6 ([MS-ADTS] section 3.1.1.3.2.25) and the principal is a member of PROTECTED_USERS ([MS-DTYP] section 2.4.2.4)), the KILE KDC MUST NOT set the PROXIABLE or FORWARDABLE ticket flags ([RFC4120] sections 2.5 and 2.6).


Note for the reproducer, you may need to use "kinit -f" (i get it from conf).
Comment 3 Isaac Boukris 2020-01-13 22:58:18 UTC
Created attachment 15726 [details]
wip patch for heimdal

With the attached I get the right behevior in heimdal, however now we get the same problem I have in MIT, we lose the forwardable flag on trust accounts for no reason, somewhere in 'db-glue.c' I guess.

$ echo -n pwd | kinit -f administrator@SMB.NET && kvno apache@SMB.NET && klist -f
Password for administrator@SMB.SMB: 
apache@SMB.NET: kvno = 2
Default principal: administrator@SMB.NET
Valid starting       Expires              Service principal
01/13/2020 23:40:20  01/14/2020 09:40:20  krbtgt/SMB.NET@SMB.NET
	renew until 01/20/2020 23:40:20, Flags: FRIA
01/13/2020 23:40:20  01/14/2020 09:40:20  apache@SMB.NET
	renew until 01/20/2020 23:40:20, Flags: RAT

$ echo -n pwd | kinit -f administrator@SMB.NET && kvno krbtgt/ACME.COM@ && klist -f
Password for administrator@SMB.NET: 
krbtgt/ACME.COM@: kvno = 0
Default principal: administrator@SMB.NET
Valid starting       Expires              Service principal
01/13/2020 23:40:35  01/14/2020 09:40:35  krbtgt/SMB.NET@SMB.NET
	renew until 01/20/2020 23:40:35, Flags: FRIA
01/13/2020 23:40:35  01/14/2020 09:40:35  krbtgt/ACME.COM@
	renew until 01/20/2020 23:40:35, Flags: RAT
	Ticket server: krbtgt/ACME.COM@SMB.NET
Comment 4 Jeffrey Altman 2020-01-14 00:29:14 UTC
Isaac,


By trust ticket do you mean a cross-realm tgt?   I believe you need to call krb5_principal_is_krbtgt() and only clear the forwardable and proxiable flags for a non-tgt.
Comment 5 Isaac Boukris 2020-01-14 01:21:56 UTC
(In reply to Jeffrey Altman from comment #4)

Yes, I mean cross-realm TGT, why not clear the flags in this case? In MIT we would like for any other server, and MS-KILE doesn't make make an exception (although I couldn't test setting DelegationNotAllowed on a trust account in AD as I get permission denied trying to change the UAC).

I think the problem now is with Samba's samba_kdc_message2entry() which for no-reason strips the forwardable in cross-realm TGT lookup (Windows doesn't), so my patch in heimdal will make samba really not set the forwardable flag in this case.
Comment 6 Isaac Boukris 2020-01-14 01:48:23 UTC
As for the impact I'm not clear either, what made me think of the trust account is upcoming windows update restricting tgt-delegation by stripping ok-as-delegate when the trust account is fetched:
https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-kile/bac4dc69-352d-416c-a9f4-730b81ababb3

I think stripping the forwardable in a similar manner might prevent the foreign target server from using a service ticket obtained with this cross-tgt to do legacy constrained-delegation to a resource in its own realm (as forwardable is required), but I'm unclear on that.
Comment 7 Isaac Boukris 2020-01-14 12:53:14 UTC
Created attachment 15730 [details]
full patch

Ok I think I figure it out, the trust case uses samba_kdc_trust_message2entry() instead and so doesn't set forwardable as windows KDC would do. It also does not set ok-as-delegate as windows KDC do unless NO_TGT_DELEGATION (MS-KILE 3.3.5.7.5).

The attached patches makes us behave like Windows for both heimdal/mit, while I need to look some more, I'd appreciate feedback on it in the meanwhile.
Comment 8 Jeffrey Altman 2020-01-14 13:02:54 UTC
I view a cross-realm tgt as a means of conveying a client request to a service that is authenticated in a foreign cell.   The cross-realm tgt is issued by the client's realm's KDC or some intermediary realm's KDC.  Why should they be imposing their own policy constraints on all services authenticated by another realm?

Although perhaps that is the point.  By setting disable-forwardable the local realm administrator is saying I trust you to use services from that other realm but I don't trust that other realm with your identity. 

Then again, if they don't trust that other realm, they shouldn't be exchanging keys.
Comment 9 Isaac Boukris 2020-01-14 13:42:06 UTC
(In reply to Jeffrey Altman from comment #8)
> I view a cross-realm tgt as a means of conveying a client request to a service that is authenticated in a foreign cell.   The cross-realm tgt is issued by the client's realm's KDC or some intermediary realm's KDC.  Why should they be imposing their own policy constraints on all services authenticated by another realm?

I agree that setting disallow-forwardable on trust account isn't that useful (unlike ok-as-delegate), but why should we make an exception for krbtgt in the kdc? the flag should not be set on the db trust account.

> Although perhaps that is the point.  By setting disable-forwardable the local realm administrator is saying I trust you to use services from that other realm but I don't trust that other realm with your identity. 

Yeah, as it may have some impact I think we should abide by it, but I didn't find any spec about it and there is no meaningful way to set it in Windows (afaik).

Also, I thinks this bug is not a security issue. Enforcing disallow-forwardable to a regular service would prevent that ticket from being used in constrained-delegation at all, but there a normal way to do that, by not setting allowed targets.
Comment 10 Isaac Boukris 2020-01-19 15:55:11 UTC
Created attachment 15744 [details]
patch for master

Patch for master including a test of using the ticket in S4U2Proxy.
Comment 11 Stefan Metzmacher 2020-01-21 09:52:30 UTC
Comment on attachment 15744 [details]
patch for master

Please don't use // comments.

Why don't we implement the
TRUST_ATTRIBUTE_CROSS_ORGANIZATION_NO_TGT_DELEGATION logic?
Comment 12 Isaac Boukris 2020-01-22 09:25:22 UTC
(In reply to Stefan Metzmacher from comment #11)

It turns out there is another attribute to take into account, but the doc isn't clear about it - sent a question to dochelp:
https://lists.samba.org/archive/cifs-protocol/2020-January/003365.html
Comment 13 Isaac Boukris 2020-01-23 12:21:41 UTC
Created attachment 15750 [details]
basic patch

New patch, not including the ok-as-delegate changes as they aren't strictly related (I'll follow up in a separate MR once I get feedback from MS).
Comment 14 Isaac Boukris 2020-01-29 14:13:45 UTC
Action plan: once the patch is reviewed I plan to get a minor CVE (since there is a case that should fail but does not), prepare all branches and join the next security release.
Comment 15 Isaac Boukris 2020-01-29 14:19:40 UTC
(In reply to Stefan Metzmacher from comment #11)

> Why don't we implement the
TRUST_ATTRIBUTE_CROSS_ORGANIZATION_NO_TGT_DELEGATION logic?

I implemented it in separate MR: 
https://gitlab.com/samba-team/samba/-/merge_requests/1086
Comment 16 Isaac Boukris 2020-02-10 15:58:14 UTC
After giving it more thoughts and consulting, I believe this is not a security concern since it is the DelegationNotAllowed is not documented for such purpose and such usage makes no sense, so it can't be misused and it certainly has no impact in practice.
If no one objects, I'll go ahead and make this bug public, add the patches to MR 1086, and submit public PRs heimdal upstream.
Comment 17 Andrew Bartlett 2020-02-10 17:33:20 UTC
I agree, make this public, I also don't see this meeting the the threshold for a security release.

I've removed the embargo.
Comment 18 Isaac Boukris 2020-02-10 20:02:58 UTC
(In reply to Andrew Bartlett from comment #17)

Thank you, i've submitted PRs heimdal upstream.
Comment 19 Isaac Boukris 2020-02-11 09:34:53 UTC
(In reply to Isaac Boukris from comment #18)

Upstream PRs merged, submitted new MR 1139 to samba, thanks!
Comment 20 Isaac Boukris 2020-06-26 13:41:50 UTC
MR 1139 is now in master closing here, thanks all!