pdb: Use "sid_compose" where appropriate

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

lib: Avoid the use of open_memstream in tevent_req_profile_string

Solaris does not have it.

Bug: https://bugzilla.samba.org/show_bug.cgi?id=13629
Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tdb: Basic test for circular freelist fix

Try to store a record for which the (circular) freelist does not have
any entry.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tdb: Basic test for circular hash chain fix

This just walks tdb_find by searching for a nonexistent record

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tdb: Make the freelist walk circular-safe

We can't really do the full check while the freelist is modified on the
fly. As long as we don't merge any freelist entries, we should be good
to apply this check.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tdb: Align integer types

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tdb: Make get_hash_length circular-safe

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tdb: Make tdb_find_dead circular-safe

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tdb: Make tdb_dump_chain circular-list safe

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tdb: Make tdb_find circular-safe

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tdb: Add tdb_chainwalk_check

This captures the tdb_rescue protection against circular hash chains
with a slow pointer updated only on every other record traverse

If a hash chain has a loop, eventually the next_ptr
will cycle around and be identical to the 'slow' pointer.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

registry: Don't use an uninitialized value

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

registry: Print failure of regdb_unpack_values

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

registry: Add error checks to regdb_unpack_values

This makes "regdb_unpack_values" take a size_t as buflen. The only
caller calls it with TDB_DATA.dsize, which *is* size_t. Convert the
internal "len" variable to the unsigned size_t as well and add overflow
checks. This depends on tdb_unpack to either return -1 or a positive
value less than or equal to the passed-in "size_t" buflen;

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

registry: Add error checks to regdb_fetch_keys_internal

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

registry: Fix a typo

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tdb_unpack: Protect against overflow

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tdb_unpack: Correct "len" arg for "B" format

All but one of the users of the "B" format specifier passed in a pointer
to uint32_t instead of what tdb_unpack expected, an "int". Because this
is a purely internal API, change the tdb_unpack function and adjust that
one caller.

To reviewers: Please check carefully, thanks :-)

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

tdb_unpack: Convert to size_t for internal calculations

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

gpo: Test process_group_policy in gp_sec_ext

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>
Autobuild-User(master): Aurélien Aptel <aaptel@samba.org>
Autobuild-Date(master): Mon Oct  8 21:25:59 CEST 2018 on sn-devel-144

gpo: test the get_deleted_gpos_list() function

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: Test the new get_applied functions

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo PEP8: balance whitespace around equals

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@suse.com>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpupdate: test the new --force option

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpupdate: Add the --force option

This option forces the reapplication of policy,
and works the same as MS 'gpupdate /force'

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: Remove unused apply_log_pop() and list() funcs

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: add unapply to the gp_sec_ext

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: Calculate deleted gpos and unapply them

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: Use the new process_group_policy() for unapply

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: avoid quadratic behaviour in guid retrieval

Signed-off-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: David Mulder <dmulder@suse.com>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: Create a function for returning applied settings

This returns a list of guids for gpos applied
plus settings applied and their previous values.

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: apply_map should not be required for gp_ext

The apply_map function should not be a requirement
to implement the gp_ext class, since only the
gp_sec_ext uses it now.

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: remove unreached non-DC branch in gp_sec_ext.apply_map()

We don't get this far if we are not a DC, and if somehow we do the
errors will be no more informative due to this special case.

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: Move policy application to the gp_ext

Policy specific setting application should be
handled by the group policy extension, not the
read/parse handler.

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: Remove unused gp_ext.list() function

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: Implement process_group_policy() gp_ext func

MS spec describes the policy callback as a
function called ProcessGroupPolicy which accepts
a pDeletedGPOList and a pChangedGPOList param.
The Group Policy Client Side Extension then
iterates over the deleted, then the changed gpo
lists and applies/unapplies policy. We should do
this also.

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: Remove unused methods from gp_sec_ext

