kill security=share and security=server

Jeremy Allison jra at samba.org
Wed Jan 26 16:55:23 MST 2011


On Thu, Jan 27, 2011 at 09:01:02AM +1000, Andrew Bartlett wrote:
> On Wed, 2011-01-26 at 14:05 -0800, Jeremy Allison wrote:
> > On Thu, Jan 27, 2011 at 07:50:21AM +1000, Andrew Bartlett wrote:
> > > 
> > > I fully support removing security=share over SMB2, and furthermore, I
> > > would like to see it marked as deprecated even on smb1 so we can
> > > eventually remove it.  
> > > 
> > > If we are trying not to break existing configurations, then we can have
> > > the deprecated parameter this force the max protocol=smb1.
> > > 
> > > There are other ways (map to guest etc) to get what almost all sane
> > > users of security=share does.  It is also not compatible (we make it
> > > almost work with kludges) with NTLMv2, which we are trying to move to. 
> > 
> > So right now in the code, for SMB2 if you have "security = share",
> > internally we convert this to:
> > 
> > security = user
> > map to guest = bad user
> > 
> > So we actually *have* gotten rid of "security = share"
> > internally in this case for all practical purposes,
> > we just don't error out the smb2 connection if someone
> > set "security = share" in their smb.conf.
> > 
> > Does this work for everyone ? Should we do the same
> > for SMB1 in 3.6.0 ? That would remove the actual code
> > complexity for "security = share" whilst still allowing
> > old smb.conf's to work.
> 
> I'm happy with this, as long as we also add the deprecation warning (so
> we don't keep a useless parameter forever).  I disagree with Chris that
> changing 'security=share -> map to guest = bad user' is that hard to
> explain (the default for security is already user). 

Ok, here's a patch to expunge SEC_SHARE from the world :-).

Internally maps inside loadparm.c.

Untested (but currently under test :-). Please comment !

Jeremy.
-------------- next part --------------
diff --git a/source3/auth/auth.c b/source3/auth/auth.c
index 790bfac..4567070 100644
--- a/source3/auth/auth.c
+++ b/source3/auth/auth.c
@@ -502,17 +502,6 @@ NTSTATUS make_auth_context_subsystem(TALLOC_CTX *mem_ctx,
 					talloc_tos(), "guest unix", NULL);
 			}
 			break;
