Added per-share security tdb. Tidied up many slprintfs (need -1 on length).

Jeremy.
(This used to be commit d786191bc116c17a3f53a1c272d969942b7e5d25)

3 files changed, 239 insertions, 98 deletions

diff --git a/source3/include/proto.h b/source3/include/proto.h
index dc2e17f341..4d42664f25 100644
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -3764,6 +3764,8 @@ BOOL api_srvsvc_rpc(pipes_struct *p);
 
 /*The following definitions come from  rpc_server/srv_srvsvc_nt.c  */
 
+BOOL share_info_db_init(void);
+SEC_DESC *get_share_security( TALLOC_CTX *ctx, int snum, size_t *psize);
 uint32 _srv_net_srv_get_info(pipes_struct *p, SRV_Q_NET_SRV_GET_INFO *q_u, SRV_R_NET_SRV_GET_INFO *r_u);
 uint32 _srv_net_file_enum(pipes_struct *p, SRV_Q_NET_FILE_ENUM *q_u, SRV_R_NET_FILE_ENUM *r_u);
 uint32 _srv_net_conn_enum(pipes_struct *p, SRV_Q_NET_CONN_ENUM *q_u, SRV_R_NET_CONN_ENUM *r_u);
diff --git a/source3/printing/nt_printing.c b/source3/printing/nt_printing.c
index ccef9bf656..0aa3b96cdd 100644
--- a/source3/printing/nt_printing.c
+++ b/source3/printing/nt_printing.c
@@ -32,7 +32,7 @@ static TDB_CONTEXT *tdb; /* used for driver files */
 #define DRIVERS_PREFIX "DRIVERS/"
 #define PRINTERS_PREFIX "PRINTERS/"
 
-#define DATABASE_VERSION 1
+#define NTDRIVERS_DATABASE_VERSION 1
 
 /* Map generic permissions to printer object specific permissions */
 
@@ -182,7 +182,8 @@ BOOL nt_printing_init(void)
 	if (tdb && local_pid == sys_getpid()) return True;
 	tdb = tdb_open(lock_path("ntdrivers.tdb"), 0, 0, O_RDWR|O_CREAT, 0600);
 	if (!tdb) {
-		DEBUG(0,("Failed to open nt drivers database\n"));
+		DEBUG(0,("Failed to open nt drivers database %s (%s)\n",
+			lock_path("ntdrivers.tdb"), strerror(errno) ));
 		return False;
 	}
 
@@ -190,9 +191,9 @@ BOOL nt_printing_init(void)
 
 	/* handle a Samba upgrade */
 	tdb_lock_bystring(tdb, vstring);
-	if (tdb_fetch_int(tdb, vstring) != DATABASE_VERSION) {
+	if (tdb_fetch_int(tdb, vstring) != NTDRIVERS_DATABASE_VERSION) {
 		tdb_traverse(tdb, (tdb_traverse_func)tdb_delete, NULL);
-		tdb_store_int(tdb, vstring, DATABASE_VERSION);
+		tdb_store_int(tdb, vstring, NTDRIVERS_DATABASE_VERSION);
 	}
 	tdb_unlock_bystring(tdb, vstring);
 
@@ -286,7 +287,7 @@ int write_ntforms(nt_forms_struct **list, int number)
 			       (*list)[i].left, (*list)[i].top, (*list)[i].right,
 			       (*list)[i].bottom);
 		if (len > sizeof(buf)) break;
-		slprintf(key, sizeof(key), "%s%s", FORMS_PREFIX, (*list)[i].name);
+		slprintf(key, sizeof(key)-1, "%s%s", FORMS_PREFIX, (*list)[i].name);
         dos_to_unix(key, True);            /* Convert key to unix-codepage */
 		kbuf.dsize = strlen(key)+1;
 		kbuf.dptr = key;
@@ -369,7 +370,7 @@ BOOL delete_a_form(nt_forms_struct **list, UNISTR2 *del_name, int *count, uint32
 		return False;
 	}
 
