Skip to content

Commit

Permalink
Don't use pam environment in setcred
Browse files Browse the repository at this point in the history
At least Arch runs pam_env from system-auth without the user_readenv
option, so ~/.pam_environment is ignored there. Therefore,
gpg-connect-agent was called with a mostly empty environment, breaking
the setup of users who override XDG_CONFIG_HOME or GNUPGHOME (e.g. #23).

I assume that in most relevant cases, we are either opening a new
session and should use the pam environment, or we are just
authenticating within an existing session, where the env is already set
up in the calling process and should be reused. Therefore, we now do not
use the pam environment during setcred anymore.

It may turn out to be necessary to add another option to make this
configurable.
  • Loading branch information
cruegge committed Nov 14, 2020
1 parent b1e4044 commit 6f95269
Showing 1 changed file with 14 additions and 7 deletions.
21 changes: 14 additions & 7 deletions src/pam_gnupg.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ void cleanup_token(pam_handle_t *pamh, void *data, int error_status) {
wipestr(data);
}

bool preset_passphrase(pam_handle_t *pamh, const char *tok, bool autostart) {
bool preset_passphrase(pam_handle_t *pamh, const char *tok, bool autostart, bool send_env) {
const char *user = NULL;
if (pam_get_user(pamh, &user, NULL) != PAM_SUCCESS || user == NULL) {
return false;
Expand All @@ -65,6 +65,16 @@ bool preset_passphrase(pam_handle_t *pamh, const char *tok, bool autostart) {
return false;
}

// pam_getenvlist() allocates, so we can't call it after fork().
char **env = NULL;
if (send_env) {
env = pam_getenvlist(pamh);
if (env == NULL) {
pam_syslog(pamh, LOG_ERR, "failed to read pam environment");
return false;
}
}

// Reset SIGCHLD handler so we can use waitpid(). If the calling process
// used a handler to manage its own child processes, and one of the
// children exits while we're busy, things will probably break, but there
Expand All @@ -78,9 +88,6 @@ bool preset_passphrase(pam_handle_t *pamh, const char *tok, bool autostart) {
sa.sa_flags = 0;
sigaction(SIGCHLD, &sa, &saved_sigchld);

// pam_getenvlist() allocates, so we can't call it after fork().
char **env = pam_getenvlist(pamh);

bool ret = true;

pid_t pid = fork();
Expand Down Expand Up @@ -121,7 +128,7 @@ bool preset_passphrase(pam_handle_t *pamh, const char *tok, bool autostart) {
if (!autostart) {
cmd[1] = NULL;
}
if (env != NULL) {
if (send_env) {
execve(cmd[0], cmd, env);
} else {
execv(cmd[0], cmd);
Expand Down Expand Up @@ -193,7 +200,7 @@ int pam_sm_setcred(pam_handle_t *pamh, int flags, int argc, const char **argv) {
return PAM_SUCCESS;
}
}
if (!preset_passphrase(pamh, tok, false)) {
if (!preset_passphrase(pamh, tok, false, false)) {
return PAM_IGNORE;
}
pam_set_data(pamh, TOKEN_DATA_NAME, NULL, NULL);
Expand All @@ -212,7 +219,7 @@ int pam_sm_open_session(pam_handle_t *pamh, int flags, int argc, const char **ar
if (pam_get_data(pamh, TOKEN_DATA_NAME, (const void **) &tok) == PAM_SUCCESS
&& tok != NULL
) {
if (!preset_passphrase(pamh, tok, autostart)) {
if (!preset_passphrase(pamh, tok, autostart, true)) {
ret = PAM_SESSION_ERR;
}
pam_set_data(pamh, TOKEN_DATA_NAME, NULL, NULL);
Expand Down

0 comments on commit 6f95269

Please sign in to comment.