pam_ssh

I think that's fixed on trunk.
2006-08-03 Wolfgang Rosenauer
* pam_ssh.c: fixed some debug output, create the agent file
writable for the user to be able to recover from a system crash
and fixed a missing credential restore which caused pam to fail
for following modules in error case

That change is old but just landed on trunk here.

Logged In: YES
user_id=385092

this bug has been fixed into Debian's package. Andrew, how
can i send you a patch?

Logged In: YES
user_id=385092

Andrew has not responded for a while... I will integrate the
patch into the package via dpatch until upstream integration.

Logged In: YES
user_id=78386

Aurelien:
you'll get the DIFF via Debian BTS. I followed up on the
relevant bug (this is an upstream issue though).

Logged In: YES
user_id=78386

I would gladly, if I could find where to attach files.

Logged In: YES
user_id=385092

I can't apply it to my source tree. Could you post-it as an
attached diff file? there may be problems due to spaces and
line wrapping

Thanks,

Aurelien

Logged In: YES
user_id=78386

Try this one:

--- libpam-ssh-1.91.0.orig/pam_ssh.c 2004-04-12
08:55:08.000000000 -0500
+++ libpam-ssh-1.91.0/pam_ssh.c 2005-03-30
15:36:26.266041056 -0600
@@ -279,9 +279,8 @@
*/

static int
-add_keys(pam_handle_t *pamh, char *socket)
+add_keys(pam_handle_t *pamh, AuthenticationConnection *ac)
{
- AuthenticationConnection *ac; /* connection to ssh-agent */
char *comment; /* private key comment */
char *data_name; /* PAM state */
int final; /* final return value */
@@ -289,13 +288,6 @@
Key *key; /* user's private key */
int retval; /* from calls */

- /* connect to the agent */
-
- if (!(ac = ssh_get_authentication_connection(socket))) {
- pam_ssh_log(LOG_ERR, "%s: %m", socket);
- return PAM_SESSION_ERR;
- }
-
/* hand off each private key to the agent */

final = 0;
@@ -324,11 +316,177 @@
if (!final)
final = retval;
}
- ssh_close_authentication_connection(ac);

return final ? PAM_SUCCESS : PAM_SESSION_ERR;
}

