Pass a pointer to user_env in to the init_session policy plugin

function so session setup can modify the user environment as needed.
For PAM authentication, merge the PAM environment with the user
environment at init_session time.  We no longer need to swap in the
user_env for environ during session init, nor do we need to disable
the env hooks at init_session time.

doc/sudo_plugin.pod

@@ -102,7 +102,7 @@ so that B<sudo> can load it.
                  const char *list_user);
      int (*validate)(void);
      void (*invalidate)(int remove);
-     int (*init_session)(struct passwd *pwd);
+     int (*init_session)(struct passwd *pwd, char **user_env[]);
      void (*register_hooks)(int version,
 	int (*register_hook)(struct sudo_hook *hook));
      void (*deregister_hooks)(int version,
@@ -384,7 +384,7 @@ itself but the I<value> might.
 
 Any (non-comment) strings immediately after the plugin path are
 treated as arguments to the plugin.  These arguments are split on
-a whitespace boundary and are passed to the plugin in the form of
+a white space boundary and are passed to the plugin in the form of
 a C<NULL>-terminated array of strings.  If no arguments were
 specified, I<plugin_args> will be the NULL pointer.
 
@@ -742,7 +742,7 @@ support credential caching.
 
 =item init_session
 
- int (*init_session)(struct passwd *pwd);
+ int (*init_session)(struct passwd *pwd, char **user_envp[);
 
 The C<init_session> function is called when B<sudo> sets up the
 execution environment for the command, immediately before the
@@ -754,6 +754,18 @@ The I<pwd> argument points to a passwd struct for the user the
 command will be run as if the uid the command will run as was found
 in the password database, otherwise it will be NULL.
 
+The I<user_env> argument points to the environment the command will
+run in, in the form of a C<NULL>-terminated vector of "name=value"
+strings.  This is the same string passed back to the front end via
+the Policy Plugin's I<user_env_out> parameter.  If the C<init_session>
+function needs to modify the user environment, it should update the
+pointer stored in I<user_env>.  The expected use case is to merge
+the contents of the PAM environment (if any) with the contents of
+I<user_env>.  NOTE: the I<user_env> parameter is only available
+starting with API version 1.2.  A plugin B<must> check the API
+version specified by the B<sudo> front end before using I<user_env>.
+Failure to do so may result in a crash.
+
 Returns 1 on success, 0 on failure and -1 on error.
 On error, the plugin may optionally call the conversation or plugin_printf
 function with C<SUDO_CONF_ERROR_MSG> to present additional
@@ -771,7 +783,7 @@ hooks, C<register_hooks> should be set to the NULL pointer.
 The I<version> argument describes the version of the hooks API
 supported by the B<sudo> front end.
 
-The C<register_hook> function should be used to register any suppored
+The C<register_hook> function should be used to register any supported
 hooks the plugin needs.  It returns 0 on success, 1 if the hook
 type is not supported and -1 if the major version in C<struct hook>
 does not match the front end's major hook API version.
@@ -979,7 +991,7 @@ itself but the I<value> might.
 
 Any (non-comment) strings immediately after the plugin path are
 treated as arguments to the plugin.  These arguments are split on
-a whitespace boundary and are passed to the plugin in the form of
+a white space boundary and are passed to the plugin in the form of
 a C<NULL>-terminated array of strings.  If no arguments were
 specified, I<plugin_args> will be the NULL pointer.
 
@@ -1183,13 +1195,6 @@ be reflected in the version of the environment that is used to
 execute a command.  A future version of the API will support
 hooking internal B<sudo> front end functions as well.
 
-Environment-related hooks are disabled prior to the execution of
-the C<init_session> policy plugin function (if any).  This is
-necessary because C<init_session> has no way of passing back a
-modified environment pointer.  However, since the user environment
-specified by the C<check_policy> function is already in place, there
-should be no need to hook the environment functions at that time.
-
 =head3 Hook structure
 
 Hooks in B<sudo> are described by the following structure:

include/sudo_plugin.h

@@ -127,7 +127,7 @@ struct policy_plugin {
 	const char *list_user);
     int (*validate)(void);
     void (*invalidate)(int remove);
-    int (*init_session)(struct passwd *pwd);
+    int (*init_session)(struct passwd *pwd, char **user_env_out[]);
     void (*register_hooks)(int version, int (*register_hook)(struct sudo_hook *hook));
     void (*deregister_hooks)(int version, int (*deregister_hook)(struct sudo_hook *hook));
 };

plugins/sudoers/auth/pam.c

@@ -196,7 +196,7 @@ sudo_pam_cleanup(struct passwd *pw, sudo_auth *auth)
 }
 
 int
