From 64b9df1b6dbf629437b7c1fa4caeca94fb62c7b0 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Wed, 22 Nov 2017 11:49:57 +0100 Subject: [PATCH 1/3] s3/loadparm: allocate a fresh sDefault object per lp_ctx This is in preperation of preventing direct access to sDefault in all places that currently modify it. As currently s3/loadparm is afaict not accessing lp_ctx->sDefault, but changes sDefault indirectly through lp_parm_ptr() this change is just a safety measure to prevent future breakage. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13051 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit 1fc103547023aa1c880713e5b65ec164acb58b54) --- source3/param/loadparm.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index d5b1c56e21e..1cef3cbf69c 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -959,7 +959,14 @@ static struct loadparm_context *setup_lp_context(TALLOC_CTX *mem_ctx) return NULL; } - lp_ctx->sDefault = &sDefault; + lp_ctx->sDefault = talloc_zero(lp_ctx, struct loadparm_service); + if (lp_ctx->sDefault == NULL) { + DBG_ERR("talloc_zero failed\n"); + TALLOC_FREE(lp_ctx); + return NULL; + } + + *lp_ctx->sDefault = sDefault; lp_ctx->services = NULL; /* We do not want to access this directly */ lp_ctx->bInGlobalSection = bInGlobalSection; lp_ctx->flags = flags_list; -- 2.13.6 From 5a320ff1662f45b3e663f17047ebb67e80cf3edb Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 21 Nov 2017 14:28:48 +0100 Subject: [PATCH 2/3] s3/loadparm: ensure default service options are not changed Rename sDefault to _sDefault and make it const. sDefault is make a copy of _sDefault in in the initialisation function lp_load_ex(). As we may end up in setup_lp_context() without going through lp_load_ex(), sDefault may still be uninitialized at that point, so I'm initializing lp_ctx->sDefault from _sDefault. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13051 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison (cherry picked from commit ea4e6f95ae5c97e8570b8090ee7e7a577b49a8c3) --- source3/param/loadparm.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index 1cef3cbf69c..4f8544761da 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -111,7 +111,7 @@ static bool defaults_saved = false; static struct loadparm_global Globals; /* This is a default service used to prime a services structure */ -static struct loadparm_service sDefault = +static const struct loadparm_service _sDefault = { .valid = true, .autoloaded = false, @@ -250,6 +250,12 @@ static struct loadparm_service sDefault = .dummy = "" }; +/* + * This is a copy of the default service structure. Service options in the + * global section would otherwise overwrite the initial default values. + */ +static struct loadparm_service sDefault; + /* local variables */ static struct loadparm_service **ServicePtrs = NULL; static int iNumServices = 0; @@ -966,7 +972,7 @@ static struct loadparm_context *setup_lp_context(TALLOC_CTX *mem_ctx) return NULL; } - *lp_ctx->sDefault = sDefault; + *lp_ctx->sDefault = _sDefault; lp_ctx->services = NULL; /* We do not want to access this directly */ lp_ctx->bInGlobalSection = bInGlobalSection; lp_ctx->flags = flags_list; @@ -3856,6 +3862,7 @@ static bool lp_load_ex(const char *pszFname, bInGlobalSection = true; bGlobalOnly = global_only; bAllowIncludeRegistry = allow_include_registry; + sDefault = _sDefault; lp_ctx = setup_lp_context(talloc_tos()); -- 2.13.6 From c9998b13ac6634b6f3c2855009164397812126d3 Mon Sep 17 00:00:00 2001 From: Ralph Boehme Date: Tue, 21 Nov 2017 14:34:28 +0100 Subject: [PATCH 3/3] s3/loadparm: don't mark IPC$ as autoloaded A related problem that affects configuration for the hidden IPC$ share. This share is marked a "autoloaded" and such shares are not reloaded when requested. That resulted in the tcon to IPC$ still using encrpytion after running the following sequence of changes: 1. stop Samba 2. set [global] smb encrypt = required 3. start Samba 4. remove [global] smb encrypt = required 5. smbcontrol smbd reload-config 6a bin/smbclient -U slow%x //localhost/raw -c quit, or 6b bin/smbclient -U slow%x -mNT1 //localhost/raw -c ls In 6a the client simply encrypted packets on the IPC$ tcon. In 6b the client got a tcon failure with NT_STATUS_ACCESS_DENIED, but silently ignore the error. Bug: https://bugzilla.samba.org/show_bug.cgi?id=13051 Signed-off-by: Ralph Boehme Reviewed-by: Jeremy Allison Autobuild-User(master): Jeremy Allison Autobuild-Date(master): Tue Nov 28 02:02:37 CET 2017 on sn-devel-144 (cherry picked from commit deaaff6843159f02bb15aeaf457f8af305e40164) --- source3/param/loadparm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c index 4f8544761da..ba19d46c7de 100644 --- a/source3/param/loadparm.c +++ b/source3/param/loadparm.c @@ -1604,7 +1604,7 @@ static bool lp_add_ipc(const char *ipc_name, bool guest_ok) ServicePtrs[i]->guest_ok = guest_ok; ServicePtrs[i]->printable = false; ServicePtrs[i]->browseable = sDefault.browseable; - ServicePtrs[i]->autoloaded = true; + ServicePtrs[i]->autoloaded = false; DEBUG(3, ("adding IPC service\n")); -- 2.13.6