Bug 14725 - [SECURITY] Andrew's Kerberos Concerns (November 9 2021)
Summary: [SECURITY] Andrew's Kerberos Concerns (November 9 2021)
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: AD: LDB/DSDB/SAMDB (show other bugs)
Version: 4.14.5
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Samba QA Contact
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on: 14557 14556 CVE-2020-25718 CVE-2020-25719 14563 CVE-2020-25722 14724
Blocks: 14834
  Show dependency treegraph
 
Reported: 2021-06-02 20:01 UTC by Andrew Bartlett
Modified: 2021-11-22 04:01 UTC (History)
6 users (show)

See Also:


Attachments
Combined patchset for master v12 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722) (1.09 MB, patch)
2021-11-03 15:58 UTC, Stefan Metzmacher
jsutton: review+
metze: ci-passed+
Details
Combined patchset for 4.15 v12 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722) (1.15 MB, patch)
2021-11-03 16:00 UTC, Stefan Metzmacher
jsutton: review+
metze: ci-passed+
Details
Combined patchset for 4.14 v12 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722) (1.24 MB, patch)
2021-11-03 16:01 UTC, Stefan Metzmacher
jsutton: review+
metze: ci-passed+
Details
Combined patchset for 4.13 v12 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722) (1.39 MB, patch)
2021-11-03 16:02 UTC, Stefan Metzmacher
jsutton: review+
metze: ci-passed+
Details
Patch to fix use-after-free regression on top of the combined v12 patchsets (all versions) (1.86 KB, patch)
2021-11-04 04:12 UTC, Joseph Sutton
metze: review+
metze: ci-passed+
Details
Combined patchset for master v13 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722) (1.09 MB, patch)
2021-11-04 07:35 UTC, Joseph Sutton
metze: review+
metze: ci-passed+
Details
Combined patchset for 4.15 v13 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722) (1.15 MB, patch)
2021-11-04 07:36 UTC, Joseph Sutton
metze: review+
metze: ci-passed+
Details
Combined patchset for 4.14 v13 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722) (1.23 MB, patch)
2021-11-04 07:37 UTC, Joseph Sutton
metze: review+
metze: ci-passed+
Details
Combined patchset for 4.13 v13 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722) (1.39 MB, patch)
2021-11-04 07:39 UTC, Joseph Sutton
metze: review+
metze: ci-passed+
Details
Combined patchset for 4.12 v13 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722) (fileserver tests DISABLED) (4.89 MB, patch)
2021-11-04 08:29 UTC, Joseph Sutton
abartlet: review+
metze: ci-passed+
Details
CVE-2020-25721 advisory (mirror of v2) (4.20 KB, text/plain)
2021-11-04 09:46 UTC, Andrew Bartlett
metze: review+
Details
Combined patchset for 4.10 v13 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722) (4.99 MB, patch)
2021-11-09 04:04 UTC, Joseph Sutton
abartlet: review+
Details
Fix regression in "allow trusted domains = false" v4.13 (1.17 KB, application/mbox)
2021-11-09 19:01 UTC, Andrew Walker
no flags Details
Fix regression in "allow trusted domains = false" v4.13 (1.17 KB, application/mbox)
2021-11-09 19:01 UTC, Andrew Walker
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Bartlett 2021-06-02 20:01:18 UTC

    
Comment 1 Andreas Schneider 2021-08-09 15:56:41 UTC
Created attachment 16724 [details]
wip-mit-kdb-fixes.patch

