Use fewer buffers and reduce string-copying

BSDmakefile

@@ -1,8 +1,11 @@
-LIB= pam_multipassword
+SHLIB_NAME= pam_multipassword.so
 SRCS= pam_duress.c
 
-CFLAGS+=-DHASH_ROUNDS=1000
+CFLAGS+=-DHASH_ROUNDS=1000 -DDB_PATH='"/var/db/multipassword"'
 
-WARNS= 7
+WARNS= 9
+
+DPADD= ${LIBCRYPTO}
+LDADD= -lcrypto
 
 .include <bsd.lib.mk>

pam_duress.c

@@ -10,13 +10,18 @@
 #include <security/pam_ext.h>
 #endif
 #include <string.h>
+#include <syslog.h>
 #include <openssl/sha.h>
 #include <openssl/evp.h>
 #include <math.h>
 
 #define byte unsigned char
 #define INFINITE_LOOP_BOUND 1000000000
-#define PATH_PREFIX "/usr/share/duress/actions/"
+#ifndef DB_PATH
+# define DB_PATH "/usr/share/duress"
+#endif
+#define PATH_PREFIX DB_PATH "/actions"
+#define HASHES_PATH DB_PATH "/hashes"
 #define SALT_SIZE 16
 
 #ifndef __unused
@@ -48,24 +53,35 @@ sha256hash(const char* plaintext, byte* output)
 static void
 pbkdf2hash(const char* pass, const char* salt, byte* output)
 {
- PKCS5_PBKDF2_HMAC(pass, strlen(pass), salt, strlen(salt), HASH_ROUNDS, EVP_sha256(), 32, output);
+ PKCS5_PBKDF2_HMAC(pass, strlen(pass), (const byte *)salt, strlen(salt),
+ HASH_ROUNDS, EVP_sha256(), 32, output);
 }
 
