Change how the umask is handled with PAM and login.conf.

If the umask is explicitly set in sudoers, use that value regardless
of what is in PAM or login.conf.  If using the default umask from
sudoers, allow PAM or login.conf to override it.  Bug #900

doc/sudo_plugin.man.in

@@ -16,7 +16,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.TH "SUDO_PLUGIN" "5" "June 20, 2019" "Sudo @PACKAGE_VERSION@" "File Formats Manual"
+.TH "SUDO_PLUGIN" "5" "October 18, 2019" "Sudo @PACKAGE_VERSION@" "File Formats Manual"
 .nh
 .if n .ad l
 .SH "NAME"
@@ -1079,6 +1079,15 @@ If non-zero then when the timeout expires the command will be killed.
 .TP 6n
 umask=octal
 The file creation mask to use when executing the command.
+This value may be overridden by PAM or login.conf on some systems
+unless the
+\fIumask_override\fR
+option is also set.
+.TP 6n
+umask_override=bool
+Force the value specified by the
+\fIumask\fR
+option to override any umask set by PAM or login.conf.
 .TP 6n
 use_pty=bool
 Allocate a pseudo-terminal to run the command in, regardless of whether
@@ -2950,6 +2959,13 @@ Version 1.13 (sudo 1.8.26)
 The
 \fRlog_suspend\fR
 field was added to the io_plugin struct.
+.TP 6n
+Version 1.14 (sudo 1.8.29)
+The
+\fIumask_override\fR
+entry was added to the
+\fRcommand_info\fR
+list.
 .SH "SEE ALSO"
 sudo.conf(@mansectform@),
 sudoers(@mansectform@),

doc/sudo_plugin.mdoc.in

@@ -15,7 +15,7 @@
 .\" ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
 .\" OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 .\"
-.Dd June 20, 2019
+.Dd October 18, 2019
 .Dt SUDO_PLUGIN @mansectform@
 .Os Sudo @PACKAGE_VERSION@
 .Sh NAME
@@ -953,6 +953,14 @@ Command timeout.
 If non-zero then when the timeout expires the command will be killed.
 .It umask=octal
 The file creation mask to use when executing the command.
+This value may be overridden by PAM or login.conf on some systems
+unless the
+.Em umask_override
+option is also set.
+.It umask_override=bool
+Force the value specified by the
+.Em umask
+option to override any umask set by PAM or login.conf.
 .It use_pty=bool
 Allocate a pseudo-terminal to run the command in, regardless of whether
 or not I/O logging is in use.
@@ -2589,6 +2597,12 @@ field was added to the io_plugin struct.
 The
 .Li log_suspend
 field was added to the io_plugin struct.
+.It Version 1.14 (sudo 1.8.29)
+The
+.Em umask_override
+entry was added to the
+.Li command_info
+list.
 .El
 .Sh SEE ALSO
 .Xr sudo.conf @mansectform@ ,

doc/sudoers.man.in

@@ -3408,20 +3408,32 @@ and
 respectively.
 .TP 18n
 umask
-Umask to use when running the command.
-Negate this option or set it to 0777 to preserve the user's umask.
-The actual umask that is used will be the union of the user's umask
-and the value of the
+File mode creation mask to use when running the command.
+Negate this option or set it to 0777 to prevent
+\fBsudoers\fR
+from changing the umask.
+Unless the
+\fIumask_override\fR
+flag is set, the actual umask will be the union of the
+user's umask and the value of the
 \fIumask\fR
-option, which defaults to
+setting, which defaults to
 \fR@sudo_umask@\fR.
 This guarantees
 that
 \fBsudo\fR
 never lowers the umask when running a command.
-Note: on systems that use PAM, the default PAM configuration may specify
-its own umask which will override the value set in
-\fIsudoers\fR.
+.sp
+If
+\fIumask\fR
+is explicitly set in
+\fIsudoers\fR,
+it will override any umask setting in PAM or login.conf.
+If
+\fIumask\fR
+is not set in
+\fIsudoers\fR,
+the umask specified by PAM or login.conf will take precedence.
 .PP
 \fBStrings\fR:
 .TP 18n

doc/sudoers.mdoc.in

@@ -3209,20 +3209,32 @@ and
 .Dq Li sudo -k
 respectively.
 .It umask
-Umask to use when running the command.
-Negate this option or set it to 0777 to preserve the user's umask.
-The actual umask that is used will be the union of the user's umask
-and the value of the
+File mode creation mask to use when running the command.
+Negate this option or set it to 0777 to prevent
+.Nm
+from changing the umask.
+Unless the
+.Em umask_override
+flag is set, the actual umask will be the union of the
+user's umask and the value of the
 .Em umask
-option, which defaults to
+setting, which defaults to
 .Li @sudo_umask@ .
 This guarantees
 that
 .Nm sudo
 never lowers the umask when running a command.