Attaching WIP patchset here. It probably covers several issues already.
Comment 3 Andrew Bartlett 2021-08-10 07:53:03 UTC
Current proposed release date is Tuesday October 12, 2021
Comment 4 Andrew Bartlett 2021-08-17 02:31:05 UTC
(In reply to Andreas Schneider from comment #1)
Once we have the PAC validation to fix the bronze-bit issues, I'm pretty sure this will fix bug 14686.  Thanks!

Tests to prove this are under development, but this is the core of the fix, once the PAC is checked in all the important codepaths.
Comment 9 Andrew Bartlett 2021-10-21 09:07:50 UTC
I hope to start creating a single branch of the master patches, at least for the AD DC soon, but currently only have some random-ish starting points.

Joseph rightly suggests starting with the tests for the KDC behaviour, so I'll probably pick his jsutton24/master-tests branch and then put the security-priv_attrs branch on that, as both of those are close to ready.

I'm currently working to sort out my abartlet/tripple-lock branch and plan to land that next.  The idea is to check that each layer passes CI before adding another, and hopefully avoid going back over the branches multiple times.
Comment 10 Andrew Bartlett 2021-10-22 20:35:18 UTC
My rough plan of action is to land:

 - jsutton24/master-tests (once test_krb5_smb is fixed and set to run on more targets)
 - security-CVE-2020-2571-member-require-pac-ok
 - abartlet/triple-lock-2 (perhaps adding one more test for $ termination).  

It is possible I can find a way to make two more tests currently knownfailed in knownfail.d/uac_objectclass_restrict work, but I think we have this better covered in newer tests that make more sense.

 - nivanova/create-child-wip once ready
 - douglas-SECURITY-upn-spn-on-triple-lock once ready
 - the KDC side changes (cherry-pick back out of other branches)
 - dsdb structural objectclass swapping test and fix (test still to be developed)
 - then evaluate any remaining patches found by rebasing the latest auth_signd branch onto this set.
Comment 11 Andrew Bartlett 2021-10-23 09:16:28 UTC
(In reply to Andrew Bartlett from comment #10)
In a WIP branch:
 abartlet/2021-11-master-release-wip-no-pac
I have landed
 SECURITY-priv_attrs
 security-CVE-2021-23192-dcerpc-middle-fraq-ok
 (the require-krb5 really meant it patches)
 abartlet/tripple-lock
 jsutton24/master-tests
 security-CVE-2020-2571-member-require-pac-ok

I need to fix up and get sign-off of the test for no_pac behaviour, so we can have a proper test/knownfail fix/remove-knownfail.  Checking for this showed that changing the error code broke the test. 

TODO next:
 - nivanova/create-child-wip (needs restructure to be on top of this branch and test/knownfail before change)
 - we need to select back out the KDC side fixes from jsutton24/auth_signd18 and apply those. 
 - douglas-SECURITY-upn-spn-on-triple-lock (once fixed)
Comment 12 Andrew Bartlett 2021-10-27 03:38:53 UTC
The patches I think are currently ready-ish are in 

 abartlet/2021-11-master-release-wip-no-pac-3

We currently have a full pass on CI minus some knownfail issues that Joseph is working on.

This includes the fileserver no-PAC changes, AD DC priv_attrs, tripple-lock and tests for the KDC issues.
Comment 13 Andreas Schneider 2021-10-27 09:39:52 UTC
I think asn-bronze-bit-ok is missing
Comment 16 Andrew Bartlett 2021-10-28 18:41:17 UTC
We will ship with the PAC being mandatory everywhere, including in the KDC, with some release notes.  This is the choice to be consistent at the expense of possible upgrade pain for a very small number of users.

Updated security-CVE-2021-23192-dcerpc-middle-fraq-ok and security-2021-11-master-release-wip-no-pac-metze branches need to be incorporated from metze.

Douglas has the UPN/SPN and ACL changes in douglas-SECURITY-upn-spn-on-triple-lock.

I'm unclear on the status of asn-bronze-bit and where this fits in with regard to the existing patches in jsutton24/abartlet/2021-11-master-release-wip-no-pac-4-kdc-fixes which is the current attempt at branch consolidation.
Comment 17 Andreas Schneider 2021-10-29 06:24:11 UTC
asn-bronze-bit is work in progress and I'm not sure if this will be ready till November 9th. This relies on MIT KRB5 features which are not in a released version.
Comment 18 Andrew Bartlett 2021-10-29 06:32:31 UTC
Thanks.  That means one less branch to integrate, sorry you won't make it, the deadline is essentially now, I'm starting backports and already triaging patches out that can't easily be backported. 

Sorry your changes didn't make it.
Comment 19 Andrew Bartlett 2021-10-29 17:54:21 UTC
TODO on this patch-set:
 - Joseph has kindly written some more tests for the 'triple lock' of $ / uac / objectclass for modify
 - Metze has pointed out a knownfail to be fixed
 - I need to fix access checks and extend the testsuite on servicePrincipalName (the falls for the multiple modify elements attack).
 - fix selftest failures in 4.14 / 4.13 backports
Comment 20 Andrew Bartlett 2021-10-29 17:55:18 UTC
Also TODO:
 - patch squashing
 - mark all patches with correct BUG and CVE
Comment 21 Andrew Bartlett 2021-11-03 01:41:54 UTC
Opening this bug up to vendors now.  This is the top level bug for "Andrew's Kerberos Concerns".
Comment 23 Stefan Metzmacher 2021-11-03 15:58:33 UTC
Created attachment 16930 [details]
Combined patchset for master v12 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722)
Comment 24 Stefan Metzmacher 2021-11-03 16:00:27 UTC
Created attachment 16931 [details]
Combined patchset for 4.15 v12 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722)
Comment 25 Stefan Metzmacher 2021-11-03 16:01:38 UTC
Created attachment 16932 [details]
Combined patchset for 4.14 v12 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722)
Comment 26 Stefan Metzmacher 2021-11-03 16:02:30 UTC
Created attachment 16933 [details]
Combined patchset for 4.13 v12 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722)
Comment 27 Joseph Sutton 2021-11-03 21:39:36 UTC
Comment on attachment 16930 [details]
Combined patchset for master v12 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722)