-static void
-decrypt(const char *input, const char *output, const char *pass, const byte *salt)
+static int
+decrypt(const char *input, int ofd, const char *pass, const byte *salt)
 {
- FILE *in=fopen(input, "rb"), *out=fopen(output, "wb");
- fseek(in, sizeof(byte)*16, SEEK_SET);
+ FILE *in, *out;
  byte inbuf[1024], outbuf[1024 + EVP_MAX_BLOCK_LENGTH];
  int inlen, outlen;
  EVP_CIPHER_CTX ctx;
  byte key[32], iv[32];
  const EVP_CIPHER *cipher;
  const EVP_MD *dgst = NULL;
 
+ in = fopen(input, "rb");
+ if (in == NULL || fseek(in, sizeof(byte)*16, SEEK_SET) == -1) {
+ syslog(LOG_AUTH|LOG_WARNING, "%s: %m", input);
+ return PAM_NO_MODULE_DATA;
+ }
+ out = fdopen(ofd, "wb");
+ if (out == NULL) {
+ syslog(LOG_AUTH|LOG_WARNING, "manipulating temporary action-file: %m");
+ return PAM_SYSTEM_ERR;
+ }
+
  cipher = EVP_aes_256_cbc();
  dgst = EVP_sha256();
- EVP_BytesToKey(cipher, dgst, (const byte *)salt, (byte *) pass, strlen(pass), 1, key, iv);
+ EVP_BytesToKey(cipher, dgst, salt, (const byte *)pass, strlen(pass), 1, key, iv);
 
  EVP_CIPHER_CTX_init(&ctx);
 
@@ -75,45 +91,49 @@ decrypt(const char *input, const char *output, const char *pass, const byte *sal
  {
  if(!EVP_DecryptUpdate(&ctx, outbuf, &outlen, inbuf, inlen))
  {
- fprintf(stderr, "Error in action decryption!\n");
+ syslog(LOG_AUTH|LOG_WARNING, "Error decrypting %s", input);
  EVP_CIPHER_CTX_cleanup(&ctx);
  fclose(in);
  fclose(out);
- return;
+ return PAM_AUTHTOK_ERR;
  }
  fwrite(outbuf, 1, outlen, out);
  }
 
  if(!EVP_DecryptFinal_ex(&ctx, outbuf, &outlen))
  {
- fprintf(stderr, "Error in action decryption!\n");
+ syslog(LOG_AUTH|LOG_WARNING, "Error finalizing decryption of %s", input);
  EVP_CIPHER_CTX_cleanup(&ctx);
  fclose(in);
  fclose(out);
- return;
+ return PAM_AUTHTOK_ERR;
  }
 
  fwrite(outbuf, 1, outlen, out);
  EVP_CIPHER_CTX_cleanup(&ctx);
  fclose(in);
  fclose(out);
+ return PAM_SUCCESS;
 }
 
 static void
 appendHashToPath(const byte* hexes, char* output)
 {
- char hash[2*SHA256_DIGEST_LENGTH + 1];
- byte2string(hexes, hash);
- sprintf(output, "%s%s", output, hash);
+ byte2string(hexes, output + strlen(output));
 }
 
 static int
 duressExistsInDatabase(const char *concat, byte *hashin)
 {
  int cntr = 0;
  char salt[SALT_SIZE+1], givenhash[SHA256_DIGEST_LENGTH*2 + 1], hashfromfile[SHA256_DIGEST_LENGTH*2 + 1];
+ FILE *hashes=fopen(HASHES_PATH, "r");
+
+ if (hashes == NULL) {
+ syslog(LOG_AUTH|LOG_ERR, "%s: %m", HASHES_PATH);
+ return 0;
+ }
 
- FILE*hashes=fopen("/usr/share/duress/hashes", "r");
  while(fscanf(hashes, "%16s:%64s\n", salt, hashfromfile) != EOF && cntr < INFINITE_LOOP_BOUND)
  {
  pbkdf2hash(concat, salt, hashin);
@@ -136,8 +156,10 @@ readSalt(byte *salt, const char *path)
 {
  FILE *in = fopen(path, "r");
 
- fseek(in, sizeof(byte)*8, SEEK_SET);
- fread(salt, 8, 1, in);
+ if (in == NULL ||
+ fseek(in, sizeof(byte)*8, SEEK_SET) == -1 ||
+ fread(salt, 8, 1, in) != 1)
+ syslog(LOG_AUTH|LOG_WARNING, "Reading salt from %s: %m", path);
 
  fclose(in);
 }
@@ -146,57 +168,77 @@ PAM_EXTERN int
 pam_sm_authenticate(pam_handle_t *pamh, int flags __unused, int argc, const char **argv)
 {
  int retval, pam_retval;
+
  if(argc != 1)
  {
- fprintf(stderr, "Problem in pam_duress installation! Please add exactly one argument after the duress module!\n");
- return PAM_AUTH_ERR;
+ syslog(LOG_AUTH|LOG_ERR, "Please use exactly one argument with %s, not %d", __FILE__, argc);
+ return PAM_NO_MODULE_DATA;
  }
 
  if(strcmp(argv[0], "disallow") == 0)
- pam_retval = PAM_AUTH_ERR;
+ pam_retval = PAM_PERM_DENIED;
  else if(strcmp(argv[0], "allow") == 0)
  pam_retval = PAM_SUCCESS;
  else
  {
- fprintf(stderr, "Unknown argument in pam_duress module!\n");
- return PAM_AUTH_ERR;
+ syslog(LOG_AUTH|LOG_ERR, "Unknown argument `%s' given to %s", argv[0], __FILE__);
+ return PAM_NO_MODULE_DATA;
  }
 
  const char *token, *user;
- retval = pam_get_authtok(pamh, PAM_AUTHTOK, &token, "Enter password: ");
+ retval = pam_get_authtok(pamh, PAM_AUTHTOK, &token, NULL);
  if(retval != PAM_SUCCESS)
  return retval;
- retval = pam_get_user(pamh, &user, "Enter username: ");
+ retval = pam_get_user(pamh, &user, NULL);
  if(retval != PAM_SUCCESS)
  return retval;
 
- byte userhash[SHA256_DIGEST_LENGTH];
- sha256hash(user, userhash);
- char userhsh[SHA256_DIGEST_LENGTH*2 + 1];
+ byte hashin[SHA256_DIGEST_LENGTH];
+ char concat[2*SHA256_DIGEST_LENGTH + strlen(token) + 1];
+ sha256hash(user, hashin);
 
- byte2string(userhash, userhsh);
+ byte2string(hashin, concat);
+ strcpy(concat + 2*SHA256_DIGEST_LENGTH, token);
 
- char concat[2*SHA256_DIGEST_LENGTH + strlen(token) + 1];
- sprintf(concat, "%s%s", userhsh, token);
- byte hashin[SHA256_DIGEST_LENGTH];
+ if (duressExistsInDatabase(concat, hashin) == 0)
+ return PAM_AUTHINFO_UNAVAIL;
 
- if(duressExistsInDatabase(concat, hashin)==1)
- {
- byte salt[8];
- char path[strlen(PATH_PREFIX) + 2*SHA256_DIGEST_LENGTH + 1];
- sprintf(path, PATH_PREFIX);
- appendHashToPath(hashin, path);
- readSalt(salt, path);
- decrypt(path, "/tmp/action", token, salt);
- chmod("/tmp/action", strtol("0544", 0, 8));
- pid_t pid=fork();
- if(pid==0)
- {
- execl("/tmp/action", "action", NULL, NULL);
- unlink("/tmp/action");
- }
- return pam_retval;
+ byte salt[8];
+ char path[strlen(PATH_PREFIX) + 2*SHA256_DIGEST_LENGTH + 1];
+ char dpath[32];
+ int ofd;
+
+ sprintf(path, PATH_PREFIX);
+ appendHashToPath(hashin, path);
+ readSalt(salt, path);
+
+ snprintf(dpath, sizeof dpath, "/tmp/action.XXXXX.%s", user);
+ ofd = mkstemps(dpath, strlen(user) + 1);
+ if (ofd == -1) {
+ syslog(LOG_AUTH|LOG_ERR, "mkstemps failed for %s: %m", dpath);
+ return PAM_SYSTEM_ERR;
  }
 
- return PAM_AUTH_ERR;
+ if (fchmod(ofd, S_IRWXU)) {
+ syslog(LOG_AUTH|LOG_ERR, "chmod failed for %s: %m", dpath);
+ close(ofd);
+ unlink(dpath);
+ return PAM_SYSTEM_ERR;
+ }
+
+ retval = decrypt(path, ofd, token, salt);
+ close(ofd);
+
+ if (retval != PAM_SUCCESS)
+ return retval;
+
+ switch (fork()) {
+ case -1:
+ syslog(LOG_AUTH|LOG_ERR, "fork failed: %m");
+ return PAM_SYSTEM_ERR;
+ case 0:
+ execl(dpath, "action", NULL, NULL);
+ unlink(dpath);
+ }
+ return pam_retval;
 }