Created attachment 16724 [details] wip-mit-kdb-fixes.patch Attaching WIP patchset here. It probably covers several issues already.
Current proposed release date is Tuesday October 12, 2021
(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.
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.
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.
(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)
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.
I think asn-bronze-bit-ok is missing
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.
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.
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.
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
Also TODO: - patch squashing - mark all patches with correct BUG and CVE
Opening this bug up to vendors now. This is the top level bug for "Andrew's Kerberos Concerns".
Created attachment 16930 [details] Combined patchset for master v12 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722)
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)
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)
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 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.
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.
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 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.
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?
(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.
(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).
The release will happen around 18:00 UTC November 9th.
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 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 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.
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.
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.
The releases are made, removing [EMBARGOED] tag. The vendor-only restriction will be removed soon once the dust settles.
(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?
The patches addressing this issue have been pushed to master and security releases made.
(In reply to Stefan Metzmacher from comment #47) Yes, that resolves the issue with "allow trusted domains = false".
The regression for "allowed trusted domains = false" case is in the 4.12 patches (by backport) but not the 4.10 patches.
Sorry, wrong see-also link
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.
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.
(In reply to Andrew Bartlett from comment #53) that is (to make it link) bug 14564
Created attachment 17431 [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), NO GIT RENAME This is a re-spin of version 13 of the 4.12 patch for older OS versions that object to git's rename style in patches.
Created attachment 17432 [details] Combined patchset for 4.10 v13 (CVE-2020-25717 CVE-2020-25721 CVE-2020-25718 CVE-2020-25719 CVE-2020-25722), NO GIT RENAME This is the same (standard diff format without renames) for the 4.10 patch.