Confirmed patch contents match contents of working branch.
Comment 28 Joseph Sutton 2021-11-04 04:12:57 UTC
Created attachment 16943 [details]
Patch to fix use-after-free regression on top of the combined v12 patchsets (all versions)

Adding a patch to fix a use-after-free regression introduced in the combined patchset.
Comment 35 Andrew Bartlett 2021-11-04 09:46:23 UTC
Created attachment 16950 [details]
CVE-2020-25721 advisory (mirror of v2)

CVE-2020-25721 is a hardening in Samba to allow other software to potentially avoid the impacts of the same kind of issues seen here.  

I've not opened up that bug for access yet so I've put a copy up here, as I've been (rightly) asked about it.
Comment 36 Stefan Metzmacher 2021-11-04 16:00:15 UTC
Comment on attachment 16943 [details]
Patch to fix use-after-free regression on top of the combined v12 patchsets (all versions)

If you already build patches with the v12 patchsets, just have this fix on top.

Otherwise use the v13 patchsets directly, they have it included.
Comment 37 Denis Cardon 2021-11-05 08:08:46 UTC
CVE-2020-25721 advisory mention the following line : "Additionally, Samba 4.15.2, 4.14.10 and 4.13.14 have been issued as security releases to correct the defect"

It does not mention 4.12, while the patchset for that version has been attached to this bugzilla entry. Is it planned to have also a 4.12.16?
Comment 38 Joseph Sutton 2021-11-05 10:11:16 UTC
(In reply to Denis Cardon from comment #37)
As I understand it, the v4.12 patches have not been fully tested (specifically, the fileserver tests were disabled in order to pass CI). So a 4.12 release is not planned, but the patches are being provided 'as is' in case they are useful.
Comment 39 Andrew Bartlett 2021-11-05 18:36:46 UTC
(In reply to Joseph Sutton from comment #38)
In the spirit of open source the Samba Team strongly encourages vendors to upload backports made to earlier releases, on an as-is basis, knowing that others may be assisted by that collaboration.  That is the case with the 4.12 work, here done by Catalyst.

The Samba Team is only releasing patches for the supported versions per:
https://wiki.samba.org/index.php/Samba_Release_Planning

(It was of course a very reasonable question, as we on the Samba Team have bent those rules a little with extra releases, even just recently).

As Joseph notes, as Catalyst's interest in Samba 4.12 is only in the AD DC, the fileserver tests have been disabled.  (The selftest tooling needs further work in the perl code to make that pass).
Comment 40 Stefan Metzmacher 2021-11-08 21:59:45 UTC
The release will happen around 18:00 UTC November 9th.
Comment 41 Joseph Sutton 2021-11-09 04:04:33 UTC
Created attachment 16973 [details]
Combined patchset for 4.10 v13 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722)
Comment 42 Andrew Bartlett 2021-11-09 10:33:05 UTC
Comment on attachment 16949 [details]
Combined patchset for 4.12 v13 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722) (fileserver tests DISABLED)