These functions were added by Luke, but have
never actually done anything. If/when we
read from these *.pol files, we won't need these
separate functions to do it.

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: Initialize gp_ext variables in constructor

Initialize variables for the gp_ext in the
constructor instead of passing them via the parse
function.
This is a dependency of the "gpo: Implement
process_group_policy() gp_ext func" patch, since
the parse() function is now called by the ext,
instead of by gpupdate within apply_gp(). The
parse() function should only take the path
variable, to simplify writing Client Side
Extensions.

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpupdate: Remove the unnecessary url parameter

The samdb object isn't initialized here anymore,
but in the gp_sec_ext, so this parameter to
gpupdate does nothing.

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: gp_sec_ext should check whether to apply

Whether an extension should apply should be
determined by the extension, not by the
calling script.

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: Initialize SamDB in the gp_sec_ext

The SamDB is only used by the gp_sec_ext, and
isn't needed elsewhere, so initialize it where
we need it and avoid passing it around
everywhere.
It makes the most sense to put this in the setter
class that uses it, so pass our creds down so we
have access to it, then initialize it there.

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

gpo: abstract methods are defined in the parent class

These methods don't need redefined in the child
class.

Signed-off-by: David Mulder <dmulder@suse.com>
Reviewed-by: Douglas Bagnall <douglas.bagnall@catalyst.net.nz>
Reviewed-by: Aurelien Aptel <aaptel@suse.com>

ctdb-tests: Improve counting of database records

Record counts are sometimes incomplete for large databases when
relevant tests are run on a real cluster.

This probably has something to do with ssh, pipes and buffering, so
move the filtering and counting to the remote end.  This means that
only the count comes across the pipe, instead of all the record data.

Instead of explicitly excluding the key for persistent database
sequence numbers, just exclude any key starting with '_'.  Such keys
are not used in tests.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>
Autobuild-User(master): Amitay Isaacs <amitay@samba.org>
Autobuild-Date(master): Mon Oct  8 05:36:11 CEST 2018 on sn-devel-144

ctdb-tests: Add extra debug to large database recovery test

This test sometimes fails, probably because the test is flakey.
Either the records aren't being added correctly or the counting of
records loses records.  Try to debug both possibilities.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Shut down transaction_loop clients more cleanly

A transaction_loop client can exit with a transaction active when its
time limit expires.  This causes a recovery and causes problems with
the test cleanup, which detects unwanted recoveries and fails.

Set a flag when the time limit expires and exit cleanly before the
next transaction is started.

Pair-programmed-with: Amitay Isaacs <amitay@gmail.com>
Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tools: Have onnode pass -n option even when regular ssh not in use

ONNODE_SSH is really a test hook, so it doesn't need to support
completely random values.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Support closing of stdin in local daemons ssh stub

Not sure this is needed but this makes it behave the same as ssh.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Be more careful when building public IP addresses

The goal is to allow more local daemons by expanding the address range
rather than generating invalid addresses.

For IPv6, use a separate address space instead of an offset for the
2nd address.

For IPv4, use the last 2 octets with addresses starting at
192.168.100.1 and 192.168.200.1.  Avoid addresses with 0 and 255 in
the last octet by using a maximum of 100 addresses per "subnet"
starting at .1.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Be more careful when building node addresses

The goal is to allow more local daemons by expanding the address range
rather than generating invalid addresses.

For IPv6, use all 4 trailing hex digits.

For IPv4, use the last 2 octets.  Although 127.0.0.0 is a /8 network,
avoid unexpected issues due to 0 and 255 in the last octet.  Use a
maximum of 100 addresses per "subnet" starting at .1.  Keep the first
group of addresses in 127.0.0.0/24 to continue to allow a reasonable
number of nodes to be tested with socket-wrapper.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Don't format IPv4 octets as hex digits

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Be more efficient about starting/stopping local daemons

Don't loop, just use onnode all.

For shutting down, use onnode -p all.  This results in a significant
time saving for stopping many deamons because "ctdb shutdown" is now
synchronous.

