The branch, master has been updated
       via  df3844f s3/utils: Add warning to testparm for "client ipc signing" 
param values
       via  61f827b unittest: Add testsuite for smb_probe_module()
       via  eaf8e3a lib:util: Make loading of modules more secure
       via  91ef234 lib:util: Make probing of modules more secure
       via  da9de19 lib:util: Rename smb_load_modules()
       via  700914b lib:util: Add new function to load modules from absolute 
path
       via  90b69ba unittest: Add testsuite for is_known_pipename()
       via  0aadb50 wafsamba: Pass down the install argument for samba modules
       via  74b3dd4 lib: Fix illegal use of 0-length arrays
      from  7b50ddd wscript: Fix some typos

https://git.samba.org/?p=samba.git;a=shortlog;h=master


- Log -----------------------------------------------------------------
commit df3844f4df14ea2143ba1856710c00b5ab856c44
Author: Noel Power <[email protected]>
Date:   Fri Jun 2 15:50:48 2017 +0100

    s3/utils: Add warning to testparm for "client ipc signing" param values
    
    We should warn about security sensitive settings where we can,
    client ipc signing has 2 values that can allow connections to proceed
    without SMB signing. This may be unavoidable (e.g. connecting to legacy
    systems) but nevertheless it is worthwhile to warn.
    
    Signed-off-by: Noel Power <[email protected]>
    Reviewed-by: Andreas Schneider <[email protected]>
    Reviewed-by: Jeremy Allison <[email protected]>
    
    Autobuild-User(master): Jeremy Allison <[email protected]>
    Autobuild-Date(master): Tue Jun  6 22:40:12 CEST 2017 on sn-devel-144

commit 61f827bcdde494d3b4a094d6816ff7556f0ff608
Author: Andreas Schneider <[email protected]>
Date:   Fri May 12 14:13:42 2017 +0200

    unittest: Add testsuite for smb_probe_module()
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12780
    
    Signed-off-by: Andreas Schneider <[email protected]>
    Reviewed-by: Jeremy Allison <[email protected]>

commit eaf8e3a88889bed2bcd0932c9642bb7a5a26abe4
Author: Andreas Schneider <[email protected]>
Date:   Mon May 15 11:08:19 2017 +0200

    lib:util: Make loading of modules more secure
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12780
    
    Signed-off-by: Andreas Schneider <[email protected]>
    Reviewed-by: Jeremy Allison <[email protected]>

commit 91ef234a0ad0edfdefeb38cf0ad1de3b3e548f1e
Author: Andreas Schneider <[email protected]>
Date:   Mon May 15 11:05:59 2017 +0200

    lib:util: Make probing of modules more secure
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12780
    
    Signed-off-by: Andreas Schneider <[email protected]>
    Reviewed-by: Jeremy Allison <[email protected]>

commit da9de19cf9a0b543eec9003f10624fa2ba5becd3
Author: Andreas Schneider <[email protected]>
Date:   Mon May 15 10:49:07 2017 +0200

    lib:util: Rename smb_load_modules()
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12780
    
    Signed-off-by: Andreas Schneider <[email protected]>
    Reviewed-by: Jeremy Allison <[email protected]>

commit 700914b45d9cfb8d14cc81fa4fdde0b59bbba798
Author: Andreas Schneider <[email protected]>
Date:   Mon May 15 09:06:51 2017 +0200

    lib:util: Add new function to load modules from absolute path
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12780
    
    Signed-off-by: Andreas Schneider <[email protected]>
    Reviewed-by: Jeremy Allison <[email protected]>

commit 90b69ba95d5ea53f5aadf3e3c271e8c4d50d21b5
Author: Andreas Schneider <[email protected]>
Date:   Thu May 11 11:29:25 2017 +0200

    unittest: Add testsuite for is_known_pipename()
    
    BUG: https://bugzilla.samba.org/show_bug.cgi?id=12780
    
    Signed-off-by: Andreas Schneider <[email protected]>
    Reviewed-by: Jeremy Allison <[email protected]>

commit 0aadb5068f7565c0c05837c577003c5f9d6667a3
Author: Andreas Schneider <[email protected]>
Date:   Thu May 11 11:29:50 2017 +0200

    wafsamba: Pass down the install argument for samba modules
    
    Signed-off-by: Andreas Schneider <[email protected]>
    Reviewed-by: Jeremy Allison <[email protected]>

commit 74b3dd4630ff57fb3b7a6ffa49d1fba678169fbb
Author: Volker Lendecke <[email protected]>
Date:   Mon May 29 21:13:16 2017 +0200

    lib: Fix illegal use of 0-length arrays
    
    Found and confirmed to work by albert chin ([email protected])
    
    Signed-off-by: Volker Lendecke <[email protected]>
    Reviewed-by: Jeremy Allison <[email protected]>