+static int
+start_ssh_agent(pam_handle_t *pamh, uid_t uid, FILE **env_read)
+{
+ pid_t child_pid; /* child process that spawns agent */
+ int child_pipe[2]; /* pipe to child process */
+ int child_status; /* child process status */
+ char *arg[3], *env[1]; /* to pass to execve() */
+
+ if (pipe(child_pipe) < 0) {
+ pam_ssh_log(LOG_ERR, "pipe: %m");
+ return PAM_SERVICE_ERR;
+ }
+ switch (child_pid = fork()) {
+ case -1: /* error */
+ pam_ssh_log(LOG_ERR, "fork: %m");
+ close(child_pipe[0]);
+ close(child_pipe[1]);
+ return PAM_SERVICE_ERR;
+ /* NOTREACHED */
+ case 0: /* child */
+
+ /* Permanently drop privileges using setuid()
+ before executing ssh-agent so that root
+ privileges can't possibly be regained (some
+ ssh-agents insist that euid == ruid
+ anyway). System V won't let us use
+ setuid() unless euid == 0, so we
+ temporarily regain root privileges first
+ with openpam_restore_cred() (which calls
+ seteuid()). */
+
+ switch (openpam_restore_cred(pamh)) {
+ case PAM_SYSTEM_ERR:
+ pam_ssh_log(LOG_ERR,
+ "can't restore privileges: %m");
+ _exit(EX_OSERR);
+ /* NOTREACHED */
+ case PAM_SUCCESS:
+ if (setuid(uid) == -1) {
+ pam_ssh_log(LOG_ERR,
+ "can't drop privileges: %m",
+ uid);
+ _exit(EX_NOPERM);
+ }
+ break;
+ }
+
+ if (close(child_pipe[0]) == -1) {
+ pam_ssh_log(LOG_ERR, "close: %m");
+ _exit(EX_OSERR);
+ }
+ if (child_pipe[1] != STDOUT_FILENO) {
+ if (dup2(child_pipe[1], STDOUT_FILENO) == -1) {
+ pam_ssh_log(LOG_ERR, "dup: %m");
+ _exit(EX_OSERR);
+ }
+ if (close(child_pipe[1]) == -1) {
+ pam_ssh_log(LOG_ERR, "close: %m");
+ _exit(EX_OSERR);
+ }
+ }
+ arg[0] = "ssh-agent";
+ arg[1] = "-s";
+ arg[2] = NULL;
+ env[0] = NULL;
+ execve(PATH_SSH_AGENT, arg, env);
+ pam_ssh_log(LOG_ERR, "%s: %m", PATH_SSH_AGENT);
+ _exit(127);
+ /* NOTREACHED */
+ }
+ if (close(child_pipe[1]) == -1) {
+ pam_ssh_log(LOG_ERR, "close: %m");
+ return PAM_SESSION_ERR;
+ }
+ if (!(*env_read = fdopen(child_pipe[0], "r"))) {
+ pam_ssh_log(LOG_ERR, "%s: %m", PATH_SSH_AGENT);
+ return PAM_SESSION_ERR;
+ }
+
+ child_status = 0;
+ if (waitpid_intr(child_pid, &child_status, 0) == -1 &&
+ errno != ECHILD) {
+ pam_ssh_log(LOG_ERR, "%s: %m", PATH_SSH_AGENT);
+ return PAM_SESSION_ERR;
+ }
+
+ if (child_status != 0) {
+ if (WIFSIGNALED(child_status))
+ pam_ssh_log(LOG_ERR, "%s exited on signal %d",
+ PATH_SSH_AGENT, WTERMSIG(child_status));
+ else
+ if (WEXITSTATUS(child_status) == 127)
+ pam_ssh_log(LOG_ERR,
+ "cannot execute %s",
+ PATH_SSH_AGENT);
+ else
+ pam_ssh_log(LOG_ERR,
+ "%s exited with status %d",
+ PATH_SSH_AGENT,
+ WEXITSTATUS(child_status));
+ return PAM_SESSION_ERR;
+ }
+
+ return PAM_SUCCESS;
+}
+
+static int
+read_write_agent_env(pam_handle_t *pamh,
+ FILE *env_read,
+ int env_write,
+ char **agent_socket)
+{
+ char *agent_pid; /* copy of agent PID */
+ char *env_end; /* end of env */
+ char env_string[BUFSIZ]; /* environment string */
+ char *env_value; /* envariable value */
+ int retval; /* from calls */
+
+ while (fgets(env_string, sizeof env_string, env_read)) {
+
+ /* parse environment definitions */
+
+ if (env_write >= 0)
+ write(env_write, env_string, strlen(env_string));
+ if (!(env_value = strchr(env_string, '=')) ||
+ !(env_end = strchr(env_value, ';')))
+ continue;
+ *env_end = '\0';
+
+ /* pass to the application */
+
+ if ((retval = pam_putenv(pamh, env_string)) != PAM_SUCCESS)
+ return retval;
+
+ *env_value++ = '\0';
+
+ /* save the agent socket so we can connect to it and add
+ the keys as well as the PID so we can
kill the agent on
+ session close. */
+
+ agent_pid = NULL;
+ if (strcmp(&env_string[strlen(env_string) -
+ strlen(ENV_SOCKET_SUFFIX)], ENV_SOCKET_SUFFIX) == 0 &&
+ !(*agent_socket = strdup(env_value))) {
+ pam_ssh_log(LOG_CRIT, "out of memory");
+ return PAM_SERVICE_ERR;
+ } else if (strcmp(&env_string[strlen(env_string) -
+ strlen(ENV_PID_SUFFIX)], ENV_PID_SUFFIX) == 0 &&
+ (!(agent_pid = strdup(env_value)) ||
+ (retval = pam_set_data(pamh, "ssh_agent_pid",
+ agent_pid, ssh_cleanup)) != PAM_SUCCESS)) {
+ if (agent_pid)
+ free(agent_pid);
+ else {
+ pam_ssh_log(LOG_CRIT, "out of memory");
+ return PAM_SERVICE_ERR;
+ }
+ if (agent_socket)
+ free(agent_socket);
+ return retval;
+ }
+
+ }
+
+ return PAM_SUCCESS;
+}
+