-		case SEC_SHARE:
-			if (lp_encrypted_passwords()) {
-				DEBUG(5,("Making default auth method list for security=share, encrypt passwords = yes\n"));
-				auth_method_list = str_list_make_v3(
-					talloc_tos(), "guest sam", NULL);
-			} else {
-				DEBUG(5,("Making default auth method list for security=share, encrypt passwords = no\n"));
-				auth_method_list = str_list_make_v3(
-					talloc_tos(), "guest unix", NULL);
-			}
-			break;
 		case SEC_ADS:
 			DEBUG(5,("Making default auth method list for security=ADS\n"));
 			auth_method_list = str_list_make_v3(
diff --git a/source3/libnet/libnet_join.c b/source3/libnet/libnet_join.c
index 6120617..57ca7ea 100644
--- a/source3/libnet/libnet_join.c
+++ b/source3/libnet/libnet_join.c
@@ -1784,7 +1784,6 @@ static WERROR libnet_join_check_config(TALLOC_CTX *mem_ctx,
 		if (!valid_security) {
 			const char *sec = NULL;
 			switch (lp_security()) {
-			case SEC_SHARE: sec = "share"; break;
 			case SEC_USER:  sec = "user"; break;
 			case SEC_DOMAIN: sec = "domain"; break;
 			case SEC_ADS: sec = "ads"; break;
diff --git a/source3/param/loadparm.c b/source3/param/loadparm.c
index b45e045..38f068e 100644
--- a/source3/param/loadparm.c
+++ b/source3/param/loadparm.c
@@ -5791,7 +5791,21 @@ FN_GLOBAL_INTEGER(lp_deadtime, &Globals.deadtime)
 FN_GLOBAL_BOOL(lp_getwd_cache, &Globals.getwd_cache)
 FN_GLOBAL_INTEGER(lp_maxprotocol, &Globals.maxprotocol)
 FN_GLOBAL_INTEGER(lp_minprotocol, &Globals.minprotocol)
-FN_GLOBAL_INTEGER(lp_security, &Globals.security)
+FN_GLOBAL_INTEGER(_lp_security, &Globals.security)
+int lp_security(void)
+{
+	int ret = _lp_security();
+
+	if (ret == SEC_SHARE) {
+		DEBUG(2,("WARNING!!: \"security = share\" is deprecated for "
+			"SMB and SMB2 servers. Mapping to \"security = user\" and "
+			"\"map to guest = Bad User\"\n" ));
+			lp_do_parameter(-1, "security", "user");
+			lp_do_parameter(-1, "map to guest", "Bad User");
+		ret = _lp_security();
+	}
+	return ret;
+}
 FN_GLOBAL_LIST(lp_auth_methods, &Globals.AuthMethods)
 FN_GLOBAL_BOOL(lp_paranoid_server_security, &Globals.paranoid_server_security)
 FN_GLOBAL_INTEGER(lp_maxdisksize, &Globals.maxdisksize)
diff --git a/source3/param/loadparm_server_role.c b/source3/param/loadparm_server_role.c
index 3cc8f35..bfc1166 100644
--- a/source3/param/loadparm_server_role.c
+++ b/source3/param/loadparm_server_role.c
@@ -59,10 +59,6 @@ void set_server_role(void)
 	server_role = ROLE_STANDALONE;
 
 	switch (lp_security()) {
-		case SEC_SHARE:
-			if (lp_domain_logons())
-				DEBUG(0, ("Server's Role (logon server) conflicts with share-level security\n"));
-			break;
 		case SEC_SERVER:
 			if (lp_domain_logons())
 				DEBUG(0, ("Server's Role (logon server) conflicts with server-level security\n"));
diff --git a/source3/smbd/negprot.c b/source3/smbd/negprot.c
index 9cc34d8..8c6f818 100644
--- a/source3/smbd/negprot.c
+++ b/source3/smbd/negprot.c
@@ -293,7 +293,6 @@ static void reply_nt1(struct smb_request *req, uint16 choice)
 	   supports it and we can do encrypted passwords */
 
 	if (sconn->smb1.negprot.encrypted_passwords &&
-	    (lp_security() != SEC_SHARE) &&
 	    lp_use_spnego() &&
 	    (req->flags2 & FLAGS2_EXTENDED_SECURITY)) {
 		negotiate_spnego = True;
diff --git a/source3/smbd/password.c b/source3/smbd/password.c
index 9be2b3b..f5cad19 100644
--- a/source3/smbd/password.c
+++ b/source3/smbd/password.c
@@ -168,12 +168,6 @@ int register_initial_vuid(struct smbd_server_connection *sconn)
 {
 	user_struct *vuser;
 
-	/* Paranoia check. */
-	if(lp_security() == SEC_SHARE) {
-		smb_panic("register_initial_vuid: "
-			"Tried to register uid in security=share");
-	}
-
 	/* Limit allowed vuids to 16bits - VUID_OFFSET. */
 	if (sconn->smb1.sessions.num_validated_vuids >= 0xFFFF-VUID_OFFSET) {
 		return UID_FIELD_INVALID;
diff --git a/source3/smbd/process.c b/source3/smbd/process.c
index 12ea28a..6f0c850 100644
--- a/source3/smbd/process.c
+++ b/source3/smbd/process.c
@@ -1457,9 +1457,7 @@ static connection_struct *switch_message(uint8 type, struct smb_request *req, in
 
 	flags = smb_messages[type].flags;
 
-	/* In share mode security we must ignore the vuid. */
-	session_tag = (lp_security() == SEC_SHARE)
-		? UID_FIELD_INVALID : req->vuid;
+	session_tag = req->vuid;
 	conn = req->conn;
 
 	DEBUG(3,("switch message %s (pid %d) conn 0x%lx\n", smb_fn_name(type),
diff --git a/source3/smbd/reply.c b/source3/smbd/reply.c
index f7265e0..00356b7 100644
--- a/source3/smbd/reply.c
+++ b/source3/smbd/reply.c
@@ -571,13 +571,6 @@ void reply_special(struct smbd_server_connection *sconn, char *inbuf, size_t inb
 			break;
 		}
 
-		/* only add the client's machine name to the list
-		   of possibly valid usernames if we are operating
-		   in share mode security */
-		if (lp_security() == SEC_SHARE) {
-			add_session_user(sconn, get_remote_machine_name());
-		}
-
 		reload_services(sconn->msg_ctx, sconn->sock, True);
 		reopen_logs();
 
@@ -730,15 +723,7 @@ void reply_tcon_and_X(struct smb_request *req)
 
 	if (sconn->smb1.negprot.encrypted_passwords) {
 		password = data_blob_talloc(talloc_tos(), req->buf, passlen);
-		if (lp_security() == SEC_SHARE) {
-			/*
-			 * Security = share always has a pad byte
-			 * after the password.
-			 */
-			p = (const char *)req->buf + passlen + 1;
-		} else {
-			p = (const char *)req->buf + passlen;
-		}
+		p = (const char *)req->buf + passlen;
 	} else {
 		password = data_blob_talloc(talloc_tos(), req->buf, passlen+1);
 		/* Ensure correct termination */
@@ -2075,11 +2060,9 @@ void reply_ulogoffX(struct smb_request *req)
 			 req->vuid));
 	}
 
-	/* in user level security we are supposed to close any files
+	/* We are supposed to close any files
 		open by this user */
-	if ((vuser != NULL) && (lp_security() != SEC_SHARE)) {
-		file_close_user(sconn, req->vuid);
-	}
+	file_close_user(sconn, req->vuid);
 
 	invalidate_vuid(sconn, req->vuid);
 
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index a58f17c..d47bbb2 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -632,29 +632,7 @@ static NTSTATUS create_connection_server_info(struct smbd_server_connection *sco
 		return NT_STATUS_OK;
         }
 
-        if (lp_security() == SEC_SHARE) {
-
-                fstring user;
-		bool guest;
-
-                /* add the sharename as a possible user name if we
-                   are in share mode security */
-
-                add_session_user(sconn, lp_servicename(snum));
-
-                /* shall we let them in? */
-
-                if (!authorise_login(sconn, snum,user,password,&guest)) {
-                        DEBUG( 2, ( "Invalid username/password for [%s]\n",
-                                    lp_servicename(snum)) );
-			return NT_STATUS_WRONG_PASSWORD;
-                }
-
-		return make_serverinfo_from_username(mem_ctx, user, guest,
-						     presult);
-        }
-
-	DEBUG(0, ("invalid VUID (vuser) but not in security=share\n"));
+	DEBUG(0, ("invalid VUID (vuser)\n"));
 	return NT_STATUS_ACCESS_DENIED;
 }
 
@@ -707,7 +685,7 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
 		goto err_root_exit;
 	}
 
-	if ((lp_guest_only(snum)) || (lp_security() == SEC_SHARE)) {
+	if (lp_guest_only(snum)) {
 		conn->force_user = true;
 	}
 
@@ -1135,14 +1113,12 @@ connection_struct *make_connection(struct smbd_server_connection *sconn,
 		return NULL;
 	}
 
-	if(lp_security() != SEC_SHARE) {
-		vuser = get_valid_user_struct(sconn, vuid);
-		if (!vuser) {
-			DEBUG(1,("make_connection: refusing to connect with "
-				 "no session setup\n"));
-			*status = NT_STATUS_ACCESS_DENIED;
-			return NULL;
-		}
+	vuser = get_valid_user_struct(sconn, vuid);
+	if (!vuser) {
+		DEBUG(1,("make_connection: refusing to connect with "
+			 "no session setup\n"));
+		*status = NT_STATUS_ACCESS_DENIED;
+		return NULL;
 	}
 
 	/* Logic to try and connect to the correct [homes] share, preferably
@@ -1155,50 +1131,22 @@ connection_struct *make_connection(struct smbd_server_connection *sconn,
 	*/
 
 	if (strequal(service_in,HOMES_NAME)) {
-		if(lp_security() != SEC_SHARE) {
-			DATA_BLOB no_pw = data_blob_null;
-			if (vuser->homes_snum == -1) {
-				DEBUG(2, ("[homes] share not available for "
-					  "this user because it was not found "
-					  "or created at session setup "
-					  "time\n"));
-				*status = NT_STATUS_BAD_NETWORK_NAME;
-				return NULL;
-			}
-			DEBUG(5, ("making a connection to [homes] service "
-				  "created at session setup time\n"));
-			return make_connection_snum(sconn,
-						    vuser->homes_snum,
-						    vuser, no_pw, 
-						    dev, status);
-		} else {
-			/* Security = share. Try with
-			 * current_user_info.smb_name as the username.  */
-			if (*current_user_info.smb_name) {
-				char *unix_username = NULL;
-				(void)map_username(talloc_tos(),
-						current_user_info.smb_name,
-						&unix_username);
-				snum = find_service(talloc_tos(),
-						unix_username,
-						&unix_username);
-				if (!unix_username) {
-					*status = NT_STATUS_NO_MEMORY;
-				}
-				return NULL;
-			}
-			if (snum != -1) {
-				DEBUG(5, ("making a connection to 'homes' "
-					  "service %s based on "
-					  "security=share\n", service_in));
-				return make_connection_snum(sconn,
-							    snum, NULL,
-							    password,
-							    dev, status);
-			}
+		DATA_BLOB no_pw = data_blob_null;
+		if (vuser->homes_snum == -1) {
+			DEBUG(2, ("[homes] share not available for "
+				  "this user because it was not found "
+				  "or created at session setup "
+				  "time\n"));
+			*status = NT_STATUS_BAD_NETWORK_NAME;
+			return NULL;
 		}
-	} else if ((lp_security() != SEC_SHARE) && (vuser->homes_snum != -1)
-		   && strequal(service_in,
+		DEBUG(5, ("making a connection to [homes] service "
+			  "created at session setup time\n"));
+		return make_connection_snum(sconn,
+					    vuser->homes_snum,
+					    vuser, no_pw, 
+					    dev, status);
+	} else if ((vuser->homes_snum != -1) && strequal(service_in,
 			       lp_servicename(vuser->homes_snum))) {
 		DATA_BLOB no_pw = data_blob_null;
 		DEBUG(5, ("making a connection to 'homes' service [%s] "
diff --git a/source3/smbd/sesssetup.c b/source3/smbd/sesssetup.c
index 12d0460..f78f58e 100644
--- a/source3/smbd/sesssetup.c
+++ b/source3/smbd/sesssetup.c
@@ -1433,7 +1433,7 @@ void reply_sesssetup_and_X(struct smb_request *req)
 		if (doencrypt) {
 			lm_resp = data_blob(p, passlen1);
 			nt_resp = data_blob(p+passlen1, passlen2);
-		} else if (lp_security() != SEC_SHARE) {
+		} else {
 			/*
 			 * In share level we should ignore any passwords, so
  			 * only read them if we're not.
@@ -1545,27 +1545,6 @@ void reply_sesssetup_and_X(struct smb_request *req)
 
 	reload_services(sconn->msg_ctx, sconn->sock, True);
 
-	if (lp_security() == SEC_SHARE) {
-		char *sub_user_mapped = NULL;
-		/* In share level we should ignore any passwords */
-
-		data_blob_free(&lm_resp);
-		data_blob_free(&nt_resp);
-		data_blob_clear_free(&plaintext_password);
-
-		(void)map_username(talloc_tos(), sub_user, &sub_user_mapped);
-		if (!sub_user_mapped) {
-			reply_nterror(req, NT_STATUS_NO_MEMORY);
-			END_PROFILE(SMBsesssetupX);
-			return;
-		}
-		fstrcpy(sub_user, sub_user_mapped);
-		add_session_user(sconn, sub_user);
-		add_session_workgroup(sconn, domain);
-		/* Then force it to null for the benfit of the code below */
-		user = "";
-	}
-
 	if (!*user) {
 
 		nt_status = check_guest_password(&server_info);
@@ -1674,38 +1653,33 @@ void reply_sesssetup_and_X(struct smb_request *req)
 	/* register the name and uid as being validated, so further connections
 	   to a uid can get through without a password, on the same VC */
 
-	if (lp_security() == SEC_SHARE) {
-		sess_vuid = UID_FIELD_INVALID;
-		TALLOC_FREE(server_info);
-	} else {
-		/* Ignore the initial vuid. */
-		sess_vuid = register_initial_vuid(sconn);
-		if (sess_vuid == UID_FIELD_INVALID) {
-			data_blob_free(&nt_resp);
-			data_blob_free(&lm_resp);
-			reply_nterror(req, nt_status_squash(
-					      NT_STATUS_LOGON_FAILURE));
-			END_PROFILE(SMBsesssetupX);
-			return;
-		}
-		/* register_existing_vuid keeps the server info */
-		sess_vuid = register_existing_vuid(sconn, sess_vuid,
-					server_info,
-					nt_resp.data ? nt_resp : lm_resp,
-					sub_user);
-		if (sess_vuid == UID_FIELD_INVALID) {
-			data_blob_free(&nt_resp);
-			data_blob_free(&lm_resp);
-			reply_nterror(req, nt_status_squash(
-					      NT_STATUS_LOGON_FAILURE));
-			END_PROFILE(SMBsesssetupX);
-			return;
-		}
-
-		/* current_user_info is changed on new vuid */
-		reload_services(sconn->msg_ctx, sconn->sock, True);
+	/* Ignore the initial vuid. */
+	sess_vuid = register_initial_vuid(sconn);
+	if (sess_vuid == UID_FIELD_INVALID) {
+		data_blob_free(&nt_resp);
+		data_blob_free(&lm_resp);
+		reply_nterror(req, nt_status_squash(
+				      NT_STATUS_LOGON_FAILURE));
+		END_PROFILE(SMBsesssetupX);
+		return;
+	}
+	/* register_existing_vuid keeps the server info */
+	sess_vuid = register_existing_vuid(sconn, sess_vuid,
+				server_info,
+				nt_resp.data ? nt_resp : lm_resp,
+				sub_user);
+	if (sess_vuid == UID_FIELD_INVALID) {
+		data_blob_free(&nt_resp);
+		data_blob_free(&lm_resp);
+		reply_nterror(req, nt_status_squash(
+				      NT_STATUS_LOGON_FAILURE));
+		END_PROFILE(SMBsesssetupX);
+		return;
 	}
 
+	/* current_user_info is changed on new vuid */
+	reload_services(sconn->msg_ctx, sconn->sock, True);
+
 	data_blob_free(&nt_resp);
 	data_blob_free(&lm_resp);
 
diff --git a/source3/smbd/smb2_server.c b/source3/smbd/smb2_server.c
index fef9ded..fc2af47 100644
--- a/source3/smbd/smb2_server.c
+++ b/source3/smbd/smb2_server.c
@@ -2169,14 +2169,6 @@ void smbd_smb2_first_negprot(struct smbd_server_connection *sconn,
 	struct smbd_smb2_request *req = NULL;
 	struct tevent_req *subreq;
 
-	if (lp_security() == SEC_SHARE) {
-		DEBUG(2,("WARNING!!: \"security = share\" is deprecated for "
-			"SMB2 servers. Mapping to \"security = user\" and "
-			"\"map to guest = Bad User\"\n" ));
-		lp_do_parameter(-1, "security", "user");
-		lp_do_parameter(-1, "map to guest", "Bad User");
-	}
-
 	DEBUG(10,("smbd_smb2_first_negprot: packet length %u\n",
 		 (unsigned int)size));
 
diff --git a/source3/smbd/uid.c b/source3/smbd/uid.c
index b573a6c..7fd512a 100644
--- a/source3/smbd/uid.c
+++ b/source3/smbd/uid.c
@@ -244,19 +244,7 @@ bool change_to_user(connection_struct *conn, uint16 vuid)
 
 	vuser = get_valid_user_struct(conn->sconn, vuid);
 
-	/*
-	 * We need a separate check in security=share mode due to vuid
-	 * always being UID_FIELD_INVALID. If we don't do this then
-	 * in share mode security we are *always* changing uid's between
-	 * SMB's - this hurts performance - Badly.
-	 */
-
-	if((lp_security() == SEC_SHARE) && (current_user.conn == conn) &&
-	   (current_user.ut.uid == conn->server_info->utok.uid)) {
-		DEBUG(4,("change_to_user: Skipping user change - already "
-			 "user\n"));
-		return(True);
-	} else if ((current_user.conn == conn) && 
+	if ((current_user.conn == conn) && 
 		   (vuser != NULL) && (current_user.vuid == vuid) &&
 		   (current_user.ut.uid == vuser->server_info->utok.uid)) {
 		DEBUG(4,("change_to_user: Skipping user change - already "
diff --git a/source3/utils/status.c b/source3/utils/status.c
index 1ad2e9b..41c6a85 100644
--- a/source3/utils/status.c
+++ b/source3/utils/status.c
@@ -411,10 +411,6 @@ static int traverse_sessionid(const char *key, struct sessionid *session,
 		d_printf("\nSamba version %s\n",samba_version_string());
 		d_printf("PID     Username      Group         Machine                        \n");
 		d_printf("-------------------------------------------------------------------\n");
-		if (lp_security() == SEC_SHARE) {
-			d_printf(" <processes do not show up in "
-				 "anonymous mode>\n");
-		}
 
 		sessionid_traverse_read(traverse_sessionid, NULL);
 


More information about the samba-technical mailing list