-sudo_pam_begin_session(struct passwd *pw, sudo_auth *auth)
+sudo_pam_begin_session(struct passwd *pw, char **user_envp[], sudo_auth *auth)
 {
     int status = PAM_SUCCESS;
     debug_decl(sudo_pam_begin_session, SUDO_DEBUG_AUTH)
@@ -230,6 +230,26 @@ sudo_pam_begin_session(struct passwd *pw, sudo_auth *auth)
      */
     (void) pam_setcred(pamh, PAM_ESTABLISH_CRED);
 
+#ifdef HAVE_PAM_GETENVLIST
+    /*
+     * Update environment based on what is stored in pamh.
+     * If no authentication is done we will only have environment
+     * variables if pam_env is called via session.
+     */
+    if (user_envp != NULL) {
+	char **pam_envp = pam_getenvlist(pamh);
+	if (pam_envp != NULL) {
+	    /* Merge pam env with user env but do not overwrite. */
+	    env_init(*user_envp);
+	    env_merge(pam_envp, false);
+	    *user_envp = env_get();
+	    env_init(NULL);
+	    efree(pam_envp);
+	    /* XXX - we leak any duplicates that were in pam_envp */
+	}
+    }
+#endif /* HAVE_PAM_GETENVLIST */
+
 #ifndef NO_PAM_SESSION
     status = pam_open_session(pamh, 0);
     if (status != PAM_SUCCESS) {

plugins/sudoers/auth/sudo_auth.c

@@ -286,7 +286,7 @@ done:
 }
 
 int
-sudo_auth_begin_session(struct passwd *pw)
+sudo_auth_begin_session(struct passwd *pw, char **user_env[])
 {
     sudo_auth *auth;
     int status;
@@ -294,7 +294,7 @@ sudo_auth_begin_session(struct passwd *pw)
 
     for (auth = auth_switch; auth->name; auth++) {
 	if (auth->begin_session && !IS_DISABLED(auth)) {
-	    status = (auth->begin_session)(pw, auth);
+	    status = (auth->begin_session)(pw, user_env, auth);
 	    if (status == AUTH_FATAL) {
 		/* XXX log */
 		audit_failure(NewArgv, "authentication failure");

plugins/sudoers/auth/sudo_auth.h

@@ -32,7 +32,7 @@ typedef struct sudo_auth {
     int (*setup)(struct passwd *pw, char **prompt, struct sudo_auth *auth);
     int (*verify)(struct passwd *pw, char *p, struct sudo_auth *auth);
     int (*cleanup)(struct passwd *pw, struct sudo_auth *auth);
-    int (*begin_session)(struct passwd *pw, struct sudo_auth *auth);
+    int (*begin_session)(struct passwd *pw, char **user_env[], struct sudo_auth *auth);
     int (*end_session)(struct passwd *pw, struct sudo_auth *auth);
 } sudo_auth;
 
@@ -66,7 +66,7 @@ int sudo_fwtk_cleanup(struct passwd *pw, sudo_auth *auth);
 int sudo_pam_init(struct passwd *pw, sudo_auth *auth);
 int sudo_pam_verify(struct passwd *pw, char *prompt, sudo_auth *auth);
 int sudo_pam_cleanup(struct passwd *pw, sudo_auth *auth);
-int sudo_pam_begin_session(struct passwd *pw, sudo_auth *auth);
+int sudo_pam_begin_session(struct passwd *pw, char **user_env[], sudo_auth *auth);
 int sudo_pam_end_session(struct passwd *pw, sudo_auth *auth);
 int sudo_securid_init(struct passwd *pw, sudo_auth *auth);
 int sudo_securid_setup(struct passwd *pw, char **prompt, sudo_auth *auth);

plugins/sudoers/env.c

@@ -90,6 +90,7 @@
 #define KEPT_MAX    	0xff00
 
 struct environment {
+    char * const *old_envp;	/* pointer the environment we passed back */
     char **envp;		/* pointer to the new environment */
     size_t env_size;		/* size of new_environ in char **'s */
     size_t env_len;		/* number of slots used, not counting NULL */
@@ -208,8 +209,10 @@ env_init(char * const envp[])
     debug_decl(env_init, SUDO_DEBUG_ENV)
 
     if (envp == NULL) {
-	/* Reset to initial state. */
+	/* Reset to initial state but keep a pointer to what we allocated. */
+	envp = env.envp;
 	memset(&env, 0, sizeof(env));
+	env.old_envp = envp;
     } else {
 	/* Make private copy of envp. */
 	for (ep = envp; *ep != NULL; ep++)
@@ -224,6 +227,10 @@ env_init(char * const envp[])
 #endif
 	memcpy(env.envp, envp, len * sizeof(char *));
 	env.envp[len] = '\0';
+
+	/* Free the old envp we allocated, if any. */
+	if (env.old_envp != NULL)
+	    efree((void *)env.old_envp);
     }
 
     debug_return;
@@ -485,6 +492,21 @@ sudo_getenv(const char *name)
     debug_return_str(val);
 }
 
+/*
+ * Merge another environment with our private copy.
+ */
+void
+env_merge(char * const envp[], bool overwrite)
+{
+    char * const *ep;
+    debug_decl(env_merge, SUDO_DEBUG_ENV)
+
+    for (ep = envp; *ep != NULL; ep++)
+	sudo_putenv(*ep, true, overwrite);
+
+    debug_return;
+}
+
 /*
  * Check the env_delete blacklist.
  * Returns true if the variable was found, else false.

plugins/sudoers/sudoers.c

@@ -122,6 +122,7 @@ sudo_conv_t sudo_conv;
 sudo_printf_t sudo_printf;
 int sudo_mode;
 
+static int sudo_version;
 static char *prev_user;
 static char *runas_user;
 static char *runas_group;
@@ -146,15 +147,16 @@ sudoers_policy_open(unsigned int version, sudo_conv_t conversation,
     struct sudo_nss *nss;
     debug_decl(sudoers_policy_open, SUDO_DEBUG_PLUGIN)
 
-    /* Plugin args are only specified for API version 1.2 and higher. */
+    sudo_version = version;
-    if (version < SUDO_API_MKVERSION(1, 2))
-	args = NULL;
-
     if (!sudo_conv)
 	sudo_conv = conversation;
     if (!sudo_printf)
 	sudo_printf = plugin_printf;
 
+    /* Plugin args are only specified for API version 1.2 and higher. */
+    if (sudo_version < SUDO_API_MKVERSION(1, 2))
+	args = NULL;
+
     if (sigsetjmp(error_jmp, 1)) {
 	/* called via error(), errorx() or log_error() */
 	rewind_perms();
@@ -281,16 +283,20 @@ sudoers_policy_close(int exit_status, int error_code)
  * and before uid/gid changes occur.
  */
 static int
-sudoers_policy_init_session(struct passwd *pwd)
+sudoers_policy_init_session(struct passwd *pwd, char **user_env[])
 {
     debug_decl(sudoers_policy_init, SUDO_DEBUG_PLUGIN)
 
+    /* user_env is only specified for API version 1.2 and higher. */
+    if (sudo_version < SUDO_API_MKVERSION(1, 2))
+	user_env = NULL;
+
     if (sigsetjmp(error_jmp, 1)) {
 	/* called via error(), errorx() or log_error() */
 	return -1;
     }
 
-    debug_return_bool(sudo_auth_begin_session(pwd));
+    debug_return_bool(sudo_auth_begin_session(pwd, user_env));
 }
 
 static int

plugins/sudoers/sudoers.h

@@ -215,9 +215,9 @@ void remove_timestamp(bool);
 bool user_is_exempt(void);
 
 /* sudo_auth.c */
-int verify_user(struct passwd *, char *);
+int verify_user(struct passwd *pw, char *prompt);
-int sudo_auth_begin_session(struct passwd *);
+int sudo_auth_begin_session(struct passwd *pw, char **user_env[]);
-int sudo_auth_end_session(struct passwd *);
+int sudo_auth_end_session(struct passwd *pw);
 int sudo_auth_init(struct passwd *pw);
 int sudo_auth_cleanup(struct passwd *pw);
 
@@ -304,6 +304,7 @@ char *expand_iolog_path(const char *prefix, const char *dir, const char *file,
 
 /* env.c */
 char **env_get(void);
+void env_merge(char * const envp[], bool overwrite);
 void env_init(char * const envp[]);
 void init_envtables(void);
 void insert_env_vars(char * const envp[]);

src/hooks.c

@@ -278,29 +278,3 @@ deregister_hook(struct sudo_hook *hook)
 
     debug_return_int(rval);
 }
-
-/* Deregister all environment handling hooks. */
-void
-deregister_env_hooks(void)
-{
-    struct sudo_hook_list *tofree;
-    debug_decl(deregister_env_hooks, SUDO_DEBUG_HOOKS)
-
-    while ((tofree = sudo_hook_setenv_list) != NULL) {
-	sudo_hook_setenv_list = sudo_hook_setenv_list->next;
-	efree(tofree);
-    }
-    while ((tofree = sudo_hook_unsetenv_list) != NULL) {
-	sudo_hook_unsetenv_list = sudo_hook_unsetenv_list->next;
-	efree(tofree);
-    }
-    while ((tofree = sudo_hook_getenv_list) != NULL) {
-	sudo_hook_getenv_list = sudo_hook_getenv_list->next;
-	efree(tofree);
-    }
-    while ((tofree = sudo_hook_putenv_list) != NULL) {
-	sudo_hook_putenv_list = sudo_hook_putenv_list->next;
-	efree(tofree);
-    }
-    debug_return;
-}

src/sudo.c

@@ -130,7 +130,7 @@ static int policy_list(struct plugin_container *plugin, int argc,
 static int policy_validate(struct plugin_container *plugin);
 static void policy_invalidate(struct plugin_container *plugin, int remove);
 static int policy_init_session(struct plugin_container *plugin,
-    struct passwd *pwd);
+    struct passwd *pwd, char **user_env[]);
 
 /* I/O log plugin convenience functions. */
 static int iolog_open(struct plugin_container *plugin, char * const settings[],
@@ -148,8 +148,6 @@ static struct rlimit corelimit;
 static struct rlimit nproclimit;
 #endif
 
-extern char **environ;
-
 int
 main(int argc, char *argv[], char *envp[])
 {
@@ -285,9 +283,6 @@ main(int argc, char *argv[], char *envp[])
 			plugin->name);
 		}
 	    }
-	    /* Now that we have the command's environment, disable env hooks. */
-	    deregister_env_hooks();
-
 	    /* Setup command details and run command/edit. */
 	    command_info_to_details(command_info, &command_details);
 	    command_details.argv = argv_out;
@@ -850,17 +845,11 @@ exec_setup(struct command_details *details, const char *ptyname, int ptyfd)
     aix_restoreauthdb();
 #endif
 
-    /*
-     * Swap in the plugin-supplied environment in case session init
-     * modifies the environment.  This is kind of a hack.
-     */
-    environ = details->envp;
-
     /*
      * Call policy plugin's session init before other setup occurs.
      * The session init code is expected to print an error as needed.
      */
-    if (policy_init_session(&policy_plugin, pw) != true)
+    if (policy_init_session(&policy_plugin, pw, &details->envp) != true)
 	goto done;
 
 #ifdef HAVE_SELINUX
@@ -912,9 +901,6 @@ exec_setup(struct command_details *details, const char *ptyname, int ptyfd)
 #endif /* HAVE_LOGIN_CAP_H */
     }
 
-    /* Update the environment pointer in command details */
-    details->envp = environ;
-
     /*
      * Set groups, including supplementary group vector.
      */
@@ -1148,12 +1134,25 @@ policy_invalidate(struct plugin_container *plugin, int remove)
 }
 
 static int
-policy_init_session(struct plugin_container *plugin, struct passwd *pwd)
+policy_init_session(struct plugin_container *plugin, struct passwd *pwd, char **user_env[])
 {
+    int rval = true;
     debug_decl(policy_init_session, SUDO_DEBUG_PCOMM)
-    if (plugin->u.policy->init_session)
+
-	debug_return_bool(plugin->u.policy->init_session(pwd));
+    if (plugin->u.policy->init_session) {
-    debug_return_bool(true);
+	/*
+	 * Backwards compatibility for older API versions
+	 */
+	switch (plugin->u.generic->version) {
+	case SUDO_API_MKVERSION(1, 0):
+	case SUDO_API_MKVERSION(1, 1):
+	    rval = plugin->u.policy_1_0->init_session(pwd);
+	    break;
+	default:
+	    rval = plugin->u.policy->init_session(pwd, user_env);
+	}
+    }
+    debug_return_bool(rval);
 }
 
 static int

src/sudo.h

@@ -231,7 +231,6 @@ int process_hooks_getenv(const char *name, char **val);
 int process_hooks_setenv(const char *name, const char *value, int overwrite);
 int process_hooks_putenv(char *string);
 int process_hooks_unsetenv(const char *name);
-void deregister_env_hooks(void);
 
 /* interfaces.c */
 int get_net_ifs(char **addrinfo);