s3-printing: follow force user/group for driver IO

Configuring force user/group settings for the print$ share currently has
unexpected results, this is explained by how the driver upload/add
process takes place. Consider the following example:

[print$]
        path = /print-drv
        write list = $DRIVER_UPLOAD_USER
        force group = ntadmin

- the client connects to the [print$] share and uploads all driver
  files to the /print-drv/W32X86 directory.

- This is permitted, as /print-drv/W32X86 is owned by group ntadmin, and
  the "force group = ntadmin" takes effect for the [print$] session.

- Once all files are uploaded, the client connects to the [ipc$]
  share and issues an AddPrinterDriverEx spoolss request.

- In handling this request move_driver_to_download_area() is called,
  which attempts to create the directory /print-drv/W32X86/3

- The create directory fails, as it is done as the user connected to
  the [ipc$] share which does not have permission to write to the driver
  directory. The [print$] "force group = ntadmin" has no effect.

This is a regression from previous behaviour prior to the commit:
783ab04 Convert move_driver_to_download_area to use create_conn_struct.

https://bugzilla.samba.org/show_bug.cgi?id=7921
Signed-off-by: Andreas Schneider <asn@samba.org>

diff --git a/source3/include/proto.h b/source3/include/proto.h
index e866dedc60a2f16065abd6eac26fae3b935ac273..199ee4836cb19a4fb5ff1aac798e35f726804a8c 100644 (file)
--- a/source3/include/proto.h
+++ b/source3/include/proto.h
@@ -5150,6 +5150,7 @@ void exit_server_fault(void);
 /* The following definitions come from smbd/service.c  */
 
 bool set_conn_connectpath(connection_struct *conn, const char *connectpath);
+NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum);
 bool set_current_service(connection_struct *conn, uint16 flags, bool do_chdir);
 void load_registry_shares(void);
 int add_home_service(const char *service, const char *username, const char *homedir);

diff --git a/source3/printing/nt_printing.c b/source3/printing/nt_printing.c
index d77320cf020626d1a2943fca23f387428d29ac84..1d075e794e06e4212bc7a15170fef3c324f73c76 100644 (file)
--- a/source3/printing/nt_printing.c
+++ b/source3/printing/nt_printing.c
@@ -625,6 +625,19 @@ static uint32 get_correct_cversion(struct pipes_struct *p,
                return -1;
        }
 