onnode -p all can be used to start daemons directly because they
daemonize.  However, this does not work under valgrind because the
valgrind process does not exit, so onnode will wait forever for it.
In this case, use onnode without the -p option.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Do not use ctdbd_wrapper in local daemon tests

Run the daemon directly and shut it down using ctdb shutdown.

The wrapper waits for ctdbd to reach >=FIRST_RECOVERY runstate within
a timeout period and shuts ctdbd down if that doesn't happen.  This is
only really used to ensure that ctdbd doesn't exit early after an
apparently successful start.  There are no known cases where ctdbd
will continue running but fail to reach >=FIRST_RECOVERY runstate.

When ctdbd is started in tests, the test code will wait until ctdbd is
in a healthy state on all nodes before proceeding, so there is
effectively no change in behaviour.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Don't remove non-existent test database directory

This directory is no longer used.  Lack of removal doesn't seem to
cause a problem.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Drop unused function maybe_stop_ctdb()

There are too many functions to start/stop daemons.  Simplify this.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Explicitly check for local daemons when shutting down

This is clearer if the logic is explicit...  and...

There are too many functions to start/stop daemons.  Simplify this.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Drop functions daemons_start(), daemons_stop()

There are too many functions to start/stop daemons.  Simplify this.

Inline the functionality into ctdb_start_all() and ctdb_stop_all().

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Don't used daemons_start()/daemons_stop() directly in tests

There are too many functions to start/stop daemons.  Simplify this.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Rename _ctdb_start_all() -> ctdb_start_all()

There are too many functions to start/stop daemons.  Simplify this.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Rename ctdb_start_all() -> ctdb_init()

There are too many functions to start/stop daemons.  Simplify this.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Drop ps_ctdbd()

This was used for debugging tests by ensuring that the arguments to
ctdbd were as expected.  It no longer outputs anything useful because
ctdbd is now started without arguments.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-tests: Drop code for RECEIVE_RECORDS control

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

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-protocol: Drop marshalling code for RECEIVE_RECORDS control

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

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-protocol: Mark RECEIVE_RECORDS control obsolete

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

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-daemon: Drop implementation of RECEIVE_RECORDS control

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

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-vacuum: Remove unnecessary check for zero records in delete list

Since no records are deleted from RB tree during step 1, there is no
need for the check.  Run step 2 unconditionally.

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

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-vacuum: Fix the incorrect counting of remote errors

If a node fails to delete a record in TRY_DELETE_RECORDS control during
vacuuming, then it's possible that other nodes also may fail to delete a
record.  So instead of deleting the record from RB tree on first failure,
keep track of the remote failures.

Update delete_list.remote_error and delete_list.left statistics only
once per record during the delete_record_traverse.

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

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-vacuum: Simplify the deletion of vacuumed records

The 3-phase deletion of vacuumed records was introduced to overcome
the problem of record(s) resurrection during recovery.  This problem
is now handled by avoiding the records from recently INACTIVE nodes in
the recovery process.

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

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-tests: Add recovery record resurrection test for volatile databases

Ensure that deleted records and vacuumed records are not resurrected
from recently inactive nodes.

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

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Amitay Isaacs <amitay@gmail.com>

ctdb-daemon: Invalidate records if a node becomes INACTIVE

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

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-daemon: Don't pull any records if records are invalidated

This avoids unnecessary work during recovery to pull records from nodes
that were INACTIVE just before the recovery.

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

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdb-daemon: Add invalid_records flag to ctdb_db_context

If a node becomes INACTIVE, then all the records in volatile databases
are invalidated.  This avoids the need to include records from such
nodes during subsequent recovery after the node comes out INACTIVE state.

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

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Martin Schwenke <martin@meltin.net>

ctdbd_conn: Generalise inaccurate error message

