1
0
mirror of https://github.com/samba-team/samba.git synced 2024-12-24 21:34:56 +03:00
Commit Graph

127845 Commits

Author SHA1 Message Date
Douglas Bagnall
5fe2633b2a CVE-2020-25722 s4/dsdb/util: remove unused dsdb_get_single_valued_attr()
Nobody uses it now. It never really did what it said it did. Almost
every use was wrong. It was a trap.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
b9962c1e5e CVE-2020-25722 s4/dsdb/pwd_hash: rework pwdLastSet bypass
This tightens the logic a bit, in that a message with trailing DELETE
elements is no longer accepted when the bypass flag is set. In any case
this is an unlikely scenario as this is an internal flag set by a private
control in pdb_samba_dsdb_replace_by_sam().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
59201d5424 CVE-2020-25722 s4/dsdb/pwd_hash: password_hash_bypass gets all values
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
ed9ec0b081 CVE-2020-25722 s4/dsdb/samldb: samldb_fsmo_role_owner_check() wants one value
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
280c07f58a CVE-2020-25722 s4/dsdb/samldb: samldb_fsmo_role_owner_check checks values
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
ecb2c3a80c CVE-2020-25722 s4/dsdb/samldb: samldb_service_principal_names_change checks values
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
d120204012 CVE-2020-25722 s4/dsdb/samldb: samldb_group_type_change() checks all values
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
4fb4136a84 CVE-2020-25722 s4/dsdb/samldb: samldb_lockout_time() checks all values
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
1e0176cf65 CVE-2020-25722 s4/dsdb/samldb: samldb_pwd_last_set_change() checks all values
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
74623b644d CVE-2020-25722 s4/dsdb/samldb _user_account_control_change() always add final value
dsdb_get_single_valued_attr() was finding the last non-delete element for
userAccountControl and changing its value to the computed value.
Unfortunately, the last non-delete element might not be the last element,
and a subsequent delete might remove it.

Instead we just add a replace on the end.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
bdfcea484e CVE-2020-25722 s4/dsdb/samldb: samldb_user_account_control_change() checks all values
There is another call to dsdb_get_expected_new_values() in this function
that we change in the next commit.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
87382e198f CVE-2020-25722 s4/dsdb/samldb: samldb_prim_group_change() checks all values
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
e4762f4c01 CVE-2020-25722 s4/dsdb/samldb: samldb_schema_add_handle_mapiid() checks all values
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
2a73827583 CVE-2020-25722 s4/dsdb/samldb: samldb_schema_add_handle_linkid() checks all values
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
4d50fe2ff2 CVE-2020-25722 s4/dsdb/samldb: samldb_sam_accountname_valid_check() check all values
Using dsdb_get_expected_new_values().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
c24a41342f CVE-2020-25722 s4/dsdb/samldb: samldb_get_single_valued_attr() check all values
using dsdb_get_expected_new_values().

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
8abf90a3ef CVE-2020-25722 s4/dsdb modules: add dsdb_get_expected_new_values()
This function collects a superset of all the new values for the specified
attribute that could result from an ldb add or modify message.

In most cases -- where there is a single add or modify -- the exact set
of added values is returned, and this is done reasonably efficiently
using the existing element. Where it gets complicated is when there are
multiple elements for the same attribute in a message. Anything added
before a replace or delete will be included in these results but may not
end up in the database if the message runs its course. Examples:

   sequence           result
1. ADD                the element is returned (exact)
2. REPLACE            the element is returned (exact)
3. ADD, ADD           both elements are concatenated together (exact)
4. ADD, REPLACE       both elements are concatenated together (superset)
5. REPLACE, ADD       both elements are concatenated together (exact)
6. ADD, DEL, ADD      adds are concatenated together (superset)
7. REPLACE, REPLACE   both concatenated (superset)
8. DEL, ADD           last element is returned (exact)

Why this? In the past we have treated dsdb_get_single_valued_attr() as if
it returned the complete set of possible database changes, when in fact it
only returned the last non-delete. That is, it could have missed values
in examples 3-7 above.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
13377f0b59 CVE-2020-25722 s4/dsdb/samldb: reject SPN with too few/many components
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
9235617c63 CVE-2020-25722 s4/dsdb/samldb: check for SPN uniqueness, including aliases
Not only should it not be possible to add a servicePrincipalName that
is already present in the domain, it should not be possible to add one
that is implied by an entry in sPNMappings, unless the user is adding
an alias to another SPN and has rights to alter that one.

For example, with the default sPNMappings, cifs/ is an alias pointing to
host/, meaning if there is no cifs/example.com SPN, the host/example.com
one will be used instead. A user can add the cifs/example.com SPN only
if they can also change the host/example.com one (because adding the
cifs/ effectively changes the host/). The reverse is refused in all cases,
unless they happen to be on the same object. That is, if there is a
cifs/example.com SPN, there is no way to add host/example.com elsewhere.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
510378f94a CVE-2020-25722 s4/dsdb/samldb: check sAMAccountName for illegal characters
This only for the real account name, not the account name implicit in
a UPN. It doesn't matter if a UPN implies an illegal sAMAccountName,
since that is not going to conflict with a real one.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
45a4a198b8 CVE-2020-25722 s4/dsdb/samldb: check for clashes in UPNs/samaccountnames
We already know duplicate sAMAccountNames and UserPrincipalNames are bad,
but we also have to check against the values these imply in each other.

