Matthias Drochner wrote:
> static int
> askhelper(const char *user, const char *pass)
> {
> 	int fd[2];
> 	pid_t pid, rpid;
> 	int res, pwlen, s;
> 
> 	res = pipe(fd);
> 	if (res < 0)
> 		return (errno);

You could use socketpair() with AF_LOCAL instead which would allow checking
credentials.

> 	pid = fork();
> 	if (pid == -1)
> 		return (errno);
> 	if (pid == 0) {
> 		dup2(fd[0], 0);
> 		close(fd[0]);
> 		close(fd[1]);
> 		execl(PATH_HELPER, PATH_HELPER, user, NULL);
> 		_exit(errno);
> 	}
> 
> 	close(fd[0]);
> 
> 	pwlen = strlen(pass);
> 	res = write(fd[1], pass, pwlen);

Wouldn't it be better to include the terminating NUL character, so
that the receiver can be absolutely sure about the length?

> 	if (res != pwlen)
> 		return (EIO);
> 
> 	close(fd[1]);
> 
> 	rpid = waitpid(pid, &s, 0);
> 	if (rpid != pid)
> 		return (errno);
> 	if (WEXITSTATUS(s))
> 		return (WEXITSTATUS(s));
> 
> 	return (0);
> }
> 
> PAM_EXTERN int
> pam_sm_authenticate(pam_handle_t *pamh, int flags,
> 		    int argc, const char ** argv)
> {
> 	const char *user, *pass;
> 	int res;
> 
> 	res = pam_get_user(pamh, &user, NULL);
> 	if (res != PAM_SUCCESS)
> 		return (res);
> 	res = pam_get_authtok(pamh, PAM_AUTHTOK, &pass, NULL);
> 	if (res != PAM_SUCCESS)
> 		return (res);
> 
> 	if (askhelper(user, pass) != 0)
> 		return (PAM_AUTH_ERR);
> 
> 	return (PAM_SUCCESS);
> }
> 
> PAM_MODULE_ENTRY("pam_passwdhelper");

> Content-Description: helper.c
> #include <pwd.h>
> #include <string.h>
> #include <errno.h>
> #include <unistd.h>
> 
> static char pwbuf[1024];
> 
> int
> main(int argc, char **argv)
> {
> 	struct passwd *pwent;
> 	int res;
> 	char *bufptr;
> 	size_t buflen;
> 
> 	if (argc != 2)
> 		return (EINVAL);
> 
> 	bufptr = pwbuf;
> 	buflen = sizeof(pwbuf);
> 	do {
> 		res = read(0, bufptr, buflen);

STDIN_FILENO is more readable.

> 		if (res < 0)
> 			return (errno);
> 		bufptr += res;
> 		buflen -= res;
> 	} while (res > 0 && buflen > 0);
> 	if (buflen == 0)
> 		return (ENOMEM);
> 
> 	pwent = getpwnam(argv[1]);
> 	if (!pwent || (pwent->pw_uid != getuid()))
> 		return (EPERM);
> 
> 	if (strcmp(crypt(pwbuf, pwent->pw_passwd), pwent->pw_passwd) == 0)
> 		return (0);

Since crypt(3) says NULL is returned on failure, it wouldn't hurt to check
for NULL.

pwbuf is/must be NUL-terminated?
 
> 	return (EAUTH);
> }

-- 
Christian