Seeing:

  ctdb_read_packet failed: Cannot allocate memory
  [...,  0] ../source3/lib/ctdbd_conn.c:121(cluster_fatal)
  cluster fatal event: ctdbd died

The error is due to a memory allocation failure rather than ctdbd
dying.  However, the error message makes people wonder why ctdbd died.

Another alternative would be to wrap cluster_fatal() and have the
wrapper interpret the return value from ctdb_read_packet() to choose
from a set of more precise messages to pass to cluster_fatal().  For a
memory allocation it isn't strictly necessary to call cluster_fatal(),
but all is probably lost and it is still probably better to try to
exit cleanly as soon as possible instead of crashing somewhere.

Signed-off-by: Martin Schwenke <martin@meltin.net>
Reviewed-by: Volker Lendecke <vl@samba.org>
Autobuild-User(master): Volker Lendecke <vl@samba.org>
Autobuild-Date(master): Fri Oct  5 16:16:29 CEST 2018 on sn-devel-144

s4/auth/tests: Fix kerberos test string size

>>> len("user0@samba.example.com")
23

But the string definition does not take a final '\0' into account.
As per Volker's suggestion, use compiler's support to allocate
the string properly.

Signed-off-by: Alexander Bokovoy <ab@samba.org>
Reviewed-by: Volker Lendecke <vl@samba.org>

leases: Streamline leases_db_key a bit

We don't need to talloc the blob, it's always the same size

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Wed Oct  3 04:11:59 CEST 2018 on sn-devel-144

smbd: Fix a warning

gcc complains that the "const" is ignored on function return
types. Right now I'm compiling this file a lot, so silence this
warning :-)

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

lib: Remove unused tdb_pack_append()

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

includes: Remove an unused #define

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

includes: Fix a typo

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

includes: Remove an unused #define

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

third_party: Update cmocka to version 1.1.3

* Added function to filter tests (cmocka_set_test_filter)
* Fixed fixture error reporting
* Some improvement for API documentation -> https://api.cmocka.org/
* Fixed subunit output on failures
* Do not abort if a test is skipped

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

smbd: Simplify downgrade_share_lease

Coalesce the NT_STATUS_OPLOCK_BREAK_IN_PROGRESS case into just one
if-condition

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>
Autobuild-User(master): Stefan Metzmacher <metze@samba.org>
Autobuild-Date(master): Tue Oct  2 22:22:37 CEST 2018 on sn-devel-144

smbd: Move downgrade_share_lease into downgrade_lease

The next step will simplify the logic of the code.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

smbd: Simplify downgrade_lease

To me, the "additive" SMB2_LEASE_WRITE|SMB2_LEASE_HANDLE is easier to
read than the negated ~SMB2_LEASE_READ.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

smbd: Slightly simplify downgrade_lease()