PAM_EXTERN int
pam_sm_authenticate(pam_handle_t *pamh, int flags __unused,
int argc,
@@ -494,17 +652,10 @@
pam_sm_open_session(pam_handle_t *pamh, int flags __unused,
int argc __unused, const char **argv __unused)
{
- char *agent_pid; /* copy of agent PID */
+ AuthenticationConnection *ac; /* connection to ssh-agent */
char *agent_socket; /* agent socket */
- char *arg[3], *env[1]; /* to pass to execve() */
- pid_t child_pid; /* child process that spawns agent */
- int child_pipe[2]; /* pipe to child process */
- int child_status; /* child process status */
char *cp; /* scratch */
- char *env_end; /* end of env */
FILE *env_read; /* env data source */
- char env_string[BUFSIZ]; /* environment string */
- char *env_value; /* envariable value */
int env_write; /* env file descriptor */
char hname[MAXHOSTNAMELEN]; /* local hostname */
int no_link; /* link per-agent file? */
@@ -515,6 +666,7 @@
int start_agent; /* start agent? */
const char *tty_raw; /* raw tty or display name */
char *tty_nodir; /* tty without / chars */
+ int attempt; /* No. of attempt to contact agent */

log_init(MODULE_NAME, SYSLOG_LEVEL_ERROR,
SYSLOG_FACILITY_AUTHPRIV, 0);

@@ -568,215 +720,70 @@
per-session filename later. Start the agent if
we can't open
the file for reading. */

- env_write = child_pid = no_link = start_agent = 0;
- env_read = NULL;
- if ((env_write = open(per_agent, O_CREAT | O_EXCL |
O_WRONLY, S_IRUSR))
- < 0 && !(env_read = fopen(per_agent, "r")))
- no_link = 1;
- if (!env_read) {
- start_agent = 1;
- if (pipe(child_pipe) < 0) {
- pam_ssh_log(LOG_ERR, "pipe: %m");
- close(env_write);
- openpam_restore_cred(pamh);
- return PAM_SERVICE_ERR;
- }
- switch (child_pid = fork()) {
- case -1: /* error */
- pam_ssh_log(LOG_ERR, "fork: %m");
- close(child_pipe[0]);
- close(child_pipe[1]);
- close(env_write);
- openpam_restore_cred(pamh);
- return PAM_SERVICE_ERR;
- /* NOTREACHED */
- case 0: /* child */
-
- /* Permanently drop privileges using setuid()
- before executing ssh-agent so that root
- privileges can't possibly be regained (some
- ssh-agents insist that euid == ruid
- anyway). System V won't let us use
- setuid() unless euid == 0, so we
- temporarily regain root privileges first
- with openpam_restore_cred() (which calls
- seteuid()). */
-
- switch (openpam_restore_cred(pamh)) {
- case PAM_SYSTEM_ERR:
- pam_ssh_log(LOG_ERR,
- "can't restore privileges: %m");
- _exit(EX_OSERR);
- /* NOTREACHED */
- case PAM_SUCCESS:
- if (setuid(pwent->pw_uid) == -1) {
- pam_ssh_log(LOG_ERR,
- "can't drop privileges: %m",
- pwent->pw_uid);
- _exit(EX_NOPERM);
- }
- break;
- }
-
- if (close(child_pipe[0]) == -1) {
- pam_ssh_log(LOG_ERR, "close: %m");
- _exit(EX_OSERR);
- }
- if (child_pipe[1] != STDOUT_FILENO) {
- if (dup2(child_pipe[1], STDOUT_FILENO) == -1) {
- pam_ssh_log(LOG_ERR, "dup: %m");
- _exit(EX_OSERR);
- }
- if (close(child_pipe[1]) == -1) {
- pam_ssh_log(LOG_ERR, "close: %m");
- _exit(EX_OSERR);
- }
+ for ( attempt = 0; attempt < 2; ++attempt ) {
+ env_write = no_link = start_agent = 0;
+ env_read = NULL;
+ if ((env_write = open(per_agent, O_CREAT | O_EXCL |
O_WRONLY, S_IRUSR))
+ < 0 && !(env_read = fopen(per_agent, "r")))
+ no_link = 1;
+ if (!env_read) {
+ start_agent = 1;
+ if ((retval = start_ssh_agent(pamh, pwent->pw_uid,
&env_read))
+ != PAM_SUCCESS) {
+ close(env_write);
+ openpam_restore_cred(pamh);
+ return retval;
}
- arg[0] = "ssh-agent";
- arg[1] = "-s";
- arg[2] = NULL;
- env[0] = NULL;
- execve(PATH_SSH_AGENT, arg, env);
- pam_ssh_log(LOG_ERR, "%s: %m", PATH_SSH_AGENT);
- _exit(127);
- /* NOTREACHED */
- }
- if (close(child_pipe[1]) == -1) {
- pam_ssh_log(LOG_ERR, "close: %m");
- openpam_restore_cred(pamh);
- return PAM_SESSION_ERR;
- }
- if (!(env_read = fdopen(child_pipe[0], "r"))) {
- pam_ssh_log(LOG_ERR, "%s: %m", PATH_SSH_AGENT);
- close(env_write);
- openpam_restore_cred(pamh);
- return PAM_SESSION_ERR;
- }
- }
-
- /* save environment for application with pam_putenv() */
-
- agent_socket = NULL;
- while (fgets(env_string, sizeof env_string, env_read)) {
-
- /* parse environment definitions */
-
- if (env_write >= 0)
- write(env_write, env_string, strlen(env_string));
- if (!(env_value = strchr(env_string, '=')) ||
- !(env_end = strchr(env_value, ';')))
- continue;
- *env_end = '\0';
-
- /* pass to the application */
-
- if ((retval = pam_putenv(pamh, env_string)) != PAM_SUCCESS) {
- fclose(env_read);
- if (start_agent)
- waitpid_intr(child_pid, &child_status, 0);
- close(env_write);
- if (agent_socket)
- free(agent_socket);
- openpam_restore_cred(pamh);
- return retval;
}

- *env_value++ = '\0';
-
- /* save the agent socket so we can connect to it and add
- the keys as well as the PID so we can
kill the agent on
- session close. */
-
- agent_pid = NULL;
- if (strcmp(&env_string[strlen(env_string) -
- strlen(ENV_SOCKET_SUFFIX)], ENV_SOCKET_SUFFIX) == 0 &&
- !(agent_socket = strdup(env_value))) {
- pam_ssh_log(LOG_CRIT, "out of memory");
- fclose(env_read);
- if (start_agent)
- waitpid_intr(child_pid, &child_status, 0);
- close(env_write);
+ agent_socket = NULL;
+ retval = read_write_agent_env(pamh, env_read, env_write,
&agent_socket);
+ close(env_write);
+ if (retval != PAM_SUCCESS) {
if (agent_socket)
free(agent_socket);
- openpam_restore_cred(pamh);
- return PAM_SERVICE_ERR;
- } else if (strcmp(&env_string[strlen(env_string) -
- strlen(ENV_PID_SUFFIX)], ENV_PID_SUFFIX) == 0 &&
- (!(agent_pid = strdup(env_value)) ||
- (retval = pam_set_data(pamh, "ssh_agent_pid",
- agent_pid, ssh_cleanup)) != PAM_SUCCESS)) {
fclose(env_read);
- if (start_agent)
- waitpid_intr(child_pid, &child_status, 0);
- close(env_write);
- if (agent_pid)
- free(agent_pid);
- else {
- pam_ssh_log(LOG_CRIT, "out of memory");
- openpam_restore_cred(pamh);
- return PAM_SERVICE_ERR;
- }
- if (agent_socket)
- free(agent_socket);
openpam_restore_cred(pamh);
return retval;
}

- }
- close(env_write);
-
- if (fclose(env_read) != 0) {
- pam_ssh_log(LOG_ERR, "fclose: %m");
- openpam_restore_cred(pamh);
- return PAM_SESSION_ERR;
- }
-
- if (start_agent) {
-
- /* Ignore ECHILD in case a SIGCHLD handler is installed. */
-
- child_status = 0;
- if (waitpid_intr(child_pid, &child_status, 0) == -1 &&
- errno != ECHILD) {
- pam_ssh_log(LOG_ERR, "%s: %m", PATH_SSH_AGENT);
+ if (fclose(env_read) != 0) {
+ pam_ssh_log(LOG_ERR, "fclose: %m");
if (agent_socket)
free(agent_socket);
openpam_restore_cred(pamh);
return PAM_SESSION_ERR;
}

- if (child_status != 0) {
- if (WIFSIGNALED(child_status))
- pam_ssh_log(LOG_ERR, "%s exited on signal %d",
- PATH_SSH_AGENT, WTERMSIG(child_status));
- else
- if (WEXITSTATUS(retval) == 127)
- pam_ssh_log(LOG_ERR,
- "cannot execute %s",
- PATH_SSH_AGENT);
- else
- pam_ssh_log(LOG_ERR,
- "%s exited with status %d",
- PATH_SSH_AGENT,
- WEXITSTATUS(child_status));
- if (agent_socket)
- free(agent_socket);
+ if (!agent_socket) {
openpam_restore_cred(pamh);
return PAM_SESSION_ERR;
}
+
+ ac = ssh_get_authentication_connection(agent_socket);
+ if (ac) {
+ free(agent_socket);
+ break;
+ }
+ pam_ssh_log(LOG_ERR, "%s: %m", agent_socket);
+ free(agent_socket);
+ if (start_agent)
+ break;
+ unlink(per_agent);
}

- if (!agent_socket) {
- openpam_restore_cred(pamh);
+ if (!ac)
return PAM_SESSION_ERR;
- }

- if (start_agent && (retval = add_keys(pamh, agent_socket))
- != PAM_SUCCESS) {
+ if (start_agent)
+ retval = add_keys(pamh, ac);
+
+ ssh_close_authentication_connection(ac);
+
+ if (start_agent && retval != PAM_SUCCESS) {
openpam_restore_cred(pamh);
return retval;
}
- free(agent_socket);

/* if we couldn't access the per-agent file, don't link a
per-session filename to it */

Logged In: YES
user_id=385092

I'm trying to write something. Andrew, could you re-read it?
I'm so far to be C guru...