-Note: on systems that use PAM, the default PAM configuration may specify
-its own umask which will override the value set in
-.Em sudoers .
+.Pp
+If
+.Em umask
+is explicitly set in
+.Em sudoers ,
+it will override any umask setting in PAM or login.conf.
+If
+.Em umask
+is not set in
+.Em sudoers ,
+the umask specified by PAM or login.conf will take precedence.
 .El
 .Pp
 .Sy Strings :

include/sudo_plugin.h

@@ -21,7 +21,7 @@
 
 /* API version major/minor */
 #define SUDO_API_VERSION_MAJOR 1
-#define SUDO_API_VERSION_MINOR 13
+#define SUDO_API_VERSION_MINOR 14
 #define SUDO_API_MKVERSION(x, y) (((x) << 16) | (y))
 #define SUDO_API_VERSION SUDO_API_MKVERSION(SUDO_API_VERSION_MAJOR, SUDO_API_VERSION_MINOR)
 

plugins/sudoers/policy.c

@@ -693,6 +693,10 @@ sudoers_policy_exec_setup(char *argv[], char *envp[], mode_t cmnd_umask,
 	if (asprintf(&command_info[info_len++], "umask=0%o", (unsigned int)cmnd_umask) == -1)
 	    goto oom;
     }
+    if (force_umask) {
+	if ((command_info[info_len++] = strdup("umask_override=true")) == NULL)
+	    goto oom;
+    }
     if (cmnd_fd != -1) {
 	if (sudo_version < SUDO_API_MKVERSION(1, 9)) {
 	    /* execfd only supported by plugin API 1.9 and higher */

plugins/sudoers/sudoers.c

@@ -79,6 +79,7 @@
 static bool cb_fqdn(const union sudo_defs_val *);
 static bool cb_runas_default(const union sudo_defs_val *);
 static bool cb_tty_tickets(const union sudo_defs_val *);
+static bool cb_umask(const union sudo_defs_val *);
 static int set_cmnd(void);
 static int create_admin_success_flag(void);
 static bool init_vars(char * const *);
@@ -97,6 +98,7 @@ gid_t timestamp_gid;
 #ifdef HAVE_BSD_AUTH_H
 char *login_style;
 #endif /* HAVE_BSD_AUTH_H */
+bool force_umask;
 int sudo_mode;
 
 static char *prev_user;
@@ -738,6 +740,9 @@ init_vars(char * const envp[])
     /* Set tty_tickets callback. */
     sudo_defs_table[I_TTY_TICKETS].callback = cb_tty_tickets;
 
+    /* Set umask callback. */
+    sudo_defs_table[I_UMASK].callback = cb_umask;
+
     /* It is now safe to use log_warningx() and set_perms() */
     if (unknown_user) {
 	log_warningx(SLOG_SEND_MAIL, N_("unknown uid: %u"),
@@ -1211,7 +1216,7 @@ cb_runas_default(const union sudo_defs_val *sd_un)
 }
 
 /*
- * Callback for runas_default sudoers setting.
+ * Callback for tty_tickets sudoers setting.
  */
 static bool
 cb_tty_tickets(const union sudo_defs_val *sd_un)
@@ -1226,6 +1231,20 @@ cb_tty_tickets(const union sudo_defs_val *sd_un)
     debug_return_bool(true);
 }
 
+/*
+ * Callback for umask sudoers setting.
+ */
+static bool
+cb_umask(const union sudo_defs_val *sd_un)
+{
+    debug_decl(cb_umask, SUDOERS_DEBUG_PLUGIN)
+
+    /* Force umask if explicitly set in sudoers. */
+    force_umask = sd_un->mode != ACCESSPERMS;
+
+    debug_return_bool(true);
+}
+
 /*
  * Cleanup hook for sudo_fatal()/sudo_fatalx()
  */

plugins/sudoers/sudoers.h

@@ -391,6 +391,7 @@ int sudoers_policy_main(int argc, char * const argv[], int pwflag, char *env_add
 void sudoers_cleanup(void);
 extern struct sudo_user sudo_user;
 extern struct passwd *list_pw;
+extern bool force_umask;
 extern int sudo_mode;
 extern uid_t timestamp_uid;
 extern gid_t timestamp_gid;

src/exec.c

@@ -147,8 +147,8 @@ exec_setup(struct command_details *details)
 	    login_cap_t *lc;
 
 	    /*
-	     * We only use setusercontext() to set the nice value and rlimits
-	     * unless this is a login shell (sudo -i).
+	     * We only use setusercontext() to set the nice value, rlimits
+	     * and umask unless this is a login shell (sudo -i).
 	     */
 	    lc = login_getclass((char *)details->login_class);
 	    if (!lc) {
@@ -160,9 +160,8 @@ exec_setup(struct command_details *details)
 		/* Set everything except user, group and login name. */
 		flags = LOGIN_SETALL;
 		CLR(flags, LOGIN_SETGROUP|LOGIN_SETLOGIN|LOGIN_SETUSER|LOGIN_SETENV|LOGIN_SETPATH);
-		CLR(details->flags, CD_SET_UMASK); /* LOGIN_UMASK instead */
 	    } else {
-		flags = LOGIN_SETRESOURCES|LOGIN_SETPRIORITY;
+		flags = LOGIN_SETRESOURCES|LOGIN_SETPRIORITY|LOGIN_SETUMASK;
 	    }
 	    if (setusercontext(lc, details->pw, details->pw->pw_uid, flags)) {
 		sudo_warn(U_("unable to set user context"));
@@ -185,8 +184,11 @@ exec_setup(struct command_details *details)
 	    goto done;
 	}
     }
-    if (ISSET(details->flags, CD_SET_UMASK))
+
+    /* Policy may override umask in PAM or login.conf. */
+    if (ISSET(details->flags, CD_OVERRIDE_UMASK))
 	(void) umask(details->umask);
+
     if (details->chroot) {
 	if (chroot(details->chroot) != 0 || chdir("/") != 0) {
 	    sudo_warn(U_("unable to change root to %s"), details->chroot);
@@ -344,7 +346,7 @@ sudo_terminated(struct command_status *cstat)
     debug_return_bool(false);
 }
 
-#if SUDO_API_VERSION != SUDO_API_MKVERSION(1, 13)
+#if SUDO_API_VERSION != SUDO_API_MKVERSION(1, 14)
 # error "Update sudo_needs_pty() after changing the plugin API"
 #endif
 static bool

src/sudo.c

@@ -816,6 +816,7 @@ command_info_to_details(char * const info[], struct command_details *details)
 		    SET(details->flags, CD_SET_UMASK);
 		    break;
 		}
+		SET_FLAG("umask_override=", CD_OVERRIDE_UMASK)
 		SET_FLAG("use_pty=", CD_USE_PTY)
 		SET_STRING("utmp_user=", utmp_user)
 		break;
@@ -826,6 +827,8 @@ command_info_to_details(char * const info[], struct command_details *details)
 	details->euid = details->uid;
     if (!ISSET(details->flags, CD_SET_EGID))
 	details->egid = details->gid;
+    if (!ISSET(details->flags, CD_SET_UMASK))
+	CLR(details->flags, CD_OVERRIDE_UMASK);
 
 #ifdef HAVE_SETAUTHDB
     aix_setauthdb(IDtouser(details->euid), NULL);
@@ -1235,6 +1238,10 @@ policy_init_session(struct command_details *details)
 	    goto done;
     }
 
+    /* Session setup may override sudoers umask so set it first. */
+    if (ISSET(details->flags, CD_SET_UMASK))
+	(void) umask(details->umask);
+
     if (policy_plugin.u.policy->init_session) {
 	/*
 	 * Backwards compatibility for older API versions

src/sudo.h

@@ -120,26 +120,27 @@ struct user_details {
     int ts_cols;
 };
 
-#define CD_SET_UID		0x00001
-#define CD_SET_EUID		0x00002
-#define CD_SET_GID		0x00004
-#define CD_SET_EGID		0x00008
-#define CD_PRESERVE_GROUPS	0x00010
-#define CD_NOEXEC		0x00020
-#define CD_SET_PRIORITY		0x00040
-#define CD_SET_UMASK		0x00080
-#define CD_SET_TIMEOUT		0x00100
-#define CD_SUDOEDIT		0x00200
-#define CD_BACKGROUND		0x00400
-#define CD_RBAC_ENABLED		0x00800
-#define CD_USE_PTY		0x01000
-#define CD_SET_UTMP		0x02000
-#define CD_EXEC_BG		0x04000
-#define CD_SUDOEDIT_COPY	0x08000
-#define CD_SUDOEDIT_FOLLOW	0x10000
-#define CD_SUDOEDIT_CHECKDIR	0x20000
-#define CD_SET_GROUPS		0x40000
-#define CD_LOGIN_SHELL		0x80000
+#define CD_SET_UID		0x000001
+#define CD_SET_EUID		0x000002
+#define CD_SET_GID		0x000004
+#define CD_SET_EGID		0x000008
+#define CD_PRESERVE_GROUPS	0x000010
+#define CD_NOEXEC		0x000020
+#define CD_SET_PRIORITY		0x000040
+#define CD_SET_UMASK		0x000080
+#define CD_SET_TIMEOUT		0x000100
+#define CD_SUDOEDIT		0x000200
+#define CD_BACKGROUND		0x000400
+#define CD_RBAC_ENABLED		0x000800
+#define CD_USE_PTY		0x001000
+#define CD_SET_UTMP		0x002000
+#define CD_EXEC_BG		0x004000
+#define CD_SUDOEDIT_COPY	0x008000
+#define CD_SUDOEDIT_FOLLOW	0x010000
+#define CD_SUDOEDIT_CHECKDIR	0x020000
+#define CD_SET_GROUPS		0x040000
+#define CD_LOGIN_SHELL		0x080000
+#define CD_OVERRIDE_UMASK	0x100000
 
 struct preserved_fd {
     TAILQ_ENTRY(preserved_fd) entries;