+       nt_status = set_conn_force_user_group(conn, printdollar_snum);
+       if (!NT_STATUS_IS_OK(nt_status)) {
+               DEBUG(0, ("failed set force user / group\n"));
+               *perr = ntstatus_to_werror(nt_status);
+               goto error_free_conn;
+       }
+
+       if (!become_user(conn, get_current_vuid(conn))) {
+               DEBUG(0, ("failed to become user\n"));
+               *perr = WERR_ACCESS_DENIED;
+               goto error_free_conn;
+       }
+
        /* Open the driver file (Portable Executable format) and determine the
         * deriver the cversion. */
        driverpath = talloc_asprintf(talloc_tos(),
@@ -720,6 +733,8 @@ static uint32 get_correct_cversion(struct pipes_struct *p,
        *perr = WERR_OK;
 
  error_exit:
+       unbecome_user();
+ error_free_conn:
        TALLOC_FREE(smb_fname);
        if (fsp != NULL) {
                close_file(NULL, fsp, NORMAL_CLOSE);
@@ -993,6 +1008,19 @@ WERROR move_driver_to_download_area(struct pipes_struct *p,
                return err;
        }
 
+       nt_status = set_conn_force_user_group(conn, printdollar_snum);
+       if (!NT_STATUS_IS_OK(nt_status)) {
+               DEBUG(0, ("failed set force user / group\n"));
+               err = ntstatus_to_werror(nt_status);
+               goto err_free_conn;
+       }
+
+       if (!become_user(conn, get_current_vuid(conn))) {
+               DEBUG(0, ("failed to become user\n"));
+               err = WERR_ACCESS_DENIED;
+               goto err_free_conn;
+       }
+
        new_dir = talloc_asprintf(ctx,
                                "%s/%d",
                                short_architecture,
@@ -1126,7 +1154,9 @@ WERROR move_driver_to_download_area(struct pipes_struct *p,
        }
 
        err = WERR_OK;
-  err_exit:
+ err_exit:
+       unbecome_user();
+ err_free_conn:
        TALLOC_FREE(smb_dname);
 
        if (conn != NULL) {
@@ -1907,6 +1937,19 @@ bool delete_driver_files(const struct auth_serversupplied_info *session_info,
                return false;
        }
 
+       nt_status = set_conn_force_user_group(conn, printdollar_snum);
+       if (!NT_STATUS_IS_OK(nt_status)) {
+               DEBUG(0, ("failed set force user / group\n"));
+               ret = false;
+               goto err_free_conn;
+       }
+
+       if (!become_user(conn, get_current_vuid(conn))) {
+               DEBUG(0, ("failed to become user\n"));
+               ret = false;
+               goto err_free_conn;
+       }
+
        if ( !CAN_WRITE(conn) ) {
                DEBUG(3,("delete_driver_files: Cannot delete print driver when [print$] is read-only\n"));
                ret = false;
@@ -1968,6 +2011,8 @@ bool delete_driver_files(const struct auth_serversupplied_info *session_info,
 
        ret = true;
  err_out:
+       unbecome_user();
+ err_free_conn:
        if (conn != NULL) {
                vfs_ChDir(conn, oldcwd);
                SMB_VFS_DISCONNECT(conn);

diff --git a/source3/smbd/service.c b/source3/smbd/service.c
index e713b7132e9024d23b26f4d96985fa600cb1574c..00b5294be66843c07f62acdcc217ea45ddbbb96e 100644 (file)
--- a/source3/smbd/service.c
+++ b/source3/smbd/service.c
@@ -659,6 +659,72 @@ static NTSTATUS create_connection_session_info(struct smbd_server_connection *sc
        return NT_STATUS_ACCESS_DENIED;
 }
 
+/****************************************************************************
+  set relavent user and group settings corresponding to force user/group
+  configuration for the given snum.
+****************************************************************************/
+
+NTSTATUS set_conn_force_user_group(connection_struct *conn, int snum)
+{
+       NTSTATUS status;
+
+       if (*lp_force_user(snum)) {
+
+               /*
+                * Replace conn->session_info with a completely faked up one
+                * from the username we are forced into :-)
+                */
+
+               char *fuser;
+               struct auth_serversupplied_info *forced_serverinfo;
+
+               fuser = talloc_string_sub(conn, lp_force_user(snum), "%S",
+                                         lp_const_servicename(snum));
+               if (fuser == NULL) {
+                       return NT_STATUS_NO_MEMORY;
+               }
+
+               status = make_serverinfo_from_username(
+                       conn, fuser, conn->session_info->guest,
+                       &forced_serverinfo);
+               if (!NT_STATUS_IS_OK(status)) {
+                       return status;
+               }
+
+               TALLOC_FREE(conn->session_info);
+               conn->session_info = forced_serverinfo;
+
+               conn->force_user = true;
+               DEBUG(3,("Forced user %s\n", fuser));
+       }
+
+       /*
+        * If force group is true, then override
+        * any groupid stored for the connecting user.
+        */
+
+       if (*lp_force_group(snum)) {
+
+               status = find_forced_group(
+                       conn->force_user, snum, conn->session_info->unix_name,
+                       &conn->session_info->security_token->sids[1],
+                       &conn->session_info->utok.gid);
+
+               if (!NT_STATUS_IS_OK(status)) {
+                       return status;
+               }
+
+               /*
+                * We need to cache this gid, to use within
+                * change_to_user() separately from the conn->session_info
+                * struct. We only use conn->session_info directly if
+                * "force_user" was set.
+                */
+               conn->force_group_gid = conn->session_info->utok.gid;
+       }
+
+       return NT_STATUS_OK;
+}
 
 /****************************************************************************
   Make a connection, given the snum to connect to, and the vuser of the
@@ -742,62 +808,11 @@ connection_struct *make_connection_snum(struct smbd_server_connection *sconn,
 
        conn->read_only = lp_readonly(SNUM(conn));
 
-       if (*lp_force_user(snum)) {
-
-               /*
-                * Replace conn->session_info with a completely faked up one
-                * from the username we are forced into :-)
-                */
-
-               char *fuser;
-               struct auth_serversupplied_info *forced_serverinfo;
-
-               fuser = talloc_string_sub(conn, lp_force_user(snum), "%S",
-                                         lp_servicename(snum));
-               if (fuser == NULL) {
-                       *pstatus = NT_STATUS_NO_MEMORY;
-                       goto err_root_exit;
-               }
-
-               status = make_serverinfo_from_username(
-                       conn, fuser, conn->session_info->guest,
-                       &forced_serverinfo);
-               if (!NT_STATUS_IS_OK(status)) {
-                       *pstatus = status;
-                       goto err_root_exit;
-               }
-
-               TALLOC_FREE(conn->session_info);
-               conn->session_info = forced_serverinfo;
-
-               conn->force_user = True;
-               DEBUG(3,("Forced user %s\n", fuser));
-       }
-
-       /*
-        * If force group is true, then override
-        * any groupid stored for the connecting user.
-        */
-
-       if (*lp_force_group(snum)) {
-
-               status = find_forced_group(
-                       conn->force_user, snum, conn->session_info->unix_name,
-                       &conn->session_info->security_token->sids[1],
-                       &conn->session_info->utok.gid);
-
-               if (!NT_STATUS_IS_OK(status)) {
-                       *pstatus = status;
-                       goto err_root_exit;
-               }
-
-               /*
-                * We need to cache this gid, to use within
-                * change_to_user() separately from the conn->session_info
-                * struct. We only use conn->session_info directly if
-                * "force_user" was set.
-                */
-               conn->force_group_gid = conn->session_info->utok.gid;
+       status = set_conn_force_user_group(conn, snum);
+       if (!NT_STATUS_IS_OK(status)) {
+               conn_free(conn);
+               *pstatus = status;
+               return NULL;
        }
 
        conn->vuid = (vuser != NULL) ? vuser->vuid : UID_FIELD_INVALID;