As much as I dislike }else{ and prefer early returns, I even more
dislike asking for the same condition in two different ways.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

smbd: Use find_share_mode_lease() in downgrade_share_lease

Simple simplification: In locking/ we did not have the direct
reference to find_share_mode_lock.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

smbd: Move downgrade_share_lease() to smbd/oplock.c

This function is pretty closely entangled with its only caller. In
particular the NT_STATUS_OPLOCK_BREAK_IN_PROGRESS triggers acitivity
in the caller, and that's the only case where "*_l" is being set to
non-NULL. Prepare for cleanup

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Stefan Metzmacher <metze@samba.org>

tests: Check pam_winbind pw change with different options

Pair-Programmed-With: Andreas Schneider <asn@samba.org>

Signed-off-by: Mathieu Parent <math.parent@gmail.com>
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>
Autobuild-User(master): Andreas Schneider <asn@cryptomilk.org>
Autobuild-Date(master): Tue Oct  2 17:30:29 CEST 2018 on sn-devel-144

nsswitch: Add try_authtok option to pam_winbind

Same as the use_authtok option, except that if the new password is not
valid, PAM will prompt for a password.

Bug-Debian: https://bugs.debian.org/858923
Bug-Ubuntu: https://bugs.launchpad.net/ubuntu/+source/samba/+bug/570944

Signed-off-by: Mathieu Parent <math.parent@gmail.com>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>

third_party: Add pam_set_items.so from pam_wrapper

Pair-Programmed-With: Andreas Schneider <asn@samba.org>
Signed-off-by: Mathieu Parent <math.parent@gmail.com>
Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>

third_party: Update pam_wrapper to version 1.0.7

Signed-off-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Guenther Deschner <gd@samba.org>

python/tests: turn GraphError into failure inside of test_verify()

That test was already marked as flapping in commit
5a73f904e192c44e304850287ac439d0b52f7be5.

However the test generates an UNEXPECTED(error)
instead of an UNEXPECTED(failure).

  [67(594)/75 at 36m28s] samba.tests.kcc.python3(vampire_dc)
  repsFrom source DSA guid (62f01940-a784-4e60-947a-a661c787c8cc) not found
  repsFrom source DSA guid (62f01940-a784-4e60-947a-a661c787c8cc) not found
  repsFrom source DSA guid (62f01940-a784-4e60-947a-a661c787c8cc) not found
  repsFrom source DSA guid (62f01940-a784-4e60-947a-a661c787c8cc) not found
  repsFrom source DSA guid (62f01940-a784-4e60-947a-a661c787c8cc) not found
  UNEXPECTED(error): samba.tests.kcc.python3.samba.tests.kcc.KCCTests.test_verify(vampire_dc)
  REASON: Exception: Exception: Traceback (most recent call last):
    File "bin/python/samba/tests/kcc/__init__.py", line 80, in test_verify
      attempt_live_connections=False)
    File "bin/python/samba/kcc/__init__.py", line 2659, in run
      ('connected',))
    File "bin/python/samba/kcc/__init__.py", line 2513, in plot_all_connections
      vertex_colors=vertex_colours)
    File "bin/python/samba/kcc/graph_utils.py", line 334, in verify_and_dot
      for p, e, doc in errors)))
  samba.kcc.graph_utils.GraphError: The 'dsa_final CN=NTDS Settings,CN=LOCALVAMPIREDC,CN=Servers,CN=Default-First-Site-Name,CN=Sites,CN=Configuration,DC=samba,DC=example,DC=com' graph lacks the following properties:
  connected: the graph is not connected, as the following vertices are unreachable:

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Andreas Schneider <asn@samba.org>

smb2_server: set req->do_encryption = true earlier

The STATUS_SESSION_EXPIRED error was returned unencrypted,
if the request was encrypted.

If clients use SMB3 encryption and the kerberos authenticated session
expires, clients disconnect the connection instead of doing a reauthentication.

From https://blogs.msdn.microsoft.com/openspecification/2012/10/05/encryption-in-smb-3-0-a-protocol-perspective/

  The sender encrypts the message if any of the following conditions is
  satisfied:

    - If the sender is sending a response to an encrypted request.
    - If Session.EncryptData is TRUE and the request or response being
      sent is not NEGOTIATE.
    - If Session.EncryptData is FALSE, the request or response being sent
      is not NEGOTIATE or SESSION_SETUP or TREE_CONNECT, and
      <TreeConnect|Share>.EncryptData is TRUE.

[MS-SMB2] 3.3.4.1.4 Encrypting the Message

 If Connection.Dialect belongs to the SMB 3.x dialect family and
 Connection.ClientCapabilities includes the SMB2_GLOBAL_CAP_ENCRYPTION
 bit, the server MUST encrypt the message before sending, if any of the
 following conditions are satisfied:

 - If the message being sent is any response to a client request for which
   Request.IsEncrypted is TRUE.

 - If Session.EncryptData is TRUE and the response being sent is not
   SMB2_NEGOTIATE or SMB2 SESSION_SETUP.

 - If Session.EncryptData is FALSE, the response being sent is not
   SMB2_NEGOTIATE or SMB2 SESSION_SETUP or SMB2 TREE_CONNECT, and
   Share.EncryptData for the share associated with the TreeId in the SMB2
   header of the response is TRUE.

 The server MUST encrypt the message as specified in section 3.1.4.3,
 before sending it to the client.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