For example, imagine users with SAM account names "Alice" and "Bob" in
the realm "example.com". If they do not have explicit UPNs, by the logic
of MS-ADTS 5.1.1.1.1 they use the implict UPNs "alice@example.com" and
"bob@example.com", respectively. If Bob's UPN gets set to
"alice@example.com", it will clash with Alice's implicit one.

Therefore we refuse to allow a UPN that implies an existing SAM account
name and vice versa.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
b6f4d931d0 CVE-2020-25722 s4/dsdb/samldb: unique_attr_check uses samldb_get_single_valued_attr()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
efbf0b77d0 CVE-2020-25722 s4/dsdb/samldb: add samldb_get_single_valued_attr() helper
This takes a string of logic out of samldb_unique_attr_check() that we
are going to need in other places, and that would be very tedious to
repeat.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
ce2930d2d2 CVE-2020-25722 s4/cracknames: add comment pointing to samldb spn handling
These need to stay a little bit in sync. The reverse comment is there.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
11540375af CVE-2020-25722 pytest: test setting servicePrincipalName over ldap
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
df34c11cbc 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.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
55752c12cf 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.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
0a555cf097 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.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
8cde237090 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.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
72a2c21f3f CVE-2020-25722 samba-tool spn add: remove --force option
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.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
7243bd7d38 CVE-2020-25722 samba-tool spn: accept -H for database url
Following the convention and making testing easier

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
5a79fca968 CVE-2020-25722 s4/cracknames: lookup_spn_alias doesn't need krb5 context
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
c7e3617cc3 CVE-2020-25722 s4/dsdb/cracknames: always free tmp_ctx in spn_alias
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
b919246c55 CVE-2020-25722 pytest: assertRaisesLdbError invents a message if you're lazy
This makes it easier to convert tests that don't have good messages.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Douglas Bagnall
de24916a82 CVE-2020-25722 pytests: add reverse lookup dict for LDB error codes
You can give ldb_err() it a number, an LdbError, or a sequence of
numbers, and it will return the corresponding strings. Examples:

ldb_err(68)       # "LDB_ERR_ENTRY_ALREADY_EXISTS"
LDB_ERR_LUT[68]   # "LDB_ERR_ENTRY_ALREADY_EXISTS"

expected = (ldb.ERR_INSUFFICIENT_ACCESS_RIGHTS,
            ldb.ERR_INVALID_CREDENTIALS)
try:
    foo()
except ldb.LdbError as e:
    self.fail(f"got {ldb_err(e)}, expected one of {ldb_err(expected)}")

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14564

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Andrew Bartlett
2c4aee1145 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>
2021-11-09 19:45:33 +00:00
Andrew Bartlett
42eb5fee22 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>
2021-11-09 19:45:33 +00:00
Nadezhda Ivanova
8da6d0bf6f 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.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14832

Signed-off-by: Nadezhda Ivanova <nivanova@symas.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Nadezhda Ivanova
6121f31c0e 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

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14832

Signed-off-by: Nadezhda Ivanova <nivanova@symas.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Joseph Sutton
48e3cf9651 CVE-2020-25722 s4:dsdb:tests: Add missing self.fail() calls
Without these calls the tests could pass if an expected error did not
occur.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14832

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]
2021-11-09 19:45:33 +00:00
Joseph Sutton
62d1cb4c19 CVE-2020-25722 Add test for SPN deletion followed by addition
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14876

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]
2021-11-09 19:45:33 +00:00
Stefan Metzmacher
757f1d20e4 CVE-2020-25717: s3:auth: simplify make_session_info_krb5() by removing unused arguments
This is only ever be called in standalone mode with an MIT realm,
so we don't have a PAC/info3 structure.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Stefan Metzmacher
e8bb009009 CVE-2020-25717: s3:auth: simplify get_user_from_kerberos_info() by removing the unused logon_info argument
This code is only every called in standalone mode on a MIT realm,
it means we never have a PAC and we also don't have winbindd arround.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Stefan Metzmacher
2609e4297e CVE-2020-25717: s3:auth: let auth3_generate_session_info_pac() reject a PAC in standalone mode
We should be strict in standalone mode, that we only support MIT realms
without a PAC in order to keep the code sane.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Stefan Metzmacher
3ed0e5b924 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.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14646
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Stefan Metzmacher
566c2b296d 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.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14646
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Stefan Metzmacher
c4ddf939e0 CVE-2020-25717: s3:ntlm_auth: let ntlm_auth_generate_session_info_pac() base the name on the PAC LOGON_INFO only
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14801
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Stefan Metzmacher
bd8d06ff15 CVE-2020-25717: s3:ntlm_auth: fix memory leaks in ntlm_auth_generate_session_info_pac()
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14801
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Stefan Metzmacher
935feff8e5 CVE-2020-25719 CVE-2020-25717: s4:auth: remove unused auth_generate_session_info_principal()
We'll require a PAC at the main gensec layer already.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14801
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:33 +00:00
Stefan Metzmacher
e2d271cb6b 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.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14801
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14556

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

[jsutton@samba.org Removed knownfail entries]
2021-11-09 19:45:33 +00:00