I'm giving this a positive review based on:
* A match of the vast bulk of the 'actual' patch with the reviewed 4.13 patch
* the inter-diff shows the disabling of the fileserver tests and some ldb backports that were required
* running https://bugzilla.samba.org/attachment.cgi?id=16881 on bug 14881 to produce the patch base and then applying only the 'rest'
* comparing the diff of that new tree with a tree reconstructed from this patch

It would still be good to have that script audited, but auditing the result (multi-MB of backported patches) is impractical.  This is why those patches were publicly merged into 4.13 over a number of releases recently.
Comment 43 Andrew Bartlett 2021-11-09 10:57:08 UTC
Comment on attachment 16973 [details]
Combined patchset for 4.10 v13 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722)

This backport is an epic work.  My review is based on the diff of the patch compared with the 4.12 patch (so under the same challenge that the backport script of cherry-picks on bug 14881 should be checked).

I've also checked each patch Jospeh marked as having modified, plus the into and trailing patches that fix issues found.

My only comment is that in hindsight I think the fixups to use ad_dc_ntvfs in the knownfails could have been avoided now that the ad_dc_ntvfs alias is added

Well done!  Bringing a patch set back all the way to 4.10 with full tests is incredibly awesome.  We are just fortunate that Samba 4.10 allows Python3.

NOTE WELL: This set changes the testing platform to Ubuntu 18.04 compared with the 16.04 reference Samba 4.10 shipped with, as a more modern Python is required for f-strings in tests.  Effort has been taken to allow a build only with Python 2.7.
Comment 44 Andrew Walker 2021-11-09 19:01:22 UTC
Created attachment 16982 [details]
Fix regression in "allow trusted domains = false" v4.13

We discovered issue preventing winbindd from starting when "allow trusted domains = false". Attached patch is for v4.13.
Comment 45 Andrew Walker 2021-11-09 19:01:30 UTC
Created attachment 16983 [details]
Fix regression in "allow trusted domains = false" v4.13

We discovered issue preventing winbindd from starting when "allow trusted domains = false". Attached patch is for v4.13.
Comment 46 Andrew Bartlett 2021-11-09 19:03:20 UTC
The releases are made, removing [EMBARGOED] tag.  The vendor-only restriction will be removed soon once the dust settles.
Comment 47 Stefan Metzmacher 2021-11-09 20:43:25 UTC
(In reply to Andrew Walker from comment #45)

I have a more generic fix for the "allow trusted domains = no" regression,
the pipeline is running here:
https://gitlab.com/samba-team/samba/-/merge_requests/2246

Andrew, can you please test this?
Comment 48 Andrew Bartlett 2021-11-09 20:54:44 UTC
The patches addressing this issue have been pushed to master and security releases made.
Comment 49 Andrew Walker 2021-11-09 20:55:55 UTC
(In reply to Stefan Metzmacher from comment #47)

Yes, that resolves the issue with "allow trusted domains = false".
Comment 50 Andrew Bartlett 2021-11-11 01:24:52 UTC
The regression for "allowed trusted domains = false" case is in the 4.12 patches (by backport) but not the 4.10 patches.
Comment 51 Andrew Bartlett 2021-11-11 01:30:26 UTC
Sorry, wrong see-also link
Comment 52 Andrew Bartlett 2021-11-11 21:02:50 UTC
Removing vendor restriction and all-vendor CC (to avoid spamming all vendors now this is public.  If you are a vendor and wish to be notified about further updates please CC individually.
Comment 53 Andrew Bartlett 2021-11-22 03:41:39 UTC
A note on backporting this issue to earlier versions.

I warned that this would be difficult, and I think the size of attachment 16973 [details] proves my point well.  However it has been possible as far back as 4.10 while fully tested, thanks to that being the first release we made with python3 fully supported.  

Even so, the tests required changing the minimum python3 version up to Python 3.6 as the backported tests start to require the f-string feature.

Backporting earlier may be possible for the careful, but would mean doing the tests manually (in the same way we can run those against a remote windows server). 

See my comments on bug 14561 and 15464 about the highest priority patches if upgrading to a supported or supportable release is impossible.
Comment 54 Andrew Bartlett 2021-11-22 03:42:11 UTC
(In reply to Andrew Bartlett from comment #53)
that is (to make it link) bug 14564