Autobuild-User(master): Volker Lendecke <vl@samba.org>
Autobuild-Date(master): Tue Oct  2 14:11:30 CEST 2018 on sn-devel-144

s4:torture: split smb2.session.expire{1,2} to run with signing and encryptpion

This reproduces the problem we have with expired encrypted sessions.

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

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

s3/locking: Fix logging of lock reference count

lock reference count is always increased and reduced by a value of 1.
But lock_ref_count variable holds the old value prior to change and
was being logged wrongly under debug level 10. DEBUG statement must
log lock_ref_count+1 and lock_ref_count-1 respectively when value
gets increased and decreased.

Signed-off-by: Anoop C S <anoopcs@redhat.com>
Reviewed-by: Andreas Schneider <asn@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

libsmb: Remove smb_share_modes.[ch]

This was declared nonfunctional in 2014. Finally remove it.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

s3: smbd: Prevent valgrind errors in smbtorture3 POSIX test.

Missing fsp talloc free and linked list delete in error
paths in close_directory(). Now matches close_normal_file()
and close_fake_file().

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

Signed-off-by: Jeremy Allison <jra@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>
Autobuild-User(master): Jeremy Allison <jra@samba.org>
Autobuild-Date(master): Sat Sep 29 05:32:41 CEST 2018 on sn-devel-144

libcli: Add debug message if fail to negoatiate SMB protocol

Currently if the client and server can't negotiate an SMB protocol, you
just get the followiing error on the client-side, which doesn't tell you
much.
ERROR(runtime): uncaught exception - (3221225667, 'The network responded
incorrectly.')

This patch adds a debug message to help highlight what's actually going
wrong.

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

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Fri Sep 28 11:25:29 CEST 2018 on sn-devel-144

s3/smbd: Server responds incorrectly if no SMB protocol chosen

The SMBnegprot response from the server contains the DialectIndex of the
selected protocol from the client's request message. Currently, if no
protocol is selected, the server is responding with a DialectIndex=zero,
which is a valid index (PROTOCOL_CORE by default). The Windows spec, and
historically the code, should return DialectIndex=0xffff if no protocol
is chosen. The following commit changed it recently (presumably
inadvertently), so that it now returns DialectIndex=zero.

06940155f315529c5b5 s3:smbd: Fix size types in reply_negprot()

This results in somewhat confusing error messages on the client side:
ERROR(runtime): uncaught exception - (3221225997, 'The transport
connection has been reset.')

or, when signing is configured as mandatory:
smbXcli_negprot: SMB signing is mandatory and the selected protocol
level (1) doesn't support it.
ERROR(runtime): uncaught exception - (3221225506, '{Access Denied} A
process has requested access to an object but has not been granted those
access rights.')

This patch restores the old behaviour of returning 0xffff.

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

Pair-Programmed-With: Ralph Boehme <slow@samba.org>
Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

netcmd: Make sure SMB connection is signed when backing up sysvol

i.e. protect the client against man-in-the-middle attacks by default.

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

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Ralph Boehme <slow@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>

s4/web_server: fix panic from wrong type extracted from conn

enabling
  server services = +web

and we get a panic

web_server.c:251: Type mismatch: name[struct web_server_data] expected[struct task_server]
smb_panic_default: PANIC (pid 29137): ../source4/web_server/web_server.c:251: Type mismatch: name[struct web_server_data] expected[struct task_server]

Signed-off-by: Noel Power <noel.power@suse.com>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
Autobuild-User(master): Andrew Bartlett <abartlet@samba.org>
Autobuild-Date(master): Fri Sep 28 03:57:47 CEST 2018 on sn-devel-144