-----------------------------------------------------------------------

Summary of changes:
 buildtools/wafsamba/wafsamba.py             |   6 +-
 lib/util/modules.c                          | 179 ++++++++++++++++++++--------
 lib/util/msghdr.c                           |  10 +-
 lib/util/samba_modules.h                    |   3 +-
 selftest/tests.py                           |   4 +
 source3/smbd/perfcount.c                    |   2 +-
 source3/smbd/process.c                      |   2 +-
 source3/utils/testparm.c                    |  12 ++
 testsuite/unittests/rpc_test_dummy_module.c |  20 ++++
 testsuite/unittests/test_lib_util_modules.c |  76 ++++++++++++
 testsuite/unittests/test_sambafs_srv_pipe.c |  76 ++++++++++++
 testsuite/unittests/wscript                 |  25 ++++
 12 files changed, 361 insertions(+), 54 deletions(-)
 create mode 100644 testsuite/unittests/rpc_test_dummy_module.c
 create mode 100644 testsuite/unittests/test_lib_util_modules.c
 create mode 100644 testsuite/unittests/test_sambafs_srv_pipe.c


Changeset truncated at 500 lines:

diff --git a/buildtools/wafsamba/wafsamba.py b/buildtools/wafsamba/wafsamba.py
index 137cb0e..1bdabf6 100644
--- a/buildtools/wafsamba/wafsamba.py
+++ b/buildtools/wafsamba/wafsamba.py
@@ -465,7 +465,8 @@ def SAMBA_MODULE(bld, modname, source,
                  pyembed=False,
                  manpages=None,
                  allow_undefined_symbols=False,
-                 allow_warnings=False
+                 allow_warnings=False,
+                 install=True
                  ):
     '''define a Samba module.'''
 
@@ -535,7 +536,8 @@ def SAMBA_MODULE(bld, modname, source,
                       pyembed=pyembed,
                       manpages=manpages,
                       allow_undefined_symbols=allow_undefined_symbols,
-                      allow_warnings=allow_warnings
+                      allow_warnings=allow_warnings,
+                      install=install
                       )
 
 
diff --git a/lib/util/modules.c b/lib/util/modules.c
index c3c05f2..cf52594 100644
--- a/lib/util/modules.c
+++ b/lib/util/modules.c
@@ -147,73 +147,53 @@ init_module_fn *load_samba_modules(TALLOC_CTX *mem_ctx, 
const char *subsystem)
        return ret;
 }
 
