smb.conf in registry

Michael Adam obnox at samba.org
Sun Sep 22 17:49:02 CEST 2013


On 2013-09-21 at 21:39 +0200, Michael Adam wrote:
> On 2013-09-20 at 22:20 -0400, Simo wrote:
> > On Sat, 2013-09-21 at 03:14 +0200, Michael Adam wrote:
> > > anybody?
> > 
> > Reviewed by: Simo Sorce <idra at samba.org>
> 
> Thanks - pushed to autobuild.

Hmm I am sorry, but it was not quite as simple as this.
(I have to admit that I hadn't run a full autobuild before...)
- Firstly, the synonym "state dir" that I added does not exist.
- Secondly, the blackbox registry.roundtrip needed to be adapted.
- Thirdly, I have also adapted the "net rpc conf" command with
  corresponding tests.
- Then I have added a testcase that explicitly tests the
  forbidden parameters.
- Finally, I have done a few cleanups in the code.

Attached find the corresponding patchset.
I have run an autobuild which faile in an unrelated test
(samba4.blackbox.dbcheck(vampire_dc)).
Running an autobuild again now.

Please review/comment/push as appropriate.

Thanks - Michael
-------------- next part --------------
From 026e9958402818bd483a1045c92df91aceb33d08 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 03:39:48 +0200
Subject: [PATCH 01/17] libsmbconf:registry: rework
 smbconf_reg_parameter_forbidden(), renaming it.

The logic is inverted, the lp_parameter_is_invalid call of
smbconf_reg_valname_valid() is included, and the function
is renamed to smbconf_reg_parameter_is_valid().

Use the new function everywhere in smbconf registry backend.
And remove corresponding reverse function smbconf_reg_valname_valid().

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/lib/smbconf/smbconf_reg.c |   34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/source3/lib/smbconf/smbconf_reg.c b/source3/lib/smbconf/smbconf_reg.c
index ec6b93f..6848a73 100644
--- a/source3/lib/smbconf/smbconf_reg.c
+++ b/source3/lib/smbconf/smbconf_reg.c
@@ -50,13 +50,14 @@ static struct reg_private_data *rpd(struct smbconf_ctx *ctx)
 	return (struct reg_private_data *)(ctx->data);
 }
 
-/*
- * check whether a given value name is forbidden in registry (smbconf)
+/**
+ * Check whether a given parameter name is valid in the
+ * smbconf registry backend.
  */
