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

4994 Commits

Author SHA1 Message Date
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
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
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
Andrew Bartlett
ccd94963bd 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.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
b001f91668 CVE-2020-25722 selftest/user_account_control: Allow a broader set of possible errors
This favors a test that confirms we got an error over getting exactly
the right error, at least for now.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
adfae12584 CVE-2020-25722 selftest: Allow self.assertRaisesLdbError() to take a list of errors to match with
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14753

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
56eff305cf CVE-2020-25722 selftest: adapt ldap.py/sam.py test_all tests to new default computer behaviour
Objects of objectclass computer are computers by default now and this changes
the sAMAccountType and primaryGroupID as well as userAccountControl

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
66986eefc6 CVE-2020-25722 selftest: Adapt sam.py test to userAccountControl/objectclass restrictions
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14753

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
6c03fb656d CVE-2020-25722 selftest: New objects of objectclass=computer are workstations by default now
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14753

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
756f116b0e CVE-2020-25722 selftest: Adjust sam.py test_userAccountControl_computer_add_trust to new reality
We now enforce that a trust account must be a user.

These can not be added over LDAP anyway, and our C
code in the RPC server gets this right in any case.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
4150264ce0 CVE-2020-25722 selftest: Split test_userAccountControl into unit tests
The parts that create and delete a single object can be
safely split out into an individual test.

At this point the parts that fail against Windows 2019 are:

error: __main__.SamTests.test_userAccountControl_computer_add_normal [
_ldb.LdbError: (53, 'LDAP error 53 LDAP_UNWILLING_TO_PERFORM -  <0000052D: SvcErr: DSID-031A1236, problem 5003 (WILL_NOT_PERFORM), data 0\n> <>')
error: __main__.SamTests.test_userAccountControl_computer_modify [
_ldb.LdbError: (53, 'LDAP error 53 LDAP_UNWILLING_TO_PERFORM -  <0000052D: SvcErr: DSID-031A1236, problem 5003 (WILL_NOT_PERFORM), data 0\n> <>')
error: __main__.SamTests.test_userAccountControl_user_add_0_uac [
_ldb.LdbError: (53, 'LDAP error 53 LDAP_UNWILLING_TO_PERFORM -  <0000052D: SvcErr: DSID-031A1236, problem 5003 (WILL_NOT_PERFORM), data 0\n> <>')
error: __main__.SamTests.test_userAccountControl_user_add_normal [
_ldb.LdbError: (53, 'LDAP error 53 LDAP_UNWILLING_TO_PERFORM -  <0000052D: SvcErr: DSID-031A1236, problem 5003 (WILL_NOT_PERFORM), data 0\n> <>')
error: __main__.SamTests.test_userAccountControl_user_modify [
_ldb.LdbError: (53, 'LDAP error 53 LDAP_UNWILLING_TO_PERFORM -  <0000052D: SvcErr: DSID-031A1236, problem 5003 (WILL_NOT_PERFORM), data 0\n> <>')

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
0b06e9a5a5 CVE-2020-25722 samdb: Fill in isCriticalSystemObject on any account type change
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14753

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
55cc9324b4 CVE-2020-25722 selftest: Adapt sam.py test_isCriticalSystemObject to new UF_WORKSTATION_TRUST_ACCOUNT default
Objects with objectclass computer now have UF_WORKSTATION_TRUST_ACCOUNT
by default and so this test must adapt.

The changes to this test passes against Windows 2019 except for
the new behaviour around the UF_WORKSTATION_TRUST_ACCOUNT default.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
53d0e5d31e CVE-2020-25722 dsdb: Add restrictions on computer accounts without a trailing $
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14753

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
adf628000f CVE-2020-25722 dsdb: samldb_objectclass_trigger() is only called on ADD, so remove indentation
This makes the code less indented and simpler to understand.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
c77f9cbaee CVE-2020-25722 selftest: Adapt selftest to restriction on swapping account types
This makes many of our tests pass again.  We do not pass against Windows 2019 on all
as this does not have this restriction at this time.

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

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

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Joseph Sutton
6a8f03c527 CVE-2020-25722 dsdb: Add tests for modifying objectClass, userAccountControl and sAMAccountName
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14753
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14889

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
9c3259e503 CVE-2020-25722 dsdb: Improve privileged and unprivileged tests for objectclass/doller/UAC
This helps ensure we cover off all the cases that matter
for objectclass/trailing-doller/userAccountControl

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

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

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
755e8a53ce CVE-2020-25722 selftest: Catch errors from samdb.modify() in user_account_control tests
This will allow these to be listed in a knownfail shortly.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
63eb24f092 CVE-2020-25722 selftest: Catch possible errors in PasswordSettingsTestCase.test_pso_none_applied()
This allows future patches to restrict changing the account type
without triggering an error.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
23983fb50b CVE-2020-25722 selftest: Test combinations of account type and objectclass for creating a user
The idea here is to split out the restrictions seen on Windows 2019
at the schema level, as seen when acting as an administrator.

These pass against Windows 2019 except for the account type swapping
which is not wanted.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
2bdff65b33 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
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14703
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14778
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14775

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

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14703
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14778
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14775

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

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
93e5902369 CVE-2020-25722 dsdb: Tests for our known set of privileged attributes
This, except for where we choose to disagree, does pass
against Windows 2019.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14703
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14778
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14775

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-11-09 19:45:32 +00:00
Andrew Bartlett
5eeb441b77 dsdb: Allow special chars like "@" in samAccountName when generating the salt
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14874

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

Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Wed Oct 20 12:54:54 UTC 2021 on sn-devel-184
2021-10-20 12:54:54 +00:00
Joseph Sutton
6292f0597f selftest: Increase account lockout windows to make test more realiable
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14868

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2021-10-20 12:02:33 +00:00
Douglas Bagnall
a169e013e6 pytest/rodc_rwdc: try to avoid race.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14868

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
2021-10-20 12:02:33 +00:00
Volker Lendecke
3a8374b831 dsdb: Simplify schema_attribute_description() & friends
Use talloc_asprintf_addbuf()

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2021-10-08 19:28:32 +00:00
Andrew Bartlett
b45190bdac 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
2021-10-04 21:55:43 +00:00
Andrew Bartlett
298515cac2 selftest: Move self.assertRaisesLdbError() to samba.tests.TestCase
This is easier to reason with regarding which cases should work
and which cases should fail, avoiding issues where more success
than expected would be OK because a self.fail() was missed in a
try: block.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2021-10-04 21:07:31 +00:00
Andrew Bartlett
b6d60e8f4d samldb: Address birthday paradox adding an RODC
It is possible that the randomly chosen RODC number will be one
that is already in use.  The samldb_krbtgtnumber_available()
function was meant to prevent that, but due to a typo did not.

There is no other race here as the whole thing is inside a transaction,
and we have duplicate protection on samAccountName, so the failure
looked like this:

...
Adding CN=krbtgt_TESTRODCDRS5320202,CN=Users,DC=samba,DC=example,DC=com
UNEXPECTED(error): samba4.drs.repl_rodc.python(ad_dc_ntvfs).repl_rodc.DrsRodcTestCase.test_msDSRevealedUsers_admin(ad_dc_ntvfs)
REASON: Exception: Exception: Traceback (most recent call last):
  File "/m/abartlet/aMASTER/b1635147/samba-def-build/source4/torture/drs/python/repl_rodc.py", line 111, in setUp
    self._create_rodc(self.rodc_ctx)
  File "/m/abartlet/aMASTER/b1635147/samba-def-build/source4/torture/drs/python/repl_rodc.py", line 693, in _create_rodc
    ctx.join_add_objects()
  File "bin/python/samba/join.py", line 641, in join_add_objects
    ctx.add_krbtgt_account()
  File "bin/python/samba/join.py", line 429, in add_krbtgt_account
    ctx.samdb.add(rec, ["rodc_join:1:1"])
_ldb.LdbError: (68, "LDAP error 68 LDAP_ENTRY_ALREADY_EXISTS -  <00002071: samldb: samAccountName krbtgt_4405 already in use!> <>")

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

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): Fri Oct  1 20:50:37 UTC 2021 on sn-devel-184
2021-10-01 20:50:36 +00:00
Andrew Bartlett
d12cb47724 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>

Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Wed Sep 15 08:49:11 UTC 2021 on sn-devel-184
2021-09-15 08:49:11 +00:00
Andrew Bartlett
8affe4a1e6 dsdb: Be careful to avoid use of the expensive talloc_is_parent()
The wrong talloc API was selected while addressing a memory leak.

commit ee2fe56ba0
Author: Aaron Haslett <aaronhaslett@catalyst.net.nz>
Date:   Tue Nov 27 11:07:44 2018 +1300

    drepl: memory leak fix

    Fixes a memory leak where schema reference attached to ldb
    instance is lost before it can be freed.

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

    Signed-off-by: Aaron Haslett <aaronhaslett@catalyst.net.nz>

    Reviewed-by: Andrew Bartlett <abartlet@samba.org>
    Reviewed-by: Garming Sam <garming@catalyst.net.nz>

    Autobuild-User(master): Garming Sam <garming@samba.org>
    Autobuild-Date(master): Wed Jul 17 06:17:10 UTC 2019 on sn-devel-184

By using talloc_get_parent() walking the entire talloc tree is
avoided.

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

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2021-09-05 02:28:29 +00:00
Joseph Sutton
b59fc43523 python: Fix usage strings
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2021-09-04 00:10:37 +00:00
Joseph Sutton
efcd1af17b dsdb/samdb/ldb_modules: Use correct member of union
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2021-09-04 00:10:37 +00:00
Andrew Bartlett
17ae0319db 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>
2021-09-02 05:03:31 +00:00
Andrew Bartlett
60f1b6cf0e 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>
2021-09-02 05:03:31 +00:00
Andrew Bartlett
8701ce492f 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>
2021-09-02 05:03:31 +00:00
Andrew Bartlett
fb6c0b9e2a pydsdb: Add API to return strings of known UF_ flags
Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-09-02 05:03:31 +00:00
Andrew Bartlett
8c45526816 selftest: Use addCleanup rather than tearDown in user_account_control.py
self.addCleanup() is called regardless of the test failure or error status
and so is more reliable, particularly during development.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-09-02 05:03:31 +00:00
Andrew Bartlett
8b078bbf87 selftest: Modernise user_account_control.py tests use a common self.OU
We set and use a single self.OU to ensure consistancy and
reduce string duplication.

Signed-off-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-09-02 05:03:31 +00:00
Volker Lendecke
d2a08f5d67 samdb: Fix an uninitialized variable read
When the "(status == LDB_SUCCESS && msg != NULL)" condition in this
routine is not evaluating to true, "new_rid" is read uninitialized,
comparing it against ~0. Initialize new_rid and compare it against
UINT32_MAX instead of ~0.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2021-08-06 17:22:30 +00:00
Andreas Schneider
feaf0d1ab7 s4:dsdsb: Check return code of cli_credentials_guess()
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-29 02:19:35 +00:00
Douglas Bagnall
2af06390c3 dsdb periodic: DNS: split aging from tombstone deletion
We have been conflating two things (under the term "scavenging"):

1. aging out stale DNS records into tombstones, which only happens if
   this feature is switched on in general and for the zone.

2. removing expired DNS tombstones, which should always happen
   (because we tombstone nodes in *other* ways).

With this patch, we un-conflate.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
020c76a523 dns scavenging: add an explanatory comment
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
3dd5ae46c9 dns scavenging: ensure tombstoned node has one record
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
f52ce9f954 dns scavenging: avoid leak in dns_tombstone_records
As always, we forget to free our zones. Also to check our zones.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
ef7daa51d8 dns scavenging: log tombstone inconsistency
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
9fb69274ca dns scavenging: tighten lifetime of filtered records
We were ending up with everything lasting as long as
kccsrv_periodic_run(), which could add to quite a pile.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
4a2bfd249d dns scavenging: avoid useless copy of msg
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
444b8178b8 dns scavenging: simplify copy_current_records
We don't need to pull and push to make a byte identical copy of an ldb
value, nor do we need a temporary array.

As part of this, we avoid leaving a dangling el->num_values pointing
into space where there were no actually allocated values, which is not
how we do things, even briefly.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
95e9da2fd7 dns scavenging: avoid passing blobs
We can construct these ldb values for each zone with minimal cost and
it is less bamboozling to do so.

While we're fiddling with signatures, we can make this used-once local
function static.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
2d98d733ab dns scavenging: ensure usual ownership of element values
An ldb message, its elements, and their values usually all share a
little talloc sub-tree with each other and nobody else. It is
conceivable that somewhere we rely on that.

In this case we were sharing an out-of-subtree values array across
multiple messages, which seems to be asking for trouble.

Also, add a comment explaining what we want.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
2c6a0265f2 dns scavenging: avoid setting same flags twice
We already did this with db_msg_add_empty(), ~20 lines up.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
25be60a1cc dns scavenging: avoid another small memory leak
We weren't freeing the zones, which is admittedly tricky with the dlink
list.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
8f8eb92903 dns scavenging: avoid a small memory leak
Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
546c64b3fc dns scavenging: correctly set tombstome timestamp
In a DNS tombstone record, the dwTimestamp is, as always, uint32_t hours
since 1601, while the wType-switched .data.EntombedTime timestamp is NTTIME.

We had that wrong, putting the hours in both places.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Douglas Bagnall
dbfbbd4297 dns scavenging: tombstone deletion uses correct time units
Before we were comparing hours to 1e-7 second units.

Now we do it both ways. That's because in dns_tombstone_records (in
this same file) we have been putting hour timestamps in EntombedTime,
but this field is supposed to have NTTIME timestamps, and those
timestamps won't have updated themselves.

This wouldn't matter much in pure Samba networks if we weren't also
using the correct timestamp in dns_common_replace().

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-06-20 23:26:32 +00:00
Joseph Sutton
2a3b82ae23 ridalloc: Don't skip the first RID of a pool
Previously, if either of the rIDPreviousAllocation and rIDNextRID
attributes were not present in a RID Set, the first RID in
rIDAllocationPool was skipped over when determining their values.

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-06-11 07:41:38 +00:00
Joseph Sutton
fec996ff27 samldb: Fix function name typo in error message
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-06-11 07:41:38 +00:00
Günther Deschner
0e1695df7f Fix gcc11 compiler issue "-Werror=maybe-uninitialized"
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14699

../../source4/dsdb/common/util_links.c: In function ‘ndr_guid_compare’:
../../source4/dsdb/common/util_links.c:38:29: error: ‘v1_data’ may be used uninitialized [-Werror=maybe-uninitialized]
   38 |         struct ldb_val v1 = data_blob_const(v1_data, sizeof(v1_data));
      |                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../source4/../lib/util/samba_util.h:48,
                 from ../../source4/include/includes.h:62,
                 from ../../source4/dsdb/common/util_links.c:22:
../../lib/util/data_blob.h:116:20: note: by argument 1 of type ‘const void *’ to ‘data_blob_const’ declared here
  116 | _PUBLIC_ DATA_BLOB data_blob_const(const void *p, size_t length);
      |                    ^~~~~~~~~~~~~~~
../../source4/dsdb/common/util_links.c:37:17: note: ‘v1_data’ declared here
   37 |         uint8_t v1_data[16];
      |                 ^~~~~~~
cc1: all warnings being treated as errors

[1729/3991] Compiling source3/smbd/smbXsrv_open.c
../../libcli/auth/smbencrypt.c: In function ‘decode_wkssvc_join_password_buffer’:
../../libcli/auth/smbencrypt.c:1045:32: error: ‘_confounder’ may be used uninitialized [-Werror=maybe-uninitialized]
 1045 |         DATA_BLOB confounder = data_blob_const(_confounder, 8);
      |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../source4/../lib/util/samba_util.h:48,
                 from ../../source4/include/includes.h:62,
                 from ../../libcli/auth/smbencrypt.c:24:
../../lib/util/data_blob.h:116:20: note: by argument 1 of type ‘const void *’ to ‘data_blob_const’ declared here
  116 | _PUBLIC_ DATA_BLOB data_blob_const(const void *p, size_t length);
      |                    ^~~~~~~~~~~~~~~
../../libcli/auth/smbencrypt.c:1044:17: note: ‘_confounder’ declared here
 1044 |         uint8_t _confounder[8];
      |                 ^~~~~~~~~~~
cc1: all warnings being treated as errors

[2624/3991] Compiling source4/torture/rpc/samr.c
../../source3/rpc_client/cli_samr.c: In function ‘dcerpc_samr_chgpasswd_user2’:
../../source3/rpc_client/cli_samr.c:158:33: error: ‘old_nt_hash’ may be used uninitialized [-Werror=maybe-uninitialized]
  158 |         DATA_BLOB session_key = data_blob_const(old_nt_hash, 16);
      |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../source3/../lib/util/samba_util.h:48,
                 from ../../source3/include/includes.h:256,
                 from ../../source3/rpc_client/cli_samr.c:24:
../../lib/util/data_blob.h:116:20: note: by argument 1 of type ‘const void *’ to ‘data_blob_const’ declared here
  116 | _PUBLIC_ DATA_BLOB data_blob_const(const void *p, size_t length);
      |                    ^~~~~~~~~~~~~~~
../../source3/rpc_client/cli_samr.c:152:17: note: ‘old_nt_hash’ declared here
  152 |         uint8_t old_nt_hash[16];
      |                 ^~~~~~~~~~~
../../source3/rpc_client/cli_samr.c: In function ‘dcerpc_samr_chgpasswd_user3’:
../../source3/rpc_client/cli_samr.c:365:33: error: ‘old_nt_hash’ may be used uninitialized [-Werror=maybe-uninitialized]
  365 |         DATA_BLOB session_key = data_blob_const(old_nt_hash, 16);
      |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../source3/../lib/util/samba_util.h:48,
                 from ../../source3/include/includes.h:256,
                 from ../../source3/rpc_client/cli_samr.c:24:
../../lib/util/data_blob.h:116:20: note: by argument 1 of type ‘const void *’ to ‘data_blob_const’ declared here
  116 | _PUBLIC_ DATA_BLOB data_blob_const(const void *p, size_t length);
      |                    ^~~~~~~~~~~~~~~
../../source3/rpc_client/cli_samr.c:358:17: note: ‘old_nt_hash’ declared here
  358 |         uint8_t old_nt_hash[16];
      |                 ^~~~~~~~~~~
cc1: all warnings being treated as errors

[3399/3991] Compiling source3/rpcclient/cmd_spotlight.c
../../source3/smbd/smbXsrv_open.c: In function ‘smbXsrv_open_set_replay_cache’:
../../source3/smbd/smbXsrv_open.c:936:26: error: ‘data’ may be used uninitialized [-Werror=maybe-uninitialized]
  936 |         DATA_BLOB blob = data_blob_const(data, ARRAY_SIZE(data));
      |                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../source3/../lib/util/samba_util.h:48,
                 from ../../source3/include/includes.h:256,
                 from ../../source3/smbd/smbXsrv_open.c:21:
../../lib/util/data_blob.h:116:20: note: by argument 1 of type ‘const void *’ to ‘data_blob_const’ declared here
  116 | _PUBLIC_ DATA_BLOB data_blob_const(const void *p, size_t length);
      |                    ^~~~~~~~~~~~~~~
../../source3/smbd/smbXsrv_open.c:935:17: note: ‘data’ declared here
  935 |         uint8_t data[SMBXSRV_OPEN_REPLAY_CACHE_FIXED_SIZE];
      |                 ^~~~
cc1: all warnings being treated as errors

../../source3/rpcclient/cmd_spotlight.c: In function ‘cmd_mdssvc_fetch_properties’:
../../source3/rpcclient/cmd_spotlight.c:60:18: error: ‘share_path’ may be used uninitialized [-Werror=maybe-uninitialized]
   60 |         status = dcerpc_mdssvc_open(b, mem_ctx,
      |                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   61 |                                     &device_id,
      |                                     ~~~~~~~~~~~
   62 |                                     &unkn1,
      |                                     ~~~~~~~
   63 |                                     &unkn2,
      |                                     ~~~~~~~
   64 |                                     argv[2],
      |                                     ~~~~~~~~
   65 |                                     argv[1],
      |                                     ~~~~~~~~
   66 |                                     share_path,
      |                                     ~~~~~~~~~~~
   67 |                                     &share_handle);
      |                                     ~~~~~~~~~~~~~~
In file included from ../../source3/rpcclient/cmd_spotlight.c:24:
source3/../librpc/gen_ndr/ndr_mdssvc_c.h:26:10: note: by argument 8 of type ‘const char *’ to ‘dcerpc_mdssvc_open’ declared here
   26 | NTSTATUS dcerpc_mdssvc_open(struct dcerpc_binding_handle *h,
      |          ^~~~~~~~~~~~~~~~~~
../../source3/rpcclient/cmd_spotlight.c:40:14: note: ‘share_path’ declared here
   40 |         char share_path[1025];
      |              ^~~~~~~~~~
cc1: all warnings being treated as errors

../../source4/torture/rpc/samr.c: In function ‘test_ChangePasswordUser2’:
../../source4/torture/rpc/samr.c:2266:19: error: ‘old_nt_hash’ may be used uninitialized [-Werror=maybe-uninitialized]
 2266 |                 = data_blob_const(old_nt_hash, sizeof(old_nt_hash));
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../source4/../lib/util/samba_util.h:48,
                 from ../../source4/include/includes.h:62,
                 from ../../source4/torture/rpc/samr.c:24:
../../lib/util/data_blob.h:116:20: note: by argument 1 of type ‘const void *’ to ‘data_blob_const’ declared here
  116 | _PUBLIC_ DATA_BLOB data_blob_const(const void *p, size_t length);
      |                    ^~~~~~~~~~~~~~~
../../source4/torture/rpc/samr.c:2263:17: note: ‘old_nt_hash’ declared here
 2263 |         uint8_t old_nt_hash[16], new_nt_hash[16];
      |                 ^~~~~~~~~~~
../../source4/torture/rpc/samr.c: In function ‘test_ChangePasswordUser2_ntstatus’:
../../source4/torture/rpc/samr.c:2371:19: error: ‘old_nt_hash’ may be used uninitialized [-Werror=maybe-uninitialized]
 2371 |                 = data_blob_const(old_nt_hash, sizeof(old_nt_hash));
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../source4/../lib/util/samba_util.h:48,
                 from ../../source4/include/includes.h:62,
                 from ../../source4/torture/rpc/samr.c:24:
../../lib/util/data_blob.h:116:20: note: by argument 1 of type ‘const void *’ to ‘data_blob_const’ declared here
  116 | _PUBLIC_ DATA_BLOB data_blob_const(const void *p, size_t length);
      |                    ^~~~~~~~~~~~~~~
../../source4/torture/rpc/samr.c:2368:17: note: ‘old_nt_hash’ declared here
 2368 |         uint8_t old_nt_hash[16], new_nt_hash[16];
      |                 ^~~~~~~~~~~
../../source4/torture/rpc/samr.c: In function ‘test_ChangePasswordUser3’:
../../source4/torture/rpc/samr.c:2478:38: error: ‘old_nt_hash’ may be used uninitialized [-Werror=maybe-uninitialized]
 2478 |         DATA_BLOB old_nt_hash_blob = data_blob_const(old_nt_hash, 16);
      |                                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from ../../source4/../lib/util/samba_util.h:48,
                 from ../../source4/include/includes.h:62,
                 from ../../source4/torture/rpc/samr.c:24:
../../lib/util/data_blob.h:116:20: note: by argument 1 of type ‘const void *’ to ‘data_blob_const’ declared here
  116 | _PUBLIC_ DATA_BLOB data_blob_const(const void *p, size_t length);
      |                    ^~~~~~~~~~~~~~~
../../source4/torture/rpc/samr.c:2473:17: note: ‘old_nt_hash’ declared here
 2473 |         uint8_t old_nt_hash[16], new_nt_hash[16];
      |                 ^~~~~~~~~~~
../../source4/torture/rpc/samr.c: In function ‘test_ChangePasswordRandomBytes’:
../../source4/torture/rpc/samr.c:2794:19: error: ‘old_nt_hash’ may be used uninitialized [-Werror=maybe-uninitialized]
 2794 |                 = data_blob_const(old_nt_hash,
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2795 |                                   sizeof(old_nt_hash));
      |                                   ~~~~~~~~~~~~~~~~~~~~
In file included from ../../source4/../lib/util/samba_util.h:48,
                 from ../../source4/include/includes.h:62,
                 from ../../source4/torture/rpc/samr.c:24:
../../lib/util/data_blob.h:116:20: note: by argument 1 of type ‘const void *’ to ‘data_blob_const’ declared here
  116 | _PUBLIC_ DATA_BLOB data_blob_const(const void *p, size_t length);
      |                    ^~~~~~~~~~~~~~~
../../source4/torture/rpc/samr.c:2792:17: note: ‘old_nt_hash’ declared here
 2792 |         uint8_t old_nt_hash[16], new_nt_hash[16];
      |                 ^~~~~~~~~~~
cc1: all warnings being treated as errors

Guenther

Signed-off-by: Guenther Deschner <gd@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>
2021-05-07 06:23:32 +00:00
Douglas Bagnall
aecb2b779b python: remove all 'from __future__ import print_function'
This made Python 2's print behave like Python 3's print().

In some cases, where we had:

   from __future__ import print_function
   """Intended module documentation..."""

this will have the side effect of making the intended module documentation
work as the actual module documentation (i.e. becoming __doc__), because
it is once again the first statement in the module.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-04-28 03:43:34 +00:00
Volker Lendecke
a20c4b183d dsdb: Slightly tune get_new_descriptor()
DBG_DEBUG only calls its arguments if required according to the debug
level. A simple talloc_new/TALLOC_FREE in the normal case should be
much cheaper than the full sddl_encode().

I just stumbled across this code, this is has not shown up in any
profiles. I just think it's cleaner this way.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
2021-04-19 18:18:32 +00:00
Joseph Sutton
c08f174c35 cracknames: Allow auto-conversion from an extended canonical name
Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-04-07 09:18:30 +00:00
Joseph Sutton
6b57583830 cracknames: Add support for SID string format
BUG: https://bugzilla.samba.org/show_bug.cgi?id=10319

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
2021-04-07 09:18:30 +00:00
Joseph Sutton
de28d915d7 s4:dsdb/password_hash: Add a more useful error message for passwords too long to be hashed
BUG: https://bugzilla.samba.org/show_bug.cgi?id=14621

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-04-07 09:18:30 +00:00
Joseph Sutton
0730b936d7 s4:dsdb/password_hash: Add additional check for crypt() and crypt_r() failure
While crypt_rn() always returns a null pointer in the event of
failure, crypt() and crypt_r() may instead return a string starting
with the character '*'. This commit adds a check to detect failure in
this case.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-04-07 09:18:30 +00:00
Joseph Sutton
88b3d3443b s4:dsdb/password_hash: Don't generate crypt() password for krbtgt account
Since the length of the krbtgt password after conversion to UTF-8 form is
typically greater than the maximum accepted by crypt(), the call usually
fails. This commit disables generation of crypt() passwords for this specific
account, as it's not necessary.

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

Signed-off-by: Joseph Sutton <josephsutton@catalyst.net.nz>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
2021-04-07 09:18:30 +00:00
Douglas Bagnall
162e6fb25a librpc/idl: dnsp tombstone timestamp name matches MS-DNSP
MS-DNSP uses the term "EntombedTime" in e.g. "2.2.2.2.4.23 DNS_RPC_RECORD_TS"
which is more descriptive than the generic "timestamp", and less likely to be
confused with dwTimestamp, which has been our curse. Let's make it grep-able,
google-able, and evocative.

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Jeremy Allison <jra@samba.org>
2021-03-29 23:20:37 +00:00