-	slprintf(key, sizeof(key), "%s%s", FORMS_PREFIX, (*list)[n].name);
+	slprintf(key, sizeof(key)-1, "%s%s", FORMS_PREFIX, (*list)[n].name);
 	dos_to_unix(key, True);                /* Convert key to unix-codepage */
 	kbuf.dsize = strlen(key)+1;
 	kbuf.dptr = key;
@@ -422,7 +423,7 @@ int get_ntdrivers(fstring **list, char *architecture, uint32 version)
 	TDB_DATA kbuf, newkey;
 
 	get_short_archi(short_archi, architecture);
-	slprintf(key, sizeof(key), "%s%s/%d/", DRIVERS_PREFIX, short_archi, version);
+	slprintf(key, sizeof(key)-1, "%s%s/%d/", DRIVERS_PREFIX, short_archi, version);
 
 	for (kbuf = tdb_firstkey(tdb);
 	     kbuf.dptr;
@@ -549,7 +550,7 @@ static uint32 get_correct_cversion(fstring architecture, fstring driverpath_in,
 
 	/* Open the driver file (Portable Executable format) and determine the
 	 * deriver the cversion. */
-	slprintf(driverpath, sizeof(driverpath), "%s/%s", architecture, driverpath_in);
+	slprintf(driverpath, sizeof(driverpath)-1, "%s/%s", architecture, driverpath_in);
 
 	unix_convert(driverpath,conn,NULL,&bad_path,&st);
 
@@ -1282,7 +1283,7 @@ BOOL move_driver_to_download_area(NT_PRINTER_DRIVER_INFO_LEVEL driver_abstract,
 	 * under the architecture directory.
 	 */
 	DEBUG(5,("Creating first directory\n"));
-	slprintf(new_dir, sizeof(new_dir), "%s/%d", architecture, driver->cversion);
+	slprintf(new_dir, sizeof(new_dir)-1, "%s/%d", architecture, driver->cversion);
 	mkdir_internal(conn, inbuf, outbuf, new_dir);
 
 	/* For each driver file, archi\filexxx.yyy, if there is a duplicate file
@@ -1305,8 +1306,8 @@ BOOL move_driver_to_download_area(NT_PRINTER_DRIVER_INFO_LEVEL driver_abstract,
 	DEBUG(5,("Moving files now !\n"));
 
 	if (driver->driverpath && strlen(driver->driverpath)) {
-		slprintf(new_name, sizeof(new_name), "%s/%s", architecture, driver->driverpath);	
-		slprintf(old_name, sizeof(old_name), "%s/%s", new_dir, driver->driverpath);	
+		slprintf(new_name, sizeof(new_name)-1, "%s/%s", architecture, driver->driverpath);	
+		slprintf(old_name, sizeof(old_name)-1, "%s/%s", new_dir, driver->driverpath);	
 		if (ver != -1 && (ver=file_version_is_newer(conn, new_name, old_name)) > 0) {
 			if (rename_internals(conn, inbuf, outbuf, new_name, old_name, True) != 0) {
 				DEBUG(0,("move_driver_to_download_area: Unable to rename [%s] to [%s]\n",
@@ -1322,8 +1323,8 @@ BOOL move_driver_to_download_area(NT_PRINTER_DRIVER_INFO_LEVEL driver_abstract,
 
 	if (driver->datafile && strlen(driver->datafile)) {
 		if (!strequal(driver->datafile, driver->driverpath)) {
-			slprintf(new_name, sizeof(new_name), "%s/%s", architecture, driver->datafile);	
-			slprintf(old_name, sizeof(old_name), "%s/%s", new_dir, driver->datafile);	
+			slprintf(new_name, sizeof(new_name)-1, "%s/%s", architecture, driver->datafile);	
+			slprintf(old_name, sizeof(old_name)-1, "%s/%s", new_dir, driver->datafile);	
 			if (ver != -1 && (ver=file_version_is_newer(conn, new_name, old_name)) > 0) {
 				if (rename_internals(conn, inbuf, outbuf, new_name, old_name, True) != 0) {
 					DEBUG(0,("move_driver_to_download_area: Unable to rename [%s] to [%s]\n",
@@ -1341,8 +1342,8 @@ BOOL move_driver_to_download_area(NT_PRINTER_DRIVER_INFO_LEVEL driver_abstract,
 	if (driver->configfile && strlen(driver->configfile)) {
 		if (!strequal(driver->configfile, driver->driverpath) &&
 			!strequal(driver->configfile, driver->datafile)) {
-			slprintf(new_name, sizeof(new_name), "%s/%s", architecture, driver->configfile);	
-			slprintf(old_name, sizeof(old_name), "%s/%s", new_dir, driver->configfile);	
+			slprintf(new_name, sizeof(new_name)-1, "%s/%s", architecture, driver->configfile);	
+			slprintf(old_name, sizeof(old_name)-1, "%s/%s", new_dir, driver->configfile);	
 			if (ver != -1 && (ver=file_version_is_newer(conn, new_name, old_name)) > 0) {
 				if (rename_internals(conn, inbuf, outbuf, new_name, old_name, True) != 0) {
 					DEBUG(0,("move_driver_to_download_area: Unable to rename [%s] to [%s]\n",
@@ -1361,8 +1362,8 @@ BOOL move_driver_to_download_area(NT_PRINTER_DRIVER_INFO_LEVEL driver_abstract,
 		if (!strequal(driver->helpfile, driver->driverpath) &&
 			!strequal(driver->helpfile, driver->datafile) &&
 			!strequal(driver->helpfile, driver->configfile)) {
-			slprintf(new_name, sizeof(new_name), "%s/%s", architecture, driver->helpfile);	
-			slprintf(old_name, sizeof(old_name), "%s/%s", new_dir, driver->helpfile);	
+			slprintf(new_name, sizeof(new_name)-1, "%s/%s", architecture, driver->helpfile);	
+			slprintf(old_name, sizeof(old_name)-1, "%s/%s", new_dir, driver->helpfile);	
 			if (ver != -1 && (ver=file_version_is_newer(conn, new_name, old_name)) > 0) {
 				if (rename_internals(conn, inbuf, outbuf, new_name, old_name, True) != 0) {
 					DEBUG(0,("move_driver_to_download_area: Unable to rename [%s] to [%s]\n",
@@ -1390,8 +1391,8 @@ BOOL move_driver_to_download_area(NT_PRINTER_DRIVER_INFO_LEVEL driver_abstract,
 					}
 				}
 
-				slprintf(new_name, sizeof(new_name), "%s/%s", architecture, driver->dependentfiles[i]);	
-				slprintf(old_name, sizeof(old_name), "%s/%s", new_dir, driver->dependentfiles[i]);	
+				slprintf(new_name, sizeof(new_name)-1, "%s/%s", architecture, driver->dependentfiles[i]);	
+				slprintf(old_name, sizeof(old_name)-1, "%s/%s", new_dir, driver->dependentfiles[i]);	
 				if (ver != -1 && (ver=file_version_is_newer(conn, new_name, old_name)) > 0) {
 					if (rename_internals(conn, inbuf, outbuf, new_name, old_name, True) != 0) {
 						DEBUG(0,("move_driver_to_download_area: Unable to rename [%s] to [%s]\n",
@@ -1434,7 +1435,7 @@ static uint32 add_a_printer_driver_3(NT_PRINTER_DRIVER_INFO_LEVEL_3 *driver)
 	 * It does make sense to NOT store the server's name in the printer TDB.
 	 */
 
-	slprintf(directory, sizeof(directory), "\\print$\\%s\\%d\\", architecture, driver->cversion);
+	slprintf(directory, sizeof(directory)-1, "\\print$\\%s\\%d\\", architecture, driver->cversion);
 
     /* .inf files do not always list a file for each of the four standard files. 
      * Don't prepend a path to a null filename, or client claims:
@@ -1444,32 +1445,32 @@ static uint32 add_a_printer_driver_3(NT_PRINTER_DRIVER_INFO_LEVEL_3 *driver)
      */
 	if (strlen(driver->driverpath)) {
     	fstrcpy(temp_name, driver->driverpath);
-    	slprintf(driver->driverpath, sizeof(driver->driverpath), "%s%s", directory, temp_name);
+    	slprintf(driver->driverpath, sizeof(driver->driverpath)-1, "%s%s", directory, temp_name);
     }
 
 	if (strlen(driver->datafile)) {
     	fstrcpy(temp_name, driver->datafile);
-    	slprintf(driver->datafile, sizeof(driver->datafile), "%s%s", directory, temp_name);
+    	slprintf(driver->datafile, sizeof(driver->datafile)-1, "%s%s", directory, temp_name);
     }
 
 	if (strlen(driver->configfile)) {
     	fstrcpy(temp_name, driver->configfile);
-    	slprintf(driver->configfile, sizeof(driver->configfile), "%s%s", directory, temp_name);
+    	slprintf(driver->configfile, sizeof(driver->configfile)-1, "%s%s", directory, temp_name);
     }
 
 	if (strlen(driver->helpfile)) {
     	fstrcpy(temp_name, driver->helpfile);
-    	slprintf(driver->helpfile, sizeof(driver->helpfile), "%s%s", directory, temp_name);
+    	slprintf(driver->helpfile, sizeof(driver->helpfile)-1, "%s%s", directory, temp_name);
     }
 
 	if (driver->dependentfiles) {
 		for (i=0; *driver->dependentfiles[i]; i++) {
             fstrcpy(temp_name, driver->dependentfiles[i]);
-            slprintf(driver->dependentfiles[i], sizeof(driver->dependentfiles[i]), "%s%s", directory, temp_name);
+            slprintf(driver->dependentfiles[i], sizeof(driver->dependentfiles[i])-1, "%s%s", directory, temp_name);
 		}
 	}
 
-	slprintf(key, sizeof(key), "%s%s/%d/%s", DRIVERS_PREFIX, architecture, driver->cversion, driver->name);
+	slprintf(key, sizeof(key)-1, "%s%s/%d/%s", DRIVERS_PREFIX, architecture, driver->cversion, driver->name);
 	dos_to_unix(key, True);                /* Convert key to unix-codepage */
 
 	DEBUG(5,("add_a_printer_driver_3: Adding driver with key %s\n", key ));
@@ -1584,7 +1585,7 @@ static uint32 get_a_printer_driver_3(NT_PRINTER_DRIVER_INFO_LEVEL_3 **info_ptr,
 
 	DEBUG(8,("get_a_printer_driver_3: [%s%s/%d/%s]\n", DRIVERS_PREFIX, architecture, version, in_prt));
 
-	slprintf(key, sizeof(key), "%s%s/%d/%s", DRIVERS_PREFIX, architecture, version, in_prt);
+	slprintf(key, sizeof(key)-1, "%s%s/%d/%s", DRIVERS_PREFIX, architecture, version, in_prt);
 
 	kbuf.dptr = key;
 	kbuf.dsize = strlen(key)+1;
@@ -1644,7 +1645,7 @@ uint32 get_a_printer_driver_9x_compatible(pstring line, fstring model)
 	int i;
 	line[0] = '\0';
 
-	slprintf(key, sizeof(key), "%s%s/%d/%s", DRIVERS_PREFIX, "WIN40", 0, model);
+	slprintf(key, sizeof(key)-1, "%s%s/%d/%s", DRIVERS_PREFIX, "WIN40", 0, model);
 	DEBUG(10,("driver key: [%s]\n", key));
 	
 	kbuf.dptr = key;
@@ -1834,7 +1835,7 @@ uint32 del_a_printer(char *sharename)
 	pstring key;
 	TDB_DATA kbuf;
 
-	slprintf(key, sizeof(key), "%s%s", PRINTERS_PREFIX, sharename);
+	slprintf(key, sizeof(key)-1, "%s%s", PRINTERS_PREFIX, sharename);
 	dos_to_unix(key, True);                /* Convert key to unix-codepage */
 
 	kbuf.dptr=key;
@@ -1921,7 +1922,7 @@ static uint32 update_a_printer_2(NT_PRINTER_INFO_LEVEL_2 *info)
 	}
 	
 
-	slprintf(key, sizeof(key), "%s%s", PRINTERS_PREFIX, info->sharename);
+	slprintf(key, sizeof(key)-1, "%s%s", PRINTERS_PREFIX, info->sharename);
 	dos_to_unix(key, True);                /* Convert key to unix-codepage */
 
 	kbuf.dptr = key;
@@ -2364,8 +2365,8 @@ static uint32 get_a_printer_2_default(NT_PRINTER_INFO_LEVEL_2 **info_ptr, fstrin
 
 	snum = lp_servicenumber(sharename);
 
-	slprintf(info.servername, sizeof(info.servername), "\\\\%s", global_myname);
-	slprintf(info.printername, sizeof(info.printername), "\\\\%s\\%s", 
+	slprintf(info.servername, sizeof(info.servername)-1, "\\\\%s", global_myname);
+	slprintf(info.printername, sizeof(info.printername)-1, "\\\\%s\\%s", 
 		 global_myname, sharename);
 	fstrcpy(info.sharename, sharename);
 	fstrcpy(info.portname, SAMBA_PRINTER_PORT_NAME);
@@ -2437,7 +2438,7 @@ static uint32 get_a_printer_2(NT_PRINTER_INFO_LEVEL_2 **info_ptr, fstring sharen
 		
 	ZERO_STRUCT(info);
 
-	slprintf(key, sizeof(key), "%s%s", PRINTERS_PREFIX, sharename);
+	slprintf(key, sizeof(key)-1, "%s%s", PRINTERS_PREFIX, sharename);
 	dos_to_unix(key, True);                /* Convert key to unix-codepage */
 
 	kbuf.dptr = key;
@@ -2475,8 +2476,8 @@ static uint32 get_a_printer_2(NT_PRINTER_INFO_LEVEL_2 **info_ptr, fstring sharen
 	info.attributes |= (PRINTER_ATTRIBUTE_SHARED|PRINTER_ATTRIBUTE_RAW_ONLY);
 
 	/* Restore the stripped strings. */
-	slprintf(info.servername, sizeof(info.servername), "\\\\%s", global_myname);
-	slprintf(printername, sizeof(printername), "\\\\%s\\%s", global_myname,
+	slprintf(info.servername, sizeof(info.servername)-1, "\\\\%s", global_myname);
+	slprintf(printername, sizeof(printername)-1, "\\\\%s\\%s", global_myname,
 			info.printername);
 	fstrcpy(info.printername, printername);
 
@@ -2982,7 +2983,7 @@ uint32 nt_printing_setsec(char *printername, SEC_DESC_BUF *secdesc_ctr)
 		goto out;
 	}
 
-	slprintf(key, sizeof(key), "SECDESC/%s", printername);
+	slprintf(key, sizeof(key)-1, "SECDESC/%s", printername);
 
 	if (tdb_prs_store(tdb, key, &ps)==0) {
 		status = 0;
@@ -2991,7 +2992,7 @@ uint32 nt_printing_setsec(char *printername, SEC_DESC_BUF *secdesc_ctr)
 		status = ERROR_INVALID_FUNCTION;
 	}
 
-	/* Free mallocated memory */
+	/* Free malloc'ed memory */
 
  out:
 
@@ -3090,7 +3091,7 @@ BOOL nt_printing_getsec(TALLOC_CTX *ctx, char *printername, SEC_DESC_BUF **secde
 
 	/* Fetch security descriptor from tdb */
 
-	slprintf(key, sizeof(key), "SECDESC/%s", printername);
+	slprintf(key, sizeof(key)-1, "SECDESC/%s", printername);
 
 	if (tdb_prs_fetch(tdb, key, &ps, ctx)!=0 ||
 	    !sec_io_desc_buf("nt_printing_getsec", secdesc_ctr, &ps, 1)) {
diff --git a/source3/rpc_server/srv_srvsvc_nt.c b/source3/rpc_server/srv_srvsvc_nt.c
index 1bff054aa4..5c1c16c02a 100644
--- a/source3/rpc_server/srv_srvsvc_nt.c
+++ b/source3/rpc_server/srv_srvsvc_nt.c
@@ -95,10 +95,43 @@ static void init_srv_share_info_2(SRV_SHARE_INFO_2 *sh2, int snum)
 }
 
 /*******************************************************************
- Fake up a Everyone, full access for now.
+ Create the share security tdb.
  ********************************************************************/
 
-static SEC_DESC *get_share_security( TALLOC_CTX *ctx, int snum, size_t *psize)
+static TDB_CONTEXT *share_tdb; /* used for share security descriptors */
+#define SHARE_DATABASE_VERSION 1
+
+BOOL share_info_db_init(void)
+{
+    static pid_t local_pid;
+    char *vstring = "INFO/version";
+ 
+    if (share_tdb && local_pid == sys_getpid()) return True;
+    share_tdb = tdb_open(lock_path("share_info.tdb"), 0, 0, O_RDWR|O_CREAT, 0600);
+    if (!share_tdb) {
+        DEBUG(0,("Failed to open share info database %s (%s)\n",
+				lock_path("share_info.tdb"), strerror(errno) ));
+        return False;
+    }
+ 
+    local_pid = sys_getpid();
+ 
+    /* handle a Samba upgrade */
+    tdb_lock_bystring(share_tdb, vstring);
+    if (tdb_fetch_int(share_tdb, vstring) != SHARE_DATABASE_VERSION) {
+        tdb_traverse(share_tdb, (tdb_traverse_func)tdb_delete, NULL);
+        tdb_store_int(share_tdb, vstring, SHARE_DATABASE_VERSION);
+    }
+    tdb_unlock_bystring(share_tdb, vstring);
+ 
+    return True;
+}
+
+/*******************************************************************
+ Fake up a Everyone, full access as a default.
+ ********************************************************************/
+
+static SEC_DESC *get_share_security_default( TALLOC_CTX *ctx, int snum, size_t *psize)
 {
 	extern DOM_SID global_sid_World;
 	SEC_ACCESS sa;
@@ -122,6 +155,94 @@ static SEC_DESC *get_share_security( TALLOC_CTX *ctx, int snum, size_t *psize)
 }
 
 /*******************************************************************
+ Pull a security descriptor from the share tdb.
+ ********************************************************************/
+
+SEC_DESC *get_share_security( TALLOC_CTX *ctx, int snum, size_t *psize)
+{
+	prs_struct ps;
+	fstring key;
+	SEC_DESC *psd;
+
+	/* Fetch security descriptor from tdb */
+ 
+	slprintf(key, sizeof(key)-1, "SECDESC/%s", lp_servicename(snum));
+ 
+    if (tdb_prs_fetch(share_tdb, key, &ps, ctx)!=0 ||
+        !sec_io_desc("get_share_security", &psd, &ps, 1)) {
+ 
+        DEBUG(4,("get_share_security: using default secdesc for %s\n", lp_servicename(snum) ));
+ 
+        return get_share_security_default(ctx, snum, psize);
+    }
+
+	prs_mem_free(&ps);
+	return psd;
+}
+
+/*******************************************************************
+ Store a security descriptor in the share db.
+ ********************************************************************/
+
+static BOOL set_share_security(TALLOC_CTX *ctx, int snum, SEC_DESC *psd)
+{
+	prs_struct ps;
+	TALLOC_CTX *mem_ctx = NULL;
+	fstring key;
+	BOOL ret = False;
+
+	mem_ctx = talloc_init();
+	if (mem_ctx == NULL)
+		return False;
+
+	prs_init(&ps, (uint32)sec_desc_size(psd), mem_ctx, MARSHALL);
+ 
+    if (!sec_io_desc("nt_printing_setsec", &psd, &ps, 1)) {
+        goto out;
+    }
+ 
+	slprintf(key, sizeof(key)-1, "SECDESC/%s", lp_servicename(snum));
+ 
+    if (tdb_prs_store(share_tdb, key, &ps)==0) {
+        ret = True;
+        DEBUG(5,("set_share_security: stored secdesc for %s\n", lp_servicename(snum) ));
+    } else {
+        DEBUG(1,("set_share_security: Failed to store secdesc for %s\n", lp_servicename(snum) ));
+    }
+
+    /* Free malloc'ed memory */
+ 
+ out:
+ 
+    prs_mem_free(&ps);
+    if (mem_ctx)
+        talloc_destroy(mem_ctx);
+    return ret;
+}
+
+/*******************************************************************
+ Delete a security descriptor.
+********************************************************************/
+
+static BOOL delete_share_security(int snum)
+{
+	TDB_DATA kbuf;
+	fstring key;
+
+	slprintf(key, sizeof(key)-1, "SECDESC/%s", lp_servicename(snum));
+	kbuf.dptr = key;
+	kbuf.dsize = strlen(key)+1;
+
+	if (tdb_delete(share_tdb, kbuf) != 0) {
+		DEBUG(0,("delete_share_security: Failed to delete entry for share %s\n",
+				lp_servicename(snum) ));
+		return False;
+	}
+
+	return True;
+}
+
+/*******************************************************************
  Fill in a share info level 502 structure.
  ********************************************************************/
 
@@ -942,62 +1063,6 @@ uint32 _srv_net_share_get_info(pipes_struct *p, SRV_Q_NET_SHARE_GET_INFO *q_u, S
 }
 
 /*******************************************************************
- Net share set info.
-********************************************************************/
-
-uint32 _srv_net_share_set_info(pipes_struct *p, SRV_Q_NET_SHARE_SET_INFO *q_u, SRV_R_NET_SHARE_SET_INFO *r_u)
-{
-	fstring share_name;
-	uint32 status = NT_STATUS_NOPROBLEMO;
-	int snum;
-#if 0
-	fstring servicename;
-	fstring comment;
-	pstring pathname;
-#endif
-
-	DEBUG(5,("_srv_net_share_set_info: %d\n", __LINE__));
-
-	unistr2_to_ascii(share_name, &q_u->uni_share_name, sizeof(share_name));
-
-	r_u->switch_value = 0;
-
-	snum = find_service(share_name);
-
-	/* For now we only handle setting the security descriptor. JRA. */
-
-	if (snum >= 0) {
-		switch (q_u->info_level) {
-		case 1:
-			status = ERROR_ACCESS_DENIED;
-			break;
-		case 2:
-			status = ERROR_ACCESS_DENIED;
-			break;
-		case 502:
-			/* we set sd's here. FIXME. JRA */
-			status = ERROR_ACCESS_DENIED;
-			break;
-		case 1005:
-			status = ERROR_ACCESS_DENIED;
-			break;
-		default:
-			DEBUG(5,("_srv_net_share_set_info: unsupported switch value %d\n", q_u->info_level));
-			status = NT_STATUS_INVALID_INFO_CLASS;
-			break;
-		}
-	} else {
-		status = NT_STATUS_BAD_NETWORK_NAME;
-	}
-
-	r_u->status = status;
-
-	DEBUG(5,("_srv_net_share_set_info: %d\n", __LINE__));
-
-	return r_u->status;
-}
-
-/*******************************************************************
  Check a given DOS pathname is valid for a share.
 ********************************************************************/
 
@@ -1041,6 +1106,68 @@ static char *valid_share_pathname(char *dos_pathname)
 }
 
 /*******************************************************************
+ Net share set info. Modify share details.
+********************************************************************/
+
+uint32 _srv_net_share_set_info(pipes_struct *p, SRV_Q_NET_SHARE_SET_INFO *q_u, SRV_R_NET_SHARE_SET_INFO *r_u)
+{
+	struct current_user user;
+	pstring command;
+	fstring share_name;
+	fstring comment;
+	pstring pathname;
+	int type;
+	int snum;
+	int ret;
+	char *ptr;
+	BOOL read_only;
+
+	DEBUG(5,("_srv_net_share_set_info: %d\n", __LINE__));
+
+	unistr2_to_ascii(share_name, &q_u->uni_share_name, sizeof(share_name));
+
+	r_u->switch_value = 0;
+
+	snum = find_service(share_name);
+
+	/* Does this share exist ? */
+	if (snum < 0)
+		return NT_STATUS_BAD_NETWORK_NAME;
+
+	get_current_user(&user,p);
+
+	if (user.uid != 0)
+		return ERROR_ACCESS_DENIED;
+
+	if (!lp_change_share_cmd())
+		return ERROR_ACCESS_DENIED;
+
+	switch (q_u->info_level) {
+	case 1:
+		return ERROR_ACCESS_DENIED;
+		break;
+	case 2:
+		return ERROR_ACCESS_DENIED;
+		break;
+	case 502:
+		/* we set sd's here. FIXME. JRA */
+		return ERROR_ACCESS_DENIED;
+		break;
+	case 1005:
+		return ERROR_ACCESS_DENIED;
+		break;
+	default:
+		DEBUG(5,("_srv_net_share_set_info: unsupported switch value %d\n", q_u->info_level));
+		return NT_STATUS_INVALID_INFO_CLASS;
+		break;
+	}
+
+	DEBUG(5,("_srv_net_share_set_info: %d\n", __LINE__));
+
+	return NT_STATUS_NOPROBLEMO;
+}
+
+/*******************************************************************
  Net share add. Call 'add_share_command "sharename" "pathname" "comment"'
 ********************************************************************/
 
@@ -1055,6 +1182,7 @@ uint32 _srv_net_share_add(pipes_struct *p, SRV_Q_NET_SHARE_ADD *q_u, SRV_R_NET_S
 	int snum;
 	int ret;
 	char *ptr;
+	BOOL read_only = False;
 
 	DEBUG(5,("_srv_net_share_add: %d\n", __LINE__));
 
@@ -1077,6 +1205,7 @@ uint32 _srv_net_share_add(pipes_struct *p, SRV_Q_NET_SHARE_ADD *q_u, SRV_R_NET_S
 		unistr2_to_ascii(comment, &q_u->info.share.info2.info_2_str.uni_remark, sizeof(share_name));
 		unistr2_to_ascii(pathname, &q_u->info.share.info2.info_2_str.uni_path, sizeof(share_name));
 		type = q_u->info.share.info2.info_2.type;
+		read_only = False; /* No SD means "Everyone full access. */
 		break;
 	case 502:
 		/* we set sd's here. FIXME. JRA */
@@ -1107,8 +1236,14 @@ uint32 _srv_net_share_add(pipes_struct *p, SRV_Q_NET_SHARE_ADD *q_u, SRV_R_NET_S
 	if (!(ptr = valid_share_pathname( pathname )))
 		return ERRbadpath;
 
-	slprintf(command, sizeof(command)-1, "%s \"%s\" \"%s\" \"%s\"",
-			lp_add_share_cmd(), share_name, ptr, comment );
+	/* Ensure share name, pathname and comment don't contain '"' characters. */
+	string_replace(share_name, '"', ' ');
+	string_replace(ptr, '"', ' ');
+	string_replace(comment, '"', ' ');
+
+	slprintf(command, sizeof(command)-1, "%s \"%s\" \"%s\" \"%s\" \"%s\"",
+			lp_add_share_cmd(), share_name, ptr, comment,
+			read_only ? "read only = yes" : "read only = no" );
 	dos_to_unix(command, True);  /* Convert to unix-codepage */
 
 	DEBUG(10,("_srv_net_share_add: Running [%s]\n", command ));
@@ -1170,6 +1305,9 @@ uint32 _srv_net_share_del(pipes_struct *p, SRV_Q_NET_SHARE_DEL *q_u, SRV_R_NET_S
 		return ERROR_ACCESS_DENIED;
 	}
 
+	/* Delete the SD in the database. */
+	delete_share_security(snum);
+
 	/* Send SIGHUP to process group. */
 	kill(0, SIGHUP);