Douglas Bagnall [Mon, 13 Sep 2021 02:15:09 +0000 (14:15 +1200)]
CVE-2020-25722 pytest: test sAMAccountName/userPrincipalName over ldap
Because the sam account name + the dns host name is used as the
default user principal name, we need to check for collisions between
these. Fixes are coming in upcoming patches.
Douglas Bagnall [Thu, 28 Oct 2021 00:07:01 +0000 (13:07 +1300)]
CVE-2020-25722 blackbox/upgrades tests: ignore SPN for ldapcmp
We need to have the SPNs there before someone else nabs them, which
makes the re-provisioned old releases different from the reference
versions that we keep for this comparison.
Douglas Bagnall [Wed, 27 Oct 2021 20:45:36 +0000 (09:45 +1300)]
CVE-2020-25722 s4/provision: add host/ SPNs at the start
There are two reasons for this. Firstly, leaving SPNs unclaimed is
dangerous, as someone else could grab them first. Secondly, in some
circumstances (self join) we try to add a DNS/ SPN a little bit later
in provision. Under the rules we are introducing for CVE-2020-25722,
this will make our later attempts to add HOST/ fail.
This causes a few errors in samba4.blackbox.dbcheck.* tests, which
assert that revivified old domains match stored reference versions.
Now they don't, because they have servicePrincipalNames.
Douglas Bagnall [Wed, 1 Sep 2021 06:35:02 +0000 (18:35 +1200)]
CVE-2020-25722 tests: blackbox samba-tool spn non-admin test
It is soon going to be impossible to add duplicate SPNs (short of
going behind DSDB's back on the local filesystem). Our test of adding
SPNs on non-admin users doubled as the test for adding a duplicate (using
--force). As --force is gone, we add these tests on Guest after the SPN
on Administrator is gone.
This did not actually *force* the creation of a duplicate SPN, it just
ignored the client-side check for the existing copy. Soon we are going
to enforce SPN uniqueness on the server side, and this --force will not
work. This will make the --force test fail, and if that tests fail, so
will others that depend the duplicate values. So we remove those tests.
It is wrong-headed to try to make duplicate SPNs in any case, which is
probably why there is no sign of anyone ever having used this option.
Andrew Bartlett [Mon, 1 Nov 2021 04:21:16 +0000 (17:21 +1300)]
CVE-2020-25722 Check for all errors from acl_check_extended_right() in acl_check_spn()
We should not fail open on error.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876 Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Andrew Bartlett [Mon, 1 Nov 2021 04:19:29 +0000 (17:19 +1300)]
CVE-2020-25722 Check all elements in acl_check_spn() not just the first one
Thankfully we are aleady in a loop over all the message elements in
acl_modify() so this is an easy and safe change to make.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876 Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Nadezhda Ivanova [Mon, 18 Oct 2021 11:27:59 +0000 (14:27 +0300)]
CVE-2020-25722: s4-acl: Make sure Control Access Rights honor the Applies-to attribute
Validate Writes and Control Access Rights only grant access if the
object is of the type listed in the Right's appliesTo attribute. For
example, even though a Validated-SPN access may be granted to a user
object in the SD, it should only pass if the object is of class
computer This patch enforces the appliesTo attribute classes for
access checks from within the ldb stack.
Nadezhda Ivanova [Mon, 25 Oct 2021 11:54:56 +0000 (14:54 +0300)]
CVE-2020-25722: s4-acl: test Control Access Rights honor the Applies-to attribute
Validate Writes and Control Access Rights should only grant access if the
object is of the type listed in the Right's appliesTo attribute.
Tests to verify this behavior
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
[abartlet@samba.org Included in backport as changing ACLs while
ACL tests are not checking for unexpected success would be bad]
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
[abartlet@samba.org Removed transaction hooks, these do nothing over
remote LDAP]
CVE-2020-25717: selftest: configure 'ktest' env with winbindd and idmap_autorid
The 'ktest' environment was/is designed to test kerberos in an active
directory member setup. It was created at a time we wanted to test
smbd/winbindd with kerberos without having the source4 ad dc available.
This still applies to testing the build with system krb5 libraries
but without relying on a running ad dc.
As a domain member setup requires a running winbindd, we should test it
that way, in order to reflect a valid setup.
As a side effect it provides a way to demonstrate that we can accept
smb connections authenticated via kerberos, but no connection to
a domain controller! In order get this working offline, we need an
idmap backend with ID_TYPE_BOTH support, so we use 'autorid', which
should be the default choice.
CVE-2020-25717: s3:auth: let auth3_generate_session_info_pac() delegate everything to make_server_info_wbcAuthUserInfo()
This consolidates the code paths used for NTLMSSP and Kerberos!
I checked what we were already doing for NTLMSSP, which is this:
a) source3/auth/auth_winbind.c calls wbcAuthenticateUserEx()
b) as a domain member we require a valid response from winbindd,
otherwise we'll return NT_STATUS_NO_LOGON_SERVERS
c) we call make_server_info_wbcAuthUserInfo(), which internally
calls make_server_info_info3()
d) auth_check_ntlm_password() calls
smb_pam_accountcheck(unix_username, rhost), where rhost
is only an ipv4 or ipv6 address (without reverse dns lookup)
e) from auth3_check_password_send/auth3_check_password_recv()
server_returned_info will be passed to auth3_generate_session_info(),
triggered by gensec_session_info(), which means we'll call into
create_local_token() in order to transform auth_serversupplied_info
into auth_session_info.
For Kerberos gensec_session_info() will call
auth3_generate_session_info_pac() via the gensec_generate_session_info_pac()
helper function. The current logic is this:
a) gensec_generate_session_info_pac() is the function that
evaluates the 'gensec:require_pac', which defaulted to 'no'
before.
b) auth3_generate_session_info_pac() called
wbcAuthenticateUserEx() in order to pass the PAC blob
to winbindd, but only to prime its cache, e.g. netsamlogon cache
and others. Most failures were just ignored.
c) If the PAC blob is available, it extracted the PAC_LOGON_INFO
from it.
d) Then we called the horrible get_user_from_kerberos_info() function:
- It uses a first part of the tickets principal name (before the @)
as username and combines that with the 'logon_info->base.logon_domain'
if the logon_info (PAC) is present.
- As a fallback without a PAC it's tries to ask winbindd for a mapping
from realm to netbios domain name.
- Finally is falls back to using the realm as netbios domain name
With this information is builds 'userdomain+winbind_separator+useraccount'
and calls map_username() followed by smb_getpwnam() with create=true,
Note this is similar to the make_server_info_info3() => check_account()
=> smb_getpwnam() logic under 3.
- It also calls smb_pam_accountcheck(), but may pass the reverse DNS lookup name
instead of the ip address as rhost.
- It does some MAP_TO_GUEST_ON_BAD_UID logic and auto creates the
guest account.
e) We called create_info3_from_pac_logon_info()
f) make_session_info_krb5() calls gets called and triggers this:
- If get_user_from_kerberos_info() mapped to guest, it calls
make_server_info_guest()
- If create_info3_from_pac_logon_info() created a info3 from logon_info,
it calls make_server_info_info3()
- Without a PAC it tries pdb_getsampwnam()/make_server_info_sam() with
a fallback to make_server_info_pw()
From there it calls create_local_token()
I tried to change auth3_generate_session_info_pac() to behave similar
to auth_winbind.c together with auth3_generate_session_info() as
a domain member, as we now rely on a PAC:
a) As domain member we require a PAC and always call wbcAuthenticateUserEx()
and require a valid response!
b) we call make_server_info_wbcAuthUserInfo(), which internally
calls make_server_info_info3(). Note make_server_info_info3()
handles MAP_TO_GUEST_ON_BAD_UID and make_server_info_guest()
internally.
c) Similar to auth_check_ntlm_password() we now call
smb_pam_accountcheck(unix_username, rhost), where rhost
is only an ipv4 or ipv6 address (without reverse dns lookup)
d) From there it calls create_local_token()
As standalone server (in an MIT realm) we continue
with the already existing code logic, which works without a PAC:
a) we keep smb_getpwnam() with create=true logic as it
also requires an explicit 'add user script' option.
b) In the following commits we assert that there's
actually no PAC in this mode, which means we can
remove unused and confusing code.
CVE-2020-25719 CVE-2020-25717: auth/gensec: always require a PAC in domain mode (DC or member)
AD domains always provide a PAC unless UF_NO_AUTH_DATA_REQUIRED is set
on the service account, which can only be explicitly configured,
but that's an invalid configuration!
We still try to support standalone servers in an MIT realm,
as legacy setup.
CVE-2020-25717: Add FreeIPA domain controller role
As we want to reduce use of 'classic domain controller' role but FreeIPA
relies on it internally, add a separate role to mark FreeIPA domain
controller role.
It means that role won't result in ROLE_STANDALONE.
CVE-2020-25717: s3:auth: don't let create_local_token depend on !winbind_ping()
We always require a running winbindd on a domain member, so
we should better fail a request instead of silently alter
the behaviour, which results in a different unix token, just
because winbindd might be restarted.
Ralph Boehme [Fri, 8 Oct 2021 10:33:16 +0000 (12:33 +0200)]
CVE-2020-25717: s3:auth: remove fallbacks in smb_getpwnam()
So far we tried getpwnam("DOMAIN\account") first and
always did a fallback to getpwnam("account") completely
ignoring the domain part, this just causes problems
as we mix "DOMAIN1\account", "DOMAIN2\account",
and "account"!
As we require a running winbindd for domain member setups
we should no longer do a fallback to just "account" for
users served by winbindd!
For users of the local SAM don't use this code path,
as check_sam_security() doesn't call check_account().
The only case where smb_getpwnam("account") happens is
when map_username() via ("username map [script]") mapped
"DOMAIN\account" to something without '\', but that is
explicitly desired by the admin.
CVE-2020-25717: s3:auth: no longer let check_account() autocreate local users
So far we autocreated local user accounts based on just the
account_name (just ignoring any domain part).
This only happens via a possible 'add user script',
which is not typically defined on domain members
and on NT4 DCs local users already exist in the
local passdb anyway.
Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Samuel Cabrero <scabrero@samba.org> Signed-off-by: Stefan Metzmacher <metze@samba.org> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
[abartlet@samba.org Removed knownfail on advice from metze]
Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Samuel Cabrero <scabrero@samba.org> Signed-off-by: Stefan Metzmacher <metze@samba.org> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
[abartlet@samba.org Fixed knowfail per instruction from metze]
Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Samuel Cabrero <scabrero@samba.org> Signed-off-by: Stefan Metzmacher <metze@samba.org>
[abartlet@samba.org backported to Samba 4.14 without offline
tests in Samba3.pm]
Pair-Programmed-With: Stefan Metzmacher <metze@samba.org>
Signed-off-by: Samuel Cabrero <scabrero@samba.org> Signed-off-by: Stefan Metzmacher <metze@samba.org>
[abartlet@samba.org Backported from master/4.15 due to
conflicts with other new parameters]
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> Reviewed-by: Stefan Metzmacher <metze@samba.org> Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Andrew Bartlett [Fri, 22 Oct 2021 10:41:23 +0000 (23:41 +1300)]
CVE-2020-25722 selftest/user_account_control: more work to cope with UAC/objectclass defaults and lock
This new restriction breaks a large number of assumptions in the tests, like
that you can remove some UF_ flags, because it turns out doing so will
make the 'computer' a 'user' again, and this will fail.
Andrew Bartlett [Fri, 22 Oct 2021 03:07:46 +0000 (16:07 +1300)]
CVE-2020-25722 dsdb: Prohibit mismatch between UF_ account types and objectclass.
There are a lot of knownfail entries added with this commit. These
all need to be addressed and removed in subsequent commits which
will restructure the tests to pass within this new reality.
The restriction is not applied to users with administrator rights,
as this breaks a lot of tests and provides no security benefit.
Andrew Bartlett [Wed, 15 Sep 2021 20:46:42 +0000 (08:46 +1200)]
CVE-2020-25722 dsdb: objectclass computer becomes UF_WORKSTATION_TRUST by default
There are a lot of knownfail entries added with this commit. These
all need to be addressed and removed in subsequent commits which
will restructure the tests to pass within this new reality.
This default applies even to users with administrator rights,
as changing the default based on permissions would break
to many assumptions.
Andrew Bartlett [Mon, 13 Sep 2021 08:34:54 +0000 (20:34 +1200)]
CVE-2020-25722 selftest: Extend priv_attrs test - work around UF_NORMAL_ACCOUNT rules on Windows 2019 (requires |UF_PASSWD_NOTREQD or a password) - extend to also cover the sensitive UF_TRUSTED_FOR_DELEGATION
Andrew Bartlett [Fri, 13 Aug 2021 05:42:23 +0000 (17:42 +1200)]
CVE-2020-25722 dsdb: Restrict the setting of privileged attributes during LDAP add/modify
The remaining failures in the priv_attrs (not the strict one) test are
due to missing objectclass constraints on the administrator which should
be addressed, but are not a security issue.
A better test for confirming constraints between objectclass and
userAccountControl UF_NORMAL_ACCONT/UF_WORKSTATION_TRUST values would
be user_account_control.py.
Andrew Bartlett [Wed, 11 Aug 2021 23:10:09 +0000 (11:10 +1200)]
CVE-2020-25722 dsdb: Move krbtgt password setup after the point of checking if any passwords are changed
This allows the add of an RODC, before setting the password, to avoid
this module, which helps isolate testing of security around the
msDS-SecondaryKrbTgtNumber attribute.
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz> Reviewed-by: Andreas Schneider <asn@cryptomilk.org>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Mon Oct 25 09:23:35 UTC 2021 on sn-devel-184
Andrew Bartlett [Thu, 16 Sep 2021 04:09:24 +0000 (16:09 +1200)]
CVE-2020-25722 selftest: Use self.assertRaisesLdbError() in user_account_control.py test
This changes most of the simple pattern with self.samdb.modify()
to use the wrapper. Some other calls still need to be converted, while
the complex decision tree tests should remain as-is for now.
Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Mon Oct 4 21:55:43 UTC 2021 on sn-devel-184
Andrew Bartlett [Mon, 30 Aug 2021 06:17:47 +0000 (18:17 +1200)]
CVE-2020-25722 selftest: Update user_account_control tests to pass against Windows 2019
This gets us closer to passing against Windows 2019, without
making major changes to what was tested. More tests are needed,
but it is important to get what was being tested tested again.
Account types (eg UF_NORMAL_ACCOUNT, UF_WORKSTATION_TRUST_ACCOUNT)
are now required on all objects, this can't be omitted any more.
Also for UF_NORMAL_ACCOUNT for these accounts without a password
set |UF_PASSWD_NOTREQD must be included.
Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Alexander Bokovoy <ab@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14753
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Wed Sep 15 08:49:11 UTC 2021 on sn-devel-184
Andrew Bartlett [Mon, 30 Aug 2021 02:54:39 +0000 (14:54 +1200)]
CVE-2020-25722 selftest: Replace internal loop in test_uac_bits_set() using @DynamicTestClass
This generates a single test per bit which is easier to
debug. Elsewhere we use this pattern where we want to
be able to put some cases in a knownfail, which is otherwise
not possible.
Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14753
(cherry picked from commit 17ae0319db53a7b88e7fb44a9e2fd4bf1d1daa0e)
Andrew Bartlett [Mon, 30 Aug 2021 02:51:27 +0000 (14:51 +1200)]
CVE-2020-25722 selftest: Replace internal loop in test_uac_bits_add() using @DynamicTestClass
This generates a single test per bit which is easier to
debug. Elsewhere we use this pattern where we want to
be able to put some cases in a knownfail, which is otherwise
not possible.
Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14753
(cherry picked from commit 60f1b6cf0ef0bf6736d8db9c53fa48fe9f3d8e75)
Andrew Bartlett [Mon, 30 Aug 2021 02:37:06 +0000 (14:37 +1200)]
CVE-2020-25722 selftest: Use @DynamicTestCase in user_account_control test_uac_bits_unrelated_modify()
This is a nice easy example of how the test generation
code works, and it combined nicely with the earlier
patch to return string names from the UF_ constants.
Signed-off-by: Andrew Bartlett <abartlet@samba.org> Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14753
(cherry picked from commit 8701ce492fc3a209035b152961d8c17e801b082a)
projects / thirdparty / samba.git / log
