[SCM] Samba Shared Repository - branch master updated
Jeremy Allison
jra at samba.org
Fri Feb 12 23:55:50 MST 2010
The branch, master has been updated
via d46d771... Simplify the logic in make_connection_snum(), and make it match Windows behavior.
from 10e54fb... Fix warning messages on compile in g_lock.c Volker & Michael please check.
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=master
- Log -----------------------------------------------------------------
commit d46d7717c7bdc1b404ff53d7831ed00d556a940f
Author: Jeremy Allison <jra at samba.org>
Date: Fri Feb 12 22:45:37 2010 -0800
Simplify the logic in make_connection_snum(), and make it match Windows behavior.
Cause all exit paths to go through one place, where all cleanup is
done. change_to_root_user() for pathname operations that should succeed if
the path exists, even if the connecting user has no access.
For example, a share can now be defined with a path of /root/only/access
(where /root/only/access is a directory path with all components only
accessible to root e.g. root owned, permissions 700 on every component).
Non-root users will now correctly connect, but get ACCESS_DENIED on
all activities (which matches Windows behavior). Previously, non-root
users would get NT_STATUS_BAD_NETWORK_NAME on doing a TConX to this
share, even though it's a perfectly valid share path (just not accessible
to them).
This change was inspired by the research I did for bug #7126, which
was reported by bepi at adria.it.
As this is a change in a core function, I'm proposing to leave
this only in master for 3.6.0, not back-port to any existing releases.
This should give us enough time to decide if this is the way we want this to
behave (as Windows) or if we prefer the previous behavior.
Jeremy.
-----------------------------------------------------------------------
Summary of changes:
source3/smbd/service.c | 104 +++++++++++++++++++++++++++++-------------------
1 files changed, 63 insertions(+), 41 deletions(-)
Changeset truncated at 500 lines:
diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index 4f55ea9..b3e833e 100644
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -647,25 +647,28 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
const char *pdev,
NTSTATUS *pstatus)
{
- connection_struct *conn;
+ connection_struct *conn = NULL;
struct smb_filename *smb_fname_cpath = NULL;
fstring dev;
int ret;
char addr[INET6_ADDRSTRLEN];
bool on_err_call_dis_hook = false;
+ bool claimed_connection = false;
+ uid_t effuid;
+ gid_t effgid;
NTSTATUS status;
fstrcpy(dev, pdev);
if (NT_STATUS_IS_ERR(*pstatus = share_sanity_checks(snum, dev))) {
- return NULL;
- }
+ goto err_root_exit;
+ }
conn = conn_new(sconn);
if (!conn) {
DEBUG(0,("Couldn't find free connection.\n"));
*pstatus = NT_STATUS_INSUFFICIENT_RESOURCES;
- return NULL;
+ goto err_root_exit;
}
conn->params->service = snum;
@@ -678,8 +681,7 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
DEBUG(1, ("create_connection_server_info failed: %s\n",
nt_errstr(status)));
*pstatus = status;
- conn_free(conn);
- return NULL;
+ goto err_root_exit;
}
if ((lp_guest_only(snum)) || (lp_security() == SEC_SHARE)) {
@@ -733,18 +735,16 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
fuser = talloc_string_sub(conn, lp_force_user(snum), "%S",
lp_servicename(snum));
if (fuser == NULL) {
- conn_free(conn);
*pstatus = NT_STATUS_NO_MEMORY;
- return NULL;
+ goto err_root_exit;
}
status = make_serverinfo_from_username(
conn, fuser, conn->server_info->guest,
&forced_serverinfo);
if (!NT_STATUS_IS_OK(status)) {
- conn_free(conn);
*pstatus = status;
- return NULL;
+ goto err_root_exit;
}
TALLOC_FREE(conn->server_info);
@@ -767,9 +767,8 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
&conn->server_info->utok.gid);
if (!NT_STATUS_IS_OK(status)) {
- conn_free(conn);
*pstatus = status;
- return NULL;
+ goto err_root_exit;
}
/*
@@ -793,16 +792,14 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
pdb_get_domain(conn->server_info->sam_account),
lp_pathname(snum));
if (!s) {
- conn_free(conn);
*pstatus = NT_STATUS_NO_MEMORY;
- return NULL;
+ goto err_root_exit;
}
if (!set_conn_connectpath(conn,s)) {
TALLOC_FREE(s);
- conn_free(conn);
*pstatus = NT_STATUS_NO_MEMORY;
- return NULL;
+ goto err_root_exit;
}
DEBUG(3,("Connect path is '%s' for service [%s]\n",s,
lp_servicename(snum)));
@@ -832,9 +829,8 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
"denied due to security "
"descriptor.\n",
lp_servicename(snum)));
- conn_free(conn);
*pstatus = NT_STATUS_ACCESS_DENIED;
- return NULL;
+ goto err_root_exit;
} else {
conn->read_only = True;
}
@@ -845,9 +841,8 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
if (!smbd_vfs_init(conn)) {
DEBUG(0, ("vfs_init failed for service %s\n",
lp_servicename(snum)));
- conn_free(conn);
*pstatus = NT_STATUS_BAD_NETWORK_NAME;
- return NULL;
+ goto err_root_exit;
}
if ((!conn->printer) && (!conn->ipc)) {
@@ -872,20 +867,19 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
DEBUG(1, ("Max connections (%d) exceeded for %s\n",
lp_max_connections(snum), lp_servicename(snum)));
- conn_free(conn);
*pstatus = NT_STATUS_INSUFFICIENT_RESOURCES;
- return NULL;
- }
+ goto err_root_exit;
+ }
/*
* Get us an entry in the connections db
*/
if (!claim_connection(conn, lp_servicename(snum), 0)) {
DEBUG(1, ("Could not store connections entry\n"));
- conn_free(conn);
*pstatus = NT_STATUS_INTERNAL_DB_ERROR;
- return NULL;
- }
+ goto err_root_exit;
+ }
+ claimed_connection = true;
/*
* Fix compatibility issue pointed out by Volker.
@@ -917,10 +911,8 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
if (ret != 0 && lp_rootpreexec_close(snum)) {
DEBUG(1,("root preexec gave %d - failing "
"connection\n", ret));
- yield_connection(conn, lp_servicename(snum));
- conn_free(conn);
*pstatus = NT_STATUS_ACCESS_DENIED;
- return NULL;
+ goto err_root_exit;
}
}
@@ -928,15 +920,16 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
if (!change_to_user(conn, conn->vuid)) {
/* No point continuing if they fail the basic checks */
DEBUG(0,("Can't become connected user!\n"));
- yield_connection(conn, lp_servicename(snum));
- conn_free(conn);
*pstatus = NT_STATUS_LOGON_FAILURE;
- return NULL;
+ goto err_root_exit;
}
+ effuid = geteuid();
+ effgid = getegid();
+
/* Remember that a different vuid can connect later without these
* checks... */
-
+
/* Preexecs are done here as they might make the dir we are to ChDir
* to below */
@@ -968,6 +961,14 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
* depend on the realpath() pointer in the vfs table. JRA.
*/
if (!lp_widelinks(snum)) {
+
+ /* We need to do the path canonicalization
+ * as root, as we may not have rights to
+ * this path as the user. */
+
+ change_to_root_user();
+
+/* ROOT Activites: */
if (!canonicalize_connect_path(conn)) {
DEBUG(0, ("canonicalize_connect_path failed "
"for service %s, path %s\n",
@@ -976,6 +977,13 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
*pstatus = NT_STATUS_BAD_NETWORK_NAME;
goto err_root_exit;
}
+
+ /* Back to the user for the VFS_CONNECT call. */
+ if (!change_to_user(conn, conn->vuid)) {
+ *pstatus = NT_STATUS_LOGON_FAILURE;
+ goto err_root_exit;
+ }
+/* USER Activites: */
}
#ifdef WITH_FAKE_KASERVER
@@ -983,7 +991,7 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
afs_login(conn);
}
#endif
-
+
/* Add veto/hide lists */
if (!IS_IPC(conn) && !IS_PRINT(conn)) {
set_namearray( &conn->veto_list, lp_veto_files(snum));
@@ -992,7 +1000,7 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
set_namearray( &conn->aio_write_behind_list,
lp_aio_write_behind(snum));
}
-
+
/* Invoke VFS make connection hook - do this before the VFS_STAT call
to allow any filesystems needing user credentials to initialize
themselves. */
@@ -1019,6 +1027,15 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
check during individual operations. To match this behaviour
I have disabled this chdir check (tridge) */
/* the alternative is just to check the directory exists */
+
+ /*
+ * we've finished with the user stuff - go back to root
+ * so the SMB_VFS_STAT call will only fail on path errors,
+ * not permission problems.
+ */
+ change_to_root_user();
+
+/* ROOT Activites: */
if ((ret = SMB_VFS_STAT(conn, smb_fname_cpath)) != 0 ||
!S_ISDIR(smb_fname_cpath->st.st_ex_mode)) {
if (ret == 0 && !S_ISDIR(smb_fname_cpath->st.st_ex_mode)) {
@@ -1057,23 +1074,28 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
dbgtext( "connect to service %s ", lp_servicename(snum) );
dbgtext( "initially as user %s ",
conn->server_info->unix_name );
- dbgtext( "(uid=%d, gid=%d) ", (int)geteuid(), (int)getegid() );
+ dbgtext( "(uid=%d, gid=%d) ", (int)effuid, (int)effgid );
dbgtext( "(pid %d)\n", (int)sys_getpid() );
}
- /* we've finished with the user stuff - go back to root */
- change_to_root_user();
return(conn);
err_root_exit:
TALLOC_FREE(smb_fname_cpath);
- change_to_root_user();
+ /* We must exit this function as root. */
+ if (geteuid() != sec_initial_uid()) {
+ change_to_root_user();
+ }
if (on_err_call_dis_hook) {
/* Call VFS disconnect hook */
SMB_VFS_DISCONNECT(conn);
}
- yield_connection(conn, lp_servicename(snum));
- conn_free(conn);
+ if (claimed_connection) {
+ yield_connection(conn, lp_servicename(snum));
+ }
+ if (conn) {
+ conn_free(conn);
+ }
return NULL;
}
--
Samba Shared Repository
More information about the samba-cvs
mailing list