-
-/* Load a dynamic module.  Only log a level 0 error if we are not checking
-   for the existence of a module (probling). */
-
-static NTSTATUS do_smb_load_module(const char *subsystem,
-                                  const char *module_name, bool is_probe)
+static NTSTATUS load_module_absolute_path(const char *module_path,
+                                         bool is_probe)
 {
        void *handle;
        init_module_fn init;
        NTSTATUS status;
 
-       char *full_path = NULL;
-       TALLOC_CTX *ctx = talloc_stackframe();
-
-       if (module_name == NULL) {
-               TALLOC_FREE(ctx);
-               return NT_STATUS_INVALID_PARAMETER;
-       }
-
-       /* Check for absolute path */
-
-       DEBUG(5, ("%s module '%s'\n", is_probe ? "Probing" : "Loading", 
module_name));
-
-       if (subsystem && module_name[0] != '/') {
-               full_path = talloc_asprintf(ctx,
-                                           "%s/%s.%s",
-                                           modules_path(ctx, subsystem),
-                                           module_name,
-                                           shlib_ext());
-               if (!full_path) {
-                       TALLOC_FREE(ctx);
-                       return NT_STATUS_NO_MEMORY;
-               }
-
-               DEBUG(5, ("%s module '%s': Trying to load from %s\n",
-                         is_probe ? "Probing": "Loading", module_name, 
full_path));
-               init = load_module(full_path, is_probe, &handle);
-       } else {
-               init = load_module(module_name, is_probe, &handle);
-       }
+       DBG_INFO("%s module '%s'\n",
+                is_probe ? "Probing" : "Loading",
+                module_path);
 
-       if (!init) {
-               TALLOC_FREE(ctx);
+       init = load_module(module_path, is_probe, &handle);
+       if (init == NULL) {
                return NT_STATUS_UNSUCCESSFUL;
        }
 
-       DEBUG(2, ("Module '%s' loaded\n", module_name));
+       DBG_NOTICE("Module '%s' loaded\n", module_path);
 
        status = init(NULL);
        if (!NT_STATUS_IS_OK(status)) {
-               DEBUG(0, ("Module '%s' initialization failed: %s\n",
-                         module_name, get_friendly_nt_error_msg(status)));
+               DBG_ERR("Module '%s' initialization failed: %s\n",
+                       module_path,
+                       get_friendly_nt_error_msg(status));
                dlclose(handle);
+               return status;
        }
-       TALLOC_FREE(ctx);
-       return status;
+
+       return NT_STATUS_OK;
 }
 
 /* Load all modules in list and return number of
  * modules that has been successfully loaded */
-int smb_load_modules(const char **modules)
+int smb_load_all_modules_absoute_path(const char **modules)
 {
        int i;
        int success = 0;
 
-       for(i = 0; modules[i]; i++){
-               if(NT_STATUS_IS_OK(do_smb_load_module(NULL, modules[i], 
false))) {
+       for(i = 0; modules[i] != NULL; i++) {
+               const char *module = modules[i];
+               NTSTATUS status;
+
+               if (module[0] != '/') {
+                       continue;
+               }
+
+               status = load_module_absolute_path(module, false);
+               if (NT_STATUS_IS_OK(status)) {
                        success++;
                }
        }
@@ -223,12 +203,117 @@ int smb_load_modules(const char **modules)
        return success;
 }
 
+/**
+ * @brief Check if a module exist and load it.
+ *
+ * @param[in]  subsystem  The name of the subsystem the module belongs too.
+ *
+ * @param[in]  module     The name of the module
+ *
+ * @return  A NTSTATUS code
+ */
 NTSTATUS smb_probe_module(const char *subsystem, const char *module)
 {
-       return do_smb_load_module(subsystem, module, true);
+       NTSTATUS status;
+       char *module_path = NULL;
+       TALLOC_CTX *tmp_ctx = talloc_stackframe();
+
+       if (subsystem == NULL) {
+               status = NT_STATUS_INVALID_PARAMETER;
+               goto done;
+       }
+       if (module == NULL) {
+               status = NT_STATUS_INVALID_PARAMETER;
+               goto done;
+       }
+
+       if (strchr(module, '/')) {
+               status = NT_STATUS_INVALID_PARAMETER;
+               goto done;
+       }
+
+       module_path = talloc_asprintf(tmp_ctx,
+                                     "%s/%s.%s",
+                                     modules_path(tmp_ctx, subsystem),
+                                     module,
+                                     shlib_ext());
+       if (module_path == NULL) {
+               status = NT_STATUS_NO_MEMORY;
+               goto done;
+       }
+
+       status = load_module_absolute_path(module_path, true);
+
+done:
+       TALLOC_FREE(tmp_ctx);
+       return status;
+}
+
+/**
+ * @brief Check if a module exist and load it.
+ *
+ * Warning: Using this function can have security implecations!
+ *
+ * @param[in]  subsystem  The name of the subsystem the module belongs too.
+ *
+ * @param[in]  module     Load a module using an abolute path.
+ *
+ * @return  A NTSTATUS code
+ */
+NTSTATUS smb_probe_module_absolute_path(const char *module)
+{
+       if (module == NULL) {
+               return NT_STATUS_INVALID_PARAMETER;
+       }
+       if (module[0] != '/') {
+               return NT_STATUS_INVALID_PARAMETER;
+       }
+
+       return load_module_absolute_path(module, true);
 }
 
+/**
+ * @brief Load a module.
+ *
+ * @param[in]  subsystem  The name of the subsystem the module belongs too.
+ *
+ * @param[in]  module     Check if a module exists and load it.
+ *
+ * @return  A NTSTATUS code
+ */
 NTSTATUS smb_load_module(const char *subsystem, const char *module)
 {
-       return do_smb_load_module(subsystem, module, false);
+       NTSTATUS status;
+       char *module_path = NULL;
+       TALLOC_CTX *tmp_ctx = talloc_stackframe();
+
+       if (subsystem == NULL) {
+               status = NT_STATUS_INVALID_PARAMETER;
+               goto done;
+       }
+       if (module == NULL) {
+               status = NT_STATUS_INVALID_PARAMETER;
+               goto done;
+       }
+
+       if (strchr(module, '/')) {
+               status = NT_STATUS_INVALID_PARAMETER;
+               goto done;
+       }
+
+       module_path = talloc_asprintf(tmp_ctx,
+                                     "%s/%s.%s",
+                                     modules_path(tmp_ctx, subsystem),
+                                     module,
+                                     shlib_ext());
+       if (module_path == NULL) {
+               status = NT_STATUS_NO_MEMORY;
+               goto done;
+       }
+
+       status = load_module_absolute_path(module_path, false);
+
+done:
+       TALLOC_FREE(tmp_ctx);
+       return status;
 }
diff --git a/lib/util/msghdr.c b/lib/util/msghdr.c
index 4b88c1a..fec5446 100644
--- a/lib/util/msghdr.c
+++ b/lib/util/msghdr.c
@@ -37,13 +37,19 @@ ssize_t msghdr_prep_fds(struct msghdr *msg, uint8_t *buf, 
size_t bufsize,
                        msg->msg_control = NULL;
                        msg->msg_controllen = 0;
                }
-               return 0;
+               /*
+                * C99 doesn't allow 0-length arrays
+                */
+               return 1;
        }
        if (num_fds > INT8_MAX) {
                return -1;
        }
        if ((msg == NULL) || (cmsg_space > bufsize)) {
-               return cmsg_space;
+               /*
+                * C99 doesn't allow 0-length arrays
+                */
+               return MAX(cmsg_space, 1);
        }
 
        msg->msg_control = buf;
diff --git a/lib/util/samba_modules.h b/lib/util/samba_modules.h
index 1ae9c6e..c698691 100644
--- a/lib/util/samba_modules.h
+++ b/lib/util/samba_modules.h
@@ -53,8 +53,9 @@ bool run_init_functions(TALLOC_CTX *ctx, init_module_fn *fns);
  */
 init_module_fn *load_samba_modules(TALLOC_CTX *mem_ctx, const char *subsystem);
 
-int smb_load_modules(const char **modules);
+int smb_load_all_modules_absoute_path(const char **modules);
 NTSTATUS smb_probe_module(const char *subsystem, const char *module);
+NTSTATUS smb_probe_module_absolute_path(const char *module);
 NTSTATUS smb_load_module(const char *subsystem, const char *module);
 
 #endif /* _SAMBA_MODULES_H */
diff --git a/selftest/tests.py b/selftest/tests.py
index e3dd914..b9c470c 100644
--- a/selftest/tests.py
+++ b/selftest/tests.py
@@ -145,3 +145,7 @@ if with_pam:
 if with_cmocka:
     plantestsuite("samba.unittests.krb5samba", "none",
                   [os.path.join(bindir(), 
"default/testsuite/unittests/test_krb5samba")])
+    plantestsuite("samba.unittests.sambafs_srv_pipe", "none",
+                  [os.path.join(bindir(), 
"default/testsuite/unittests/test_sambafs_srv_pipe")])
+    plantestsuite("samba.unittests.lib_util_modules", "none",
+                  [os.path.join(bindir(), 
"default/testsuite/unittests/test_lib_util_modules")])
diff --git a/source3/smbd/perfcount.c b/source3/smbd/perfcount.c
index a7c268a..1555ea2 100644
--- a/source3/smbd/perfcount.c
+++ b/source3/smbd/perfcount.c
@@ -144,7 +144,7 @@ static bool smb_load_perfcount_module(const char *name)
 
        /* load the perfcounter module */
        if((entry = smb_perfcount_find_module(module_name)) ||
-          (NT_STATUS_IS_OK(smb_probe_module("perfcount", module_path)) &&
+          (NT_STATUS_IS_OK(smb_probe_module_absolute_path(module_path)) &&
                (entry = smb_perfcount_find_module(module_name)))) {
 
                DEBUG(3,("Successfully loaded perfcounter module [%s] \n", 
name));
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index 8f097ec..d5c03b9 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -4018,7 +4018,7 @@ void smbd_process(struct tevent_context *ev_ctx,
                           locaddr);
 
        if (lp_preload_modules()) {
-               smb_load_modules(lp_preload_modules());
+               smb_load_all_modules_absoute_path(lp_preload_modules());
        }
 
        smb_perfcount_init();
diff --git a/source3/utils/testparm.c b/source3/utils/testparm.c
index 7883bca..9589201 100644
--- a/source3/utils/testparm.c
+++ b/source3/utils/testparm.c
@@ -229,6 +229,18 @@ static int do_global_checks(void)
                                "must differ.\n\n");
        }
 
+       if (lp_client_ipc_signing() == SMB_SIGNING_IF_REQUIRED
+        || lp_client_ipc_signing() == SMB_SIGNING_OFF) {
+               fprintf(stderr, "WARNING: The 'client ipc signing' value "
+                       "%s SMB signing is not used when contacting a "
+                       "domain controller or other server. "
+                       "This setting is not recommended; please be "
+                       "aware of the security implications when using "
+                       "this configuration setting.\n\n",
+                       lp_client_ipc_signing() == SMB_SIGNING_OFF ?
+                       "ensures" : "may mean");
+       }
+
        if (strlen(lp_netbios_name()) > 15) {
                fprintf(stderr, "WARNING: The 'netbios name' is too long "
                                "(max. 15 chars).\n\n");
diff --git a/testsuite/unittests/rpc_test_dummy_module.c 
b/testsuite/unittests/rpc_test_dummy_module.c
new file mode 100644
index 0000000..d067b6e
--- /dev/null
+++ b/testsuite/unittests/rpc_test_dummy_module.c
@@ -0,0 +1,20 @@
+#include <stdio.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <unistd.h>
+
+int samba_init_module(void);
+int samba_init_module(void)
+{
+       int rc;
+
+       fprintf(stderr, "Test dummy executed!\n");
+
+       rc = setenv("UNITTEST_DUMMY_MODULE_LOADED", "TRUE", 1);
+       if (rc < 0) {
+               kill(getpid(), SIGILL);
+               exit(-1);
+       }
+
+       return 0;
+}
diff --git a/testsuite/unittests/test_lib_util_modules.c 
b/testsuite/unittests/test_lib_util_modules.c
new file mode 100644
index 0000000..c92dafd
--- /dev/null
+++ b/testsuite/unittests/test_lib_util_modules.c
@@ -0,0 +1,76 @@
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <talloc.h>
+
+#include "include/config.h"
+#include "libcli/util/ntstatus.h"
+#include "lib/util/samba_modules.h"
+
+static int teardown(void **state)
+{
+       unsetenv("UNITTEST_DUMMY_MODULE_LOADED");
+
+       return 0;
+}
+
+static void test_samba_module_probe(void **state)
+{
+       NTSTATUS status;
+
+       status = smb_probe_module("auth", "unix");
+       assert_true(NT_STATUS_IS_OK(status));
+}
+
+static void test_samba_module_probe_dummy(void **state)
+{
+       const char *module_env;
+       NTSTATUS status;
+
+       status = smb_probe_module("rpc", "test_dummy_module");
+       assert_true(NT_STATUS_IS_OK(status));
+
+       module_env = getenv("UNITTEST_DUMMY_MODULE_LOADED");
+       assert_non_null(module_env);
+       assert_string_equal(module_env, "TRUE");
+}
+
+static void test_samba_module_probe_slash(void **state)
+{
+       char dummy_module_path[4096] = {0};
+       const char *module_env;
+       NTSTATUS status;
+
+       snprintf(dummy_module_path,
+                sizeof(dummy_module_path),
+                "%s/bin/modules/rpc/test_dummy_module.so",
+                SRCDIR);
+
+       status = smb_probe_module("rpc", dummy_module_path);
+       assert_true(NT_STATUS_IS_ERR(status));
+
+       module_env = getenv("UNITTEST_DUMMY_MODULE_LOADED");
+       assert_null(module_env);
+}
+
+int main(void) {
+       const struct CMUnitTest tests[] = {
+               cmocka_unit_test_teardown(test_samba_module_probe,
+                                         teardown),
+               cmocka_unit_test_teardown(test_samba_module_probe_dummy,
+                                         teardown),
+               cmocka_unit_test_teardown(test_samba_module_probe_slash,
+                                         teardown),
+       };
+
+       cmocka_set_message_output(CM_OUTPUT_SUBUNIT);
+       return cmocka_run_group_tests(tests, NULL, NULL);
+}
diff --git a/testsuite/unittests/test_sambafs_srv_pipe.c 
b/testsuite/unittests/test_sambafs_srv_pipe.c
new file mode 100644
index 0000000..641e99d
--- /dev/null
+++ b/testsuite/unittests/test_sambafs_srv_pipe.c
@@ -0,0 +1,76 @@
+#include <errno.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <cmocka.h>
+
+#include "include/config.h"
+#include "librpc/gen_ndr/ndr_samr.h"
+#include "source3/rpc_server/srv_pipe.h"
+#include "librpc/gen_ndr/srv_samr.h"
+
+static int setup_samr(void **state)
+{
+       rpc_samr_init(NULL);
+
+       return 0;
+}
+
+static int teardown(void **state)
+{
+       unsetenv("UNITTEST_DUMMY_MODULE_LOADED");
+
+       return 0;
+}
+
+static int teardown_samr(void **state)
+{
+       rpc_samr_shutdown();
+
+       teardown(state);
+
+       return 0;
+}
+
+static void test_is_known_pipename(void **state)


-- 
Samba Shared Repository

Reply via email to