-static bool smbconf_reg_valname_forbidden(const char *valname)
+static bool smbconf_reg_parameter_is_valid(const char *param_name)
 {
 	/* hard code the list of forbidden names here for now */
-	const char *forbidden_valnames[] = {
+	const char *forbidden_names[] = {
 		"lock directory",
 		"lock dir",
 		"config backend",
@@ -66,18 +67,17 @@ static bool smbconf_reg_valname_forbidden(const char *valname)
 	};
 	const char **forbidden = NULL;
 
-	for (forbidden = forbidden_valnames; *forbidden != NULL; forbidden++) {
-		if (strwicmp(valname, *forbidden) == 0) {
-			return true;
+	if (!lp_parameter_is_valid(param_name)) {
+		return false;
+	}
+
+	for (forbidden = forbidden_names; *forbidden != NULL; forbidden++) {
+		if (strwicmp(param_name, *forbidden) == 0) {
+			return false;
 		}
 	}
-	return false;
-}
 
-static bool smbconf_reg_valname_valid(const char *valname)
-{
-	return (!smbconf_reg_valname_forbidden(valname) &&
-		lp_parameter_is_valid(valname));
+	return true;
 }
 
 /**
@@ -189,7 +189,7 @@ static sbcErr smbconf_reg_set_value(struct registry_key *key,
 		goto done;
 	}
 
-	if (smbconf_reg_valname_forbidden(canon_valname)) {
+	if (!smbconf_reg_parameter_is_valid(canon_valname)) {
 		DEBUG(5, ("Parameter '%s' not allowed in registry.\n",
 			  canon_valname));
 		err = SBC_ERR_INVALID_PARAM;
@@ -456,7 +456,7 @@ static sbcErr smbconf_reg_get_values(TALLOC_CTX *mem_ctx,
 	{
 		char *valstring;
 
-		if (!smbconf_reg_valname_valid(valname)) {
+		if (!smbconf_reg_parameter_is_valid(valname)) {
 			continue;
 		}
 
@@ -1008,7 +1008,7 @@ static sbcErr smbconf_reg_get_parameter(struct smbconf_ctx *ctx,
 		goto done;
 	}
 
-	if (!smbconf_reg_valname_valid(param)) {
+	if (!smbconf_reg_parameter_is_valid(param)) {
 		err = SBC_ERR_INVALID_PARAM;
 		goto done;
 	}
@@ -1053,7 +1053,7 @@ static sbcErr smbconf_reg_delete_parameter(struct smbconf_ctx *ctx,
 		goto done;
 	}
 
-	if (!smbconf_reg_valname_valid(param)) {
+	if (!smbconf_reg_parameter_is_valid(param)) {
 		err = SBC_ERR_INVALID_PARAM;
 		goto done;
 	}
-- 
1.7.9.5


From 17d41a36b7f27d918e66d880ceabd165d5da5620 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 10:27:38 +0200
Subject: [PATCH 02/17] libsmbconf:registry: publish
 smbconf_reg_parameter_is_valid()

So that this does not need to be duplicated..

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/lib/smbconf/smbconf_reg.c |    2 +-
 source3/lib/smbconf/smbconf_reg.h |    5 +++++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/source3/lib/smbconf/smbconf_reg.c b/source3/lib/smbconf/smbconf_reg.c
index 6848a73..696dd1f 100644
--- a/source3/lib/smbconf/smbconf_reg.c
+++ b/source3/lib/smbconf/smbconf_reg.c
@@ -54,7 +54,7 @@ static struct reg_private_data *rpd(struct smbconf_ctx *ctx)
  * Check whether a given parameter name is valid in the
  * smbconf registry backend.
  */
-static bool smbconf_reg_parameter_is_valid(const char *param_name)
+bool smbconf_reg_parameter_is_valid(const char *param_name)
 {
 	/* hard code the list of forbidden names here for now */
 	const char *forbidden_names[] = {
diff --git a/source3/lib/smbconf/smbconf_reg.h b/source3/lib/smbconf/smbconf_reg.h
index 2c49057..a3f343f 100644
--- a/source3/lib/smbconf/smbconf_reg.h
+++ b/source3/lib/smbconf/smbconf_reg.h
@@ -29,5 +29,10 @@ struct smbconf_ctx;
 sbcErr smbconf_init_reg(TALLOC_CTX *mem_ctx, struct smbconf_ctx **conf_ctx,
 			const char *path);
 
+/**
+ * Check whether a given parameter name is valid in the
+ * smbconf registry backend.
+ */
+bool smbconf_reg_parameter_is_valid(const char *param_name);
 
 #endif /*  _LIBSMBCONF_REG_H_  */
-- 
1.7.9.5


From 5140bc2ed2efd264dd9847be31fe2d5a4b453135 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 03:44:58 +0200
Subject: [PATCH 03/17] s3:net rpc conf: use the published
 smbconf_reg_parameter_is_valid()

Instead of the duplicated rpc_conf_reg_valname_forbidden()

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/utils/net_rpc_conf.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/utils/net_rpc_conf.c b/source3/utils/net_rpc_conf.c
index 8c5717c..5396e0f 100644
--- a/source3/utils/net_rpc_conf.c
+++ b/source3/utils/net_rpc_conf.c
@@ -1859,7 +1859,7 @@ static NTSTATUS rpc_conf_setparm_internal(struct net_context *c,
 		goto error;
 	}
 
-	if (rpc_conf_reg_valname_forbidden(canon_valname)) {
+	if (!smbconf_reg_parameter_is_valid(canon_valname)) {
 		d_fprintf(stderr, "Parameter '%s' not allowed in registry.\n",
 			  canon_valname);
 		werr = WERR_INVALID_PARAM;
-- 
1.7.9.5


From 8564407345e2f1b1b36d8e775ad060dbef606d0f Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 03:45:28 +0200
Subject: [PATCH 04/17] s3:net rpc conf: remove the (now) unused
 rpc_conf_reg_valname_forbidden()

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/utils/net_rpc_conf.c |   21 ---------------------
 1 file changed, 21 deletions(-)

diff --git a/source3/utils/net_rpc_conf.c b/source3/utils/net_rpc_conf.c
index 5396e0f..0893442 100644
--- a/source3/utils/net_rpc_conf.c
+++ b/source3/utils/net_rpc_conf.c
@@ -175,27 +175,6 @@ static int rpc_conf_delincludes_usage(struct net_context *c, int argc,
  *
  **********************************************************/
 
-static bool rpc_conf_reg_valname_forbidden(const char * valname)
-{
-	const char *forbidden_valnames[] = {
-		"lock directory",
-		"lock dir",
-		"config backend",
-		"include",
-		"includes", /* this has a special meaning internally */
-		NULL
-	};
-	const char **forbidden = NULL;
-
-	for (forbidden = forbidden_valnames; *forbidden != NULL; forbidden++) {
-		if (strwicmp(valname, *forbidden) == 0) {
-			return true;
-		}
-	}
-	return false;
-
-}
-
 /*
  * The function deletes a registry value with the name 'value' from the share
  * with the name 'share_name'. 'parent_hnd' is the handle for the smbconf key.
-- 
1.7.9.5


From 650d412b8c2f69c0367bd7cbde674179136b9bdf Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 07:51:05 +0200
Subject: [PATCH 05/17] s3:net rpc conf: print the provided parameter name on
 error, not the canonicalized one

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/utils/net_rpc_conf.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/source3/utils/net_rpc_conf.c b/source3/utils/net_rpc_conf.c
index 0893442..49c2eaf 100644
--- a/source3/utils/net_rpc_conf.c
+++ b/source3/utils/net_rpc_conf.c
@@ -1840,7 +1840,7 @@ static NTSTATUS rpc_conf_setparm_internal(struct net_context *c,
 
 	if (!smbconf_reg_parameter_is_valid(canon_valname)) {
 		d_fprintf(stderr, "Parameter '%s' not allowed in registry.\n",
-			  canon_valname);
+			  argv[1]);
 		werr = WERR_INVALID_PARAM;
 		goto error;
 	}
-- 
1.7.9.5


From 53932936ef8f2083b4517037a07cdd744ce06daf Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 08:47:14 +0200
Subject: [PATCH 06/17] libsmbconf:registry: reorganize the validity check and
 canonicalization of the input in "setparm"

- first check that the name is an smbconf parameter
- then check that the parameter is allowed in the registry config
- then check that a global parameter is not to be set in a service section
- then canonicalize the parameter and value name, thereby checking that the
  value has valid format

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/lib/smbconf/smbconf_reg.c |   33 +++++++++++++++++++--------------
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/source3/lib/smbconf/smbconf_reg.c b/source3/lib/smbconf/smbconf_reg.c
index 696dd1f..9bdcbfd 100644
--- a/source3/lib/smbconf/smbconf_reg.c
+++ b/source3/lib/smbconf/smbconf_reg.c
@@ -174,24 +174,15 @@ static sbcErr smbconf_reg_set_value(struct registry_key *key,
 	const char *canon_valname;
 	const char *canon_valstr;
 
-	if (!lp_canonicalize_parameter_with_value(valname, valstr,
-						  &canon_valname,
-						  &canon_valstr))
-	{
-		if (canon_valname == NULL) {
-			DEBUG(5, ("invalid parameter '%s' given\n",
-				  valname));
-		} else {
-			DEBUG(5, ("invalid value '%s' given for "
-				  "parameter '%s'\n", valstr, valname));
-		}
+	if (!lp_parameter_is_valid(valname)) {
+		DEBUG(5, ("Invalid parameter '%s' given.\n", valname));
 		err = SBC_ERR_INVALID_PARAM;
 		goto done;
 	}
 
-	if (!smbconf_reg_parameter_is_valid(canon_valname)) {
+	if (!smbconf_reg_parameter_is_valid(valname)) {
 		DEBUG(5, ("Parameter '%s' not allowed in registry.\n",
-			  canon_valname));
+			  valname));
 		err = SBC_ERR_INVALID_PARAM;
 		goto done;
 	}
@@ -208,12 +199,26 @@ static sbcErr smbconf_reg_set_value(struct registry_key *key,
 	    lp_parameter_is_global(valname))
 	{
 		DEBUG(5, ("Global parameter '%s' not allowed in "
-			  "service definition ('%s').\n", canon_valname,
+			  "service definition ('%s').\n", valname,
 			  subkeyname));
 		err = SBC_ERR_INVALID_PARAM;
 		goto done;
 	}
 
+	if (!lp_canonicalize_parameter_with_value(valname, valstr,
+						  &canon_valname,
+						  &canon_valstr))
+	{
+		/*
+		 * We already know the parameter name is valid.
+		 * So the value must be invalid.
+		 */
+		DEBUG(5, ("invalid value '%s' given for parameter '%s'\n",
+			  valstr, valname));
+		err = SBC_ERR_INVALID_PARAM;
+		goto done;
+	}
+
 	ZERO_STRUCT(val);
 
 	val.type = REG_SZ;
-- 
1.7.9.5


From 892a3e35e14612d0f7be260c0f10d194f45e6bb9 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 08:54:30 +0200
Subject: [PATCH 07/17] libsmbconf:registry: clarify the appearance of
 "includes" in forbidden_names

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/lib/smbconf/smbconf_reg.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/source3/lib/smbconf/smbconf_reg.c b/source3/lib/smbconf/smbconf_reg.c
index 9bdcbfd..84294ab 100644
--- a/source3/lib/smbconf/smbconf_reg.c
+++ b/source3/lib/smbconf/smbconf_reg.c
@@ -62,7 +62,13 @@ bool smbconf_reg_parameter_is_valid(const char *param_name)
 		"lock dir",
 		"config backend",
 		"include",
-		"includes", /* this has a special meaning internally */
+		/*
+		 * "includes" has a special meaning internally.
+		 * It is currently not necessary to list it here since it is
+		 * not a valid parameter. But for clarity and safety, we keep
+		 * it for now.
+		 */
+		INCLUDES_VALNAME,
 		NULL
 	};
 	const char **forbidden = NULL;
-- 
1.7.9.5


From c353a352405f9d3498e299b8b8f7c9970028b382 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 08:26:47 +0200
Subject: [PATCH 08/17] s3:net rpc conf: reorganize the validity check and
 canonicalization of the input in "setparm"

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/utils/net_rpc_conf.c |   40 +++++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/source3/utils/net_rpc_conf.c b/source3/utils/net_rpc_conf.c
index 49c2eaf..edf6120 100644
--- a/source3/utils/net_rpc_conf.c
+++ b/source3/utils/net_rpc_conf.c
@@ -1822,39 +1822,45 @@ static NTSTATUS rpc_conf_setparm_internal(struct net_context *c,
 			break;
 	}
 
-	/* check if parameter is valid for writing */
-	if (!lp_canonicalize_parameter_with_value(argv[1], argv[2],
-						  &canon_valname,
-						  &canon_valstr))
-	{
-		if (canon_valname == NULL) {
-			d_fprintf(stderr, "invalid parameter '%s' given\n",
-				  argv[1]);
-		} else {
-			d_fprintf(stderr, "invalid value '%s' given for "
-				  "parameter '%s'\n", argv[1], argv[2]);
-		}
+	/*
+	 * check if parameter is valid for writing
+	 */
+
+	if (!lp_parameter_is_valid(argv[1])) {
+		d_fprintf(stderr, "Invalid parameter '%s' given.\n", argv[1]);
 		werr = WERR_INVALID_PARAM;
 		goto error;
 	}
 
-	if (!smbconf_reg_parameter_is_valid(canon_valname)) {
+	if (!smbconf_reg_parameter_is_valid(argv[1])) {
 		d_fprintf(stderr, "Parameter '%s' not allowed in registry.\n",
 			  argv[1]);
 		werr = WERR_INVALID_PARAM;
 		goto error;
 	}
 
-	if (!strequal(argv[0], "global") &&
-	    lp_parameter_is_global(argv[1]))
-	{
+	if (!strequal(argv[0], "global") && lp_parameter_is_global(argv[1])) {
 		d_fprintf(stderr, "Global parameter '%s' not allowed in "
-			  "service definition ('%s').\n", canon_valname,
+			  "service definition ('%s').\n", argv[1],
 			  argv[0]);
 		werr = WERR_INVALID_PARAM;
 		goto error;
 	}
 
+	if (!lp_canonicalize_parameter_with_value(argv[1], argv[2],
+						  &canon_valname,
+						  &canon_valstr))
+	{
+		/*
+		 * We already know the parameter name is valid.
+		 * So the value must be invalid.
+		 */
+		d_fprintf(stderr, "invalid value '%s' given for "
+			  "parameter '%s'\n", argv[1], argv[2]);
+		werr = WERR_INVALID_PARAM;
+		goto error;
+	}
+
 	/* set the parameter */
 	status = dcerpc_winreg_set_sz(frame, b, &share_hnd,
 					argv[1], argv[2], &werr);
-- 
1.7.9.5


From e8f3ff5b6d977fcbcb24d1dfde32ad8bfab0db10 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 09:11:55 +0200
Subject: [PATCH 09/17] s3:net rpc conf: setparm: introduce variables
 service_name, param_name, valstr for clarity

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/utils/net_rpc_conf.c |   49 +++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/source3/utils/net_rpc_conf.c b/source3/utils/net_rpc_conf.c
index edf6120..61555e2 100644
--- a/source3/utils/net_rpc_conf.c
+++ b/source3/utils/net_rpc_conf.c
@@ -1739,6 +1739,7 @@ static NTSTATUS rpc_conf_setparm_internal(struct net_context *c,
 	struct winreg_String key, keyclass;
 	enum winreg_CreateAction action = 0;
 
+	const char *service_name, *param_name, *valstr;
 	const char *canon_valname;
 	const char *canon_valstr;
 
@@ -1770,7 +1771,11 @@ static NTSTATUS rpc_conf_setparm_internal(struct net_context *c,
 		goto error;
 	}
 
-	key.name = argv[0];
+	service_name = argv[0];
+	param_name = argv[1];
+	valstr = argv[2];
+
+	key.name = service_name;
 	keyclass.name = "";
 
 	status = dcerpc_winreg_CreateKey(b, frame, &key_hnd, key, keyclass,
@@ -1779,13 +1784,13 @@ static NTSTATUS rpc_conf_setparm_internal(struct net_context *c,
 
 	if (!(NT_STATUS_IS_OK(status))) {
 		d_fprintf(stderr, _("ERROR: Could not create share key '%s'\n%s\n"),
-				argv[0], nt_errstr(status));
+			  service_name, nt_errstr(status));
 		goto error;
 	}
 
 	if (!W_ERROR_IS_OK(werr)) {
 		d_fprintf(stderr, _("ERROR: Could not create share key '%s'\n%s\n"),
-				argv[0], win_errstr(werr));
+			  service_name, win_errstr(werr));
 		goto error;
 	}
 
@@ -1793,22 +1798,23 @@ static NTSTATUS rpc_conf_setparm_internal(struct net_context *c,
 		case REG_ACTION_NONE:
 			werr = WERR_CREATE_FAILED;
 			d_fprintf(stderr, _("ERROR: Could not create share key '%s'\n%s\n"),
-				argv[0], win_errstr(werr));
+				  service_name, win_errstr(werr));
 			goto error;
 		case REG_CREATED_NEW_KEY:
 			DEBUG(5, ("net rpc conf setparm:"
-					"createkey created %s\n", argv[0]));
+				  "createkey created %s\n", service_name));
 			break;
 		case REG_OPENED_EXISTING_KEY:
 			DEBUG(5, ("net rpc conf setparm:"
-					"createkey opened existing %s\n", argv[0]));
+				  "createkey opened existing %s\n",
+				  service_name));
 
 			/* delete posibly existing value */
 			status = rpc_conf_del_value(frame,
 						    b,
 						    &key_hnd,
-						    argv[0],
-						    argv[1],
+						    service_name,
+						    param_name,
 						    &werr);
 
 			if (!(NT_STATUS_IS_OK(status))) {
@@ -1826,28 +1832,31 @@ static NTSTATUS rpc_conf_setparm_internal(struct net_context *c,
 	 * check if parameter is valid for writing
 	 */
 
-	if (!lp_parameter_is_valid(argv[1])) {
-		d_fprintf(stderr, "Invalid parameter '%s' given.\n", argv[1]);
+	if (!lp_parameter_is_valid(param_name)) {
+		d_fprintf(stderr, "Invalid parameter '%s' given.\n",
+			  param_name);
 		werr = WERR_INVALID_PARAM;
 		goto error;
 	}
 
-	if (!smbconf_reg_parameter_is_valid(argv[1])) {
+	if (!smbconf_reg_parameter_is_valid(param_name)) {
 		d_fprintf(stderr, "Parameter '%s' not allowed in registry.\n",
-			  argv[1]);
+			  param_name);
 		werr = WERR_INVALID_PARAM;
 		goto error;
 	}
 
-	if (!strequal(argv[0], "global") && lp_parameter_is_global(argv[1])) {
+	if (!strequal(service_name, "global") &&
+	   lp_parameter_is_global(param_name))
+	{
 		d_fprintf(stderr, "Global parameter '%s' not allowed in "
-			  "service definition ('%s').\n", argv[1],
-			  argv[0]);
+			  "service definition ('%s').\n", param_name,
+			  service_name);
 		werr = WERR_INVALID_PARAM;
 		goto error;
 	}
 
-	if (!lp_canonicalize_parameter_with_value(argv[1], argv[2],
+	if (!lp_canonicalize_parameter_with_value(param_name, valstr,
 						  &canon_valname,
 						  &canon_valstr))
 	{
@@ -1856,26 +1865,26 @@ static NTSTATUS rpc_conf_setparm_internal(struct net_context *c,
 		 * So the value must be invalid.
 		 */
 		d_fprintf(stderr, "invalid value '%s' given for "
-			  "parameter '%s'\n", argv[1], argv[2]);
+			  "parameter '%s'\n", param_name, valstr);
 		werr = WERR_INVALID_PARAM;
 		goto error;
 	}
 
 	/* set the parameter */
 	status = dcerpc_winreg_set_sz(frame, b, &share_hnd,
-					argv[1], argv[2], &werr);
+				      param_name, valstr, &werr);
 
 	if (!(NT_STATUS_IS_OK(status))) {
 		d_fprintf(stderr, "ERROR: Could not set parameter '%s'"
 				" with value %s\n %s\n",
-				argv[1], argv[2], nt_errstr(status));
+				param_name, valstr, nt_errstr(status));
 		goto error;
 	}
 
 	if (!(W_ERROR_IS_OK(werr))) {
 		d_fprintf(stderr, "ERROR: Could not set parameter '%s'"
 				" with value %s\n %s\n",
-				argv[1], argv[2], win_errstr(werr));
+				param_name, valstr, win_errstr(werr));
 		goto error;
 	}
 
-- 
1.7.9.5


From d3db395d47b4afab4a46eb05e3765623638b64f4 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 09:59:19 +0200
Subject: [PATCH 10/17] s3:net rpc conf: rename
 canon_valname->canon_param_name for clarity in
 setparm.

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/utils/net_rpc_conf.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/source3/utils/net_rpc_conf.c b/source3/utils/net_rpc_conf.c
index 61555e2..4b5f58e 100644
--- a/source3/utils/net_rpc_conf.c
+++ b/source3/utils/net_rpc_conf.c
@@ -1740,7 +1740,7 @@ static NTSTATUS rpc_conf_setparm_internal(struct net_context *c,
 	enum winreg_CreateAction action = 0;
 
 	const char *service_name, *param_name, *valstr;
-	const char *canon_valname;
+	const char *canon_param_name;
 	const char *canon_valstr;
 
 	ZERO_STRUCT(hive_hnd);
@@ -1857,7 +1857,7 @@ static NTSTATUS rpc_conf_setparm_internal(struct net_context *c,
 	}
 
 	if (!lp_canonicalize_parameter_with_value(param_name, valstr,
-						  &canon_valname,
+						  &canon_param_name,
 						  &canon_valstr))
 	{
 		/*
-- 
1.7.9.5


From 065b18a726d8653419eb694ce34bc0d7b463d2db Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 10:00:55 +0200
Subject: [PATCH 11/17] s3:net conf: add the same parameter checks to
 "setparm" as in "net rpc conf".

In "net rpc conf" these checks are necessary, since the that command
uses the plain rpc-registry interface at this moment, and so unfortunately
it has to duplicate the checks from the smbconf library.

Since "net conf" uses the registry, these checks are not necessary in
this command. I add them nonetheless to make the output more similar
to "net rpc conf". It is also a little more user friendy than just
printing "INVLID_PARAMETER" as handed back from libsmbconf.

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/utils/net_conf.c |   32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/source3/utils/net_conf.c b/source3/utils/net_conf.c
index e43cd12..19bf083 100644
--- a/source3/utils/net_conf.c
+++ b/source3/utils/net_conf.c
@@ -764,6 +764,7 @@ static int net_conf_setparm(struct net_context *c, struct smbconf_ctx *conf_ctx,
 	char *service = NULL;
 	char *param = NULL;
 	const char *value_str = NULL;
+	const char *canon_param, *canon_valstr;
 	TALLOC_CTX *mem_ctx = talloc_stackframe();
 
 	if (argc != 3 || c->display_usage) {
@@ -788,6 +789,37 @@ static int net_conf_setparm(struct net_context *c, struct smbconf_ctx *conf_ctx,
 	}
 	value_str = argv[2];
 
+	if (!lp_parameter_is_valid(param)) {
+		d_fprintf(stderr, _("Invalid parameter '%s' given.\n"), param);
+		goto done;
+	}
+
+	if (!smbconf_reg_parameter_is_valid(param)) {
+		d_fprintf(stderr,
+			  _("Parameter '%s' not allowed in registry.\n"),
+			  param);
+		goto done;
+	}
+
+	if (!strequal(service, "global") && lp_parameter_is_global(param)) {
+		d_fprintf(stderr, _("Global parameter '%s' not allowed in "
+			  "service definition ('%s').\n"), param, service);
+		goto done;
+	}
+
+	if (!lp_canonicalize_parameter_with_value(param, value_str,
+						  &canon_param,
+						  &canon_valstr))
+	{
+		/*
+		 * We already know the parameter name is valid.
+		 * So the value must be invalid.
+		 */
+		d_fprintf(stderr, _("Invalid value '%s' given for "
+			  "parameter '%s'.\n"), param, value_str);
+		goto done;
+	}
+
 	err = smbconf_transaction_start(conf_ctx);
 	if (!SBC_ERROR_IS_OK(err)) {
 		d_printf(_("error starting transaction: %s\n"),
-- 
1.7.9.5


From a00dbd8239e8174cf2f14f819afd649e379f6651 Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 03:02:51 +0200
Subject: [PATCH 12/17] selftest: remove unused variables (copy'n'paste...)
 from test_net_conf.sh

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/script/tests/test_net_conf.sh |   19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/source3/script/tests/test_net_conf.sh b/source3/script/tests/test_net_conf.sh
index 6d3d2a1..1a773fa 100755
--- a/source3/script/tests/test_net_conf.sh
+++ b/source3/script/tests/test_net_conf.sh
@@ -42,25 +42,6 @@ incdir=`dirname $0`/../../../testprogs/blackbox
 
 failed=0
 
-SED_INVALID_PARAMS="{
-s/lock directory/;&/g
-s/lock dir/;&/g
-s/modules dir/;&/g
-s/logging/;&/g
-s/status/;&/g
-s/logdir/;&/g
-s/read prediction/;&/g
-s/mkprofile/;&/g
-s/valid chars/;&/g
-s/timesync/;&/g
-s/sambaconf/;&/g
-s/logtype/;&/g
-s/servername/;&/g
-s/postscript/;&/g
-}"
-
-REGPATH="HKLM\Software\Samba"
-
 log_print() {
     RC=$?
     echo "CMD: $*" >>$LOG
-- 
1.7.9.5


From 67da17f6a948424587353a9cf2e2d2f93d954b1b Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 03:06:12 +0200
Subject: [PATCH 13/17] selftest: explain variable SED_INVALID_PARAMS in the
 registry.roundtrip test

Signed-off-by: Michael Adam <obnox at samba.org>
---
 .../script/tests/test_net_registry_roundtrip.sh    |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/source3/script/tests/test_net_registry_roundtrip.sh b/source3/script/tests/test_net_registry_roundtrip.sh
index f215887..88e7297 100755
--- a/source3/script/tests/test_net_registry_roundtrip.sh
+++ b/source3/script/tests/test_net_registry_roundtrip.sh
@@ -32,6 +32,11 @@ incdir=`dirname $0`/../../../testprogs/blackbox
 
 failed=0
 
+#
+# List of parameters to skip when importing configuration files:
+# Either because they are forbidden in the registry or because
+# they are source4-only. Both reasons lead to failure when importing.
+#
 SED_INVALID_PARAMS="{
 s/lock directory/;&/g
 s/lock dir/;&/g
-- 
1.7.9.5


From 8e715a0cdb1a1a562c2e1f8e2bd3c330453fc30e Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sat, 21 Sep 2013 22:34:31 +0200
Subject: [PATCH 14/17] selftest: add regression test for setting invalid
 parameters in registry config via "net [rpc] conf"

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/script/tests/test_net_conf.sh |   49 +++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/source3/script/tests/test_net_conf.sh b/source3/script/tests/test_net_conf.sh
index 1a773fa..c432c3e 100755
--- a/source3/script/tests/test_net_conf.sh
+++ b/source3/script/tests/test_net_conf.sh
@@ -422,6 +422,51 @@ test_conf_setparm_existing()
     fi
 }
 
+test_conf_setparm_forbidden()
+{
+	FORBIDDEN_PARAMS="lock directory
+lock dir
+config backend
+include"
+
+	echo '\nTrying to set forbidden parameters' >> $LOG
+
+	echo '\nDropping existing configuration' >> $LOG
+	$NETCMD conf drop
+	log_print $NETCMD conf drop
+	test "x$?" = "x0" || {
+		echo 'ERROR: RC does not match, expected: 0' | tee -a $LOG
+		return 1
+	}
+
+	OLD_IFS="$IFS"
+	IFS='
+'
+	for PARAM in $FORBIDDEN_PARAMS ; do
+		IFS="$OLD_IFS"
+		echo "Trying to set parameter '$PARAM'" | tee -a $LOG
+		$NETCMD conf setparm global "$PARAM" "value" > $DIR/setparm_forbidden_out 2>&1
+		log_print $NETCMD conf setparm global \""$PARAM"\" "value"
+		test "x$?" = "x0" && {
+			echo "ERROR: setting forbidden parameter '$PARAM' succeeded" | tee -a $LOG
+			return 1
+		}
+
+		echo "output of net command: " | tee -a $LOG
+		cat $DIR/setparm_forbidden_out | tee -a $LOG
+
+		SEARCH="Parameter '$PARAM' not allowed in registry."
+		grep "$SEARCH" $DIR/setparm_forbidden_out >/dev/null 2>>$LOG
+		test "x$?" = "x0" || {
+			echo "ERROR: expected '$SEARCH'" | tee -a $LOG
+			return 1
+		}
+	done
+
+	IFS="$OLD_IFS"
+	return 0
+}
+
 test_conf_setparm_usage()
 {
     echo '\nChecking usage' >>$LOG
@@ -884,6 +929,10 @@ CONF_FILES=$SERVERCONFFILE
 	test_conf_setparm_existing \
 	|| failed=`expr $failed + 1`
 
+    testit "conf_setparm_forbidden" \
+	test_conf_setparm_forbidden \
+	|| failed=`expr $failed + 1`
+
     testit "conf_setparm_usage" \
 	test_conf_setparm_usage \
 	|| failed=`expr $failed + 1`
-- 
1.7.9.5


From 8003976983401df8a184ec904f78dc45be41a8da Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Tue, 17 Sep 2013 19:10:48 +0200
Subject: [PATCH 15/17] libsmbconf:registry: add "state directory" to the list
 of forbidden parameters

At the time when the registry configuration was introduced,
the registry database file was placed in the "lock directory".
So the "lock directory" was added to the list of parameters
that may not be changed in the registry configuration
(because the next config reload would then load a different
 registry and drop all the original seetings).

Later, "state directory" and "cache directory" were introduced,
both defaulting to "lock directory". And the registry's location
was changed to "state directory".

It slipped my attention that the forbidden parameters for the
should have been adapted at the time.

So this patch adds "state directory" to the list.
It keeps the lock directory, to catch the case
where the state directory is not explicitly set, hence
defaulting to the "lock directory".

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/lib/smbconf/smbconf_reg.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/lib/smbconf/smbconf_reg.c b/source3/lib/smbconf/smbconf_reg.c
index 84294ab..ac6b84d 100644
--- a/source3/lib/smbconf/smbconf_reg.c
+++ b/source3/lib/smbconf/smbconf_reg.c
@@ -58,6 +58,7 @@ bool smbconf_reg_parameter_is_valid(const char *param_name)
 {
 	/* hard code the list of forbidden names here for now */
 	const char *forbidden_names[] = {
+		"state directory",
 		"lock directory",
 		"lock dir",
 		"config backend",
-- 
1.7.9.5


From 44b12fb72bc3ed25f8ea4dc0a095f90043a1723c Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sat, 21 Sep 2013 22:38:31 +0200
Subject: [PATCH 16/17] selftest: add "state directory" to the forbidden
 parameters test in net conf

Signed-off-by: Michael Adam <obnox at samba.org>
---
 source3/script/tests/test_net_conf.sh |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/source3/script/tests/test_net_conf.sh b/source3/script/tests/test_net_conf.sh
index c432c3e..a81b21e 100755
--- a/source3/script/tests/test_net_conf.sh
+++ b/source3/script/tests/test_net_conf.sh
@@ -424,7 +424,8 @@ test_conf_setparm_existing()
 
 test_conf_setparm_forbidden()
 {
-	FORBIDDEN_PARAMS="lock directory
+	FORBIDDEN_PARAMS="state directory
+lock directory
 lock dir
 config backend
 include"
-- 
1.7.9.5


From ec34c7af106a011c59624cc224f5ecc30f6189ab Mon Sep 17 00:00:00 2001
From: Michael Adam <obnox at samba.org>
Date: Sun, 22 Sep 2013 03:03:41 +0200
Subject: [PATCH 17/17] selftest: include "state directory" in invalid
 parameters in registry roundtrip test

Signed-off-by: Michael Adam <obnox at samba.org>
---
 .../script/tests/test_net_registry_roundtrip.sh    |    1 +
 1 file changed, 1 insertion(+)

diff --git a/source3/script/tests/test_net_registry_roundtrip.sh b/source3/script/tests/test_net_registry_roundtrip.sh
index 88e7297..6c2c090 100755
--- a/source3/script/tests/test_net_registry_roundtrip.sh
+++ b/source3/script/tests/test_net_registry_roundtrip.sh
@@ -38,6 +38,7 @@ failed=0
 # they are source4-only. Both reasons lead to failure when importing.
 #
 SED_INVALID_PARAMS="{
+s/state directory/;&/g
 s/lock directory/;&/g
 s/lock dir/;&/g
 s/modules dir/;&/g
-- 
1.7.9.5

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 215 bytes
Desc: Digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20130922/3b427b9c/attachment.pgp>


More information about the samba-technical mailing list