src/ctrl/ChangeLog:
authorMoritz Schulte <mo@g10code.com>
Sat, 29 Oct 2005 10:45:56 +0000 (10:45 +0000)
committerMoritz Schulte <mo@g10code.com>
Sat, 29 Oct 2005 10:45:56 +0000 (10:45 +0000)
2005-10-29  Moritz Schulte  <moritz@g10code.com>

* poldi-ctrl.c (cmd_remove_user): Print a warning instead of an
error in case the serial number could not be looked up; set ERR to
0 in this case and only try to remove key file in case SERIALNO is
non-zero.
(cmd_remove_user): Make sure to lookup the serial number before
removing the user from the user database.

src/common/ChangeLog:

2005-10-29  Moritz Schulte  <moritz@g10code.com>

* support.c (sexp_to_string): Initialize FMT directly, declare
const.
Added even more comments.

src/pam/ChangeLog:

2005-10-29  Moritz Schulte  <moritz@g10code.com>

* pam_poldi.c: More verbose logging.

src/common/ChangeLog
src/common/support.c
src/common/support.h
src/ctrl/ChangeLog
src/ctrl/poldi-ctrl.c
src/pam/ChangeLog
src/pam/pam_poldi.c

index a16f465..78631ca 100644 (file)
@@ -1,3 +1,9 @@
+2005-10-29  Moritz Schulte  <moritz@g10code.com>
+
+       * support.c (sexp_to_string): Initialize FMT directly, declare
+       const.
+       Added even more comments.
+
 2005-10-26  Moritz Schulte  <moritz@g10code.com>
 
        * support.c (usersdb_remove_entry): Use assert to make verify to
index 575ea1e..177b30c 100644 (file)
 #include <jnlib/xmalloc.h>
 #include <jnlib/stringhelp.h>
 
+\f
+
 #define CHALLENGE_MD_ALGORITHM GCRY_MD_SHA1
 
+\f
+
 gpg_error_t
 challenge_generate (unsigned char **challenge, size_t *challenge_n)
 {
@@ -76,7 +80,7 @@ challenge_verify_sexp (gcry_sexp_t sexp_key,
     {
       if (gcry_mpi_scan (&mpi_signature, GCRYMPI_FMT_USG, response, response_n,
                         NULL))
-       err = GPG_ERR_INTERNAL; /* FIXME.  */
+       err = gpg_error (GPG_ERR_BAD_MPI);
     }
 
   /* Create according S-Expressions.  */
@@ -107,7 +111,7 @@ challenge_verify (gcry_sexp_t key,
                  unsigned char *challenge, size_t challenge_n,
                  unsigned char *response, size_t response_n)
 {
-  gpg_error_t err = GPG_ERR_NO_ERROR;
+  gpg_error_t err;
 
   err = challenge_verify_sexp (key,
                               challenge, challenge_n, response, response_n);
@@ -157,43 +161,38 @@ usersdb_translate (const char *serialno, const char *username, char **found)
        }
 
       line_serialno = strtok (line, delimiters);
-      if (! line_serialno)
-       {
-         err = gpg_error (GPG_ERR_INTERNAL); /* FIXME?  */
-         break;
-       }
-
       line_username = strtok (NULL, delimiters);
-      if (! line_username)
-       {
-         err = gpg_error (GPG_ERR_INTERNAL); /* FIXME?  */
-         break;
-       }
 
-      if (serialno)
+      if (line_serialno && line_username)
        {
-         if (! strcmp (serialno, line_serialno))
+         /* Only process this line in case it is `valid' (contains of
+            two tokens).  */
+
+         if (serialno)
            {
-             if (found)
+             if (! strcmp (serialno, line_serialno))
                {
-                 token_found = strdup (line_username);
-                 if (! token_found)
-                   err = gpg_error_from_errno (errno);
+                 if (found)
+                   {
+                     token_found = strdup (line_username);
+                     if (! token_found)
+                       err = gpg_error_from_errno (errno);
+                   }
+                 break;
                }
-             break;
            }
-       }
-      else
-       {
-         if (! strcmp (username, line_username))
+         else
            {
-             if (found)
+             if (! strcmp (username, line_username))
                {
-                 token_found = strdup (line_serialno);
-                 if (! token_found)
-                   err = gpg_error_from_errno (errno);
+                 if (found)
+                   {
+                     token_found = strdup (line_serialno);
+                     if (! token_found)
+                       err = gpg_error_from_errno (errno);
+                   }
+                 break;
                }
-             break;
            }
        }
 
@@ -319,24 +318,26 @@ usersdb_remove_entry (const char *username, const char *serialno,
        }
 
       line_serialno = strtok (line, delimiters);
-      if (! line_serialno)
-       {
-         err = gpg_error (GPG_ERR_INTERNAL); /* FIXME?  */
-         break;
-       }
-
       line_username = strtok (NULL, delimiters);
-      if (! line_username)
+
+      if (line_serialno && line_username)
        {
-         err = gpg_error (GPG_ERR_INTERNAL); /* FIXME?  */
-         break;
-       }
+         /* Complete line (consisting of two tokens).  */
 
-      if ((username && strcmp (username, line_username))
-         || (serialno && strcmp (serialno, line_serialno)))
-       fprintf (users_file_new_fp, "%s\t%s\n", line_serialno, line_username);
+         if ((username && strcmp (username, line_username))
+             || (serialno && strcmp (serialno, line_serialno)))
+           fprintf (users_file_new_fp, "%s\t%s\n",
+                    line_serialno, line_username);
+         else
+           nentries_removed++;
+       }
       else
-       nentries_removed++;
+       {
+         /* Incomplete line (less than two tokens), pass through.  */
+
+         fprintf (users_file_new_fp, "%s\n",
+                  line_serialno ? line_serialno : "");
+       }
 
       free (line);
       line = NULL;
@@ -375,24 +376,32 @@ usersdb_remove_entry (const char *username, const char *serialno,
   return err;
 }
 
+/* This function converts the given S-Expression SEXP into it's
+   `ADVANCED' string representation, using newly-allocated memory,
+   storing the resulting NUL-terminated string in *SEXP_STRING.
+   Returns a proper error code.  */
 gpg_error_t
 sexp_to_string (gcry_sexp_t sexp, char **sexp_string)
 {
+  const int fmt = GCRYSEXP_FMT_ADVANCED;
   gpg_error_t err;
-  char *buffer;
   size_t buffer_n;
-  int fmt;
+  char *buffer;
+
+  assert (sexp);
 
   buffer = NULL;
-  fmt = GCRYSEXP_FMT_ADVANCED;
 
+  /* Figure out amount of memory required for
+     string-representation.  */
   buffer_n = gcry_sexp_sprint (sexp, fmt, NULL, 0);
   if (! buffer_n)
     {
-      err = gpg_error (GPG_ERR_INTERNAL); /* ? */
+      err = gpg_error (GPG_ERR_INV_SEXP); /* ? */
       goto out;
     }
 
+  /* Allocate memory.  */
   buffer = malloc (buffer_n);
   if (! buffer)
     {
@@ -400,10 +409,11 @@ sexp_to_string (gcry_sexp_t sexp, char **sexp_string)
       goto out;
     }
 
+  /* And write string-representation into buffer.  */
   buffer_n = gcry_sexp_sprint (sexp, fmt, buffer, buffer_n);
   if (! buffer_n)
     {
-      err = gpg_error (GPG_ERR_INTERNAL);
+      err = gpg_error (GPG_ERR_INV_SEXP); /* ? */
       goto out;
     }
 
@@ -418,6 +428,9 @@ sexp_to_string (gcry_sexp_t sexp, char **sexp_string)
   return err;
 }
 
+/* This functions converts the given string-representation of an
+   S-Expression into a new S-Expression object, which is to be stored
+   in *SEXP.  Returns proper error code.  */
 gpg_error_t
 string_to_sexp (gcry_sexp_t *sexp, char *string)
 {
@@ -428,18 +441,22 @@ string_to_sexp (gcry_sexp_t *sexp, char *string)
   return err;
 }
 
+/* This function retrieves the content from the file specified by
+   FILENAMED and writes it into a newly allocated chunk of memory,
+   which is then stored in *STRING.  Returns proper error code.  */
 gpg_error_t
 file_to_string (const char *filename, char **string)
 {
-  gpg_error_t err;
   struct stat statbuf;
   char *string_new;
+  gpg_error_t err;
   FILE *fp;
   int ret;
 
-  fp = NULL;
   string_new = NULL;
+  fp = NULL;
 
+  /* Retrieve file size.  */
   ret = stat (filename, &statbuf);
   if (ret)
     {
@@ -468,8 +485,6 @@ file_to_string (const char *filename, char **string)
          goto out;
        }
       string_new[statbuf.st_size] = 0;
-      fclose (fp);             /* FIXME?  */
-      fp = NULL;
     }
 
   err = 0;
@@ -488,6 +503,10 @@ file_to_string (const char *filename, char **string)
 
 \f
 
+/* This functions construct a new C-string containing the absolute
+   path for the file, which is to expected to contain the public key
+   for the card identified by SERIALNO.  Returns proper error
+   code.  */
 gpg_error_t
 key_filename_construct (char **filename, const char *serialno)
 {
@@ -499,6 +518,12 @@ key_filename_construct (char **filename, const char *serialno)
   return 0;
 }
 
+/* This function retrieves the username of the user associated with
+   the current process and stores it *USERNAME.
+
+   Note: the username is contained in statically (!) allocated memory,
+   which may be overwritten by calls to this functions or
+   getpwuid().  */
 gpg_error_t
 lookup_own_username (const char **username)
 {
index ebffeca..bfadfc3 100644 (file)
@@ -32,10 +32,35 @@ gpg_error_t usersdb_lookup_by_username (const char *username, char **serialno);
 gpg_error_t usersdb_remove_entry (const char *username, const char *serialno,
                                  unsigned int *nentries);
 gpg_error_t usersdb_add_entry (const char *username, const char *serialno);
+
+/* This function converts the given S-Expression SEXP into it's
+   `ADVANCED' string representation, using newly-allocated memory,
+   storing the resulting NUL-terminated string in *SEXP_STRING.
+   Returns a proper error code.  */
 gpg_error_t sexp_to_string (gcry_sexp_t sexp, char **sexp_string);
+
+/* This function retrieves the content from the file specified by
+   FILENAMED and writes it into a newly allocated chunk of memory,
+   which is then stored in *STRING.  Returns proper error code.  */
 gpg_error_t file_to_string (const char *filename, char **string);
+
+/* This functions converts the given string-representation of an
+   S-Expression into a new S-Expression object, which is to be stored
+   in *SEXP.  Returns proper error code.  */
 gpg_error_t string_to_sexp (gcry_sexp_t *sexp, char *string);
+
+/* This functions construct a new C-string containing the absolute
+   path for the file, which is to expected to contain the public key
+   for the card identified by SERIALNO.  Returns proper error
+   code.  */
 gpg_error_t key_filename_construct (char **filename, const char *serialno);
+
+/* This function retrieves the username of the user associated with
+   the current process and stores it *USERNAME.
+
+   Note: the username is contained in statically (!) allocated memory,
+   which may be overwritten by calls to this functions or
+   getpwuid().  */
 gpg_error_t lookup_own_username (const char **username);
 
 #endif
index ef08579..f6a2db9 100644 (file)
@@ -1,3 +1,12 @@
+2005-10-29  Moritz Schulte  <moritz@g10code.com>
+
+       * poldi-ctrl.c (cmd_remove_user): Print a warning instead of an
+       error in case the serial number could not be looked up; set ERR to
+       0 in this case and only try to remove key file in case SERIALNO is
+       non-zero.
+       (cmd_remove_user): Make sure to lookup the serial number before
+       removing the user from the user database.
+
 2005-10-26  Moritz Schulte  <moritz@g10code.com>
 
        * poldi-ctrl.c (poldi_ctrl_options_cb): Use gpg_error_t instead of
index 87eab64..9307bba 100644 (file)
@@ -982,6 +982,23 @@ cmd_remove_user (void)
       exit (EXIT_FAILURE);
     }
 
+  /* Make sure to have the serial number.  */
+
+  if (poldi_ctrl_opt.serialno)
+    serialno = poldi_ctrl_opt.serialno;
+  else
+    {
+      serialno = NULL;
+      err = usersdb_lookup_by_username (poldi_ctrl_opt.account, &serialno);
+      if (err)
+       {
+         log_error ("Warning: failed to lookup serial number "
+                    "for username `%s': %s; thus cannot remove key file\n",
+                    poldi_ctrl_opt.account, gpg_strerror (err));
+         err = 0;
+       }
+    }
+
   /* Try to remove entry from user database.  */
 
   err = usersdb_remove_entry (poldi_ctrl_opt.account, poldi_ctrl_opt.serialno,
@@ -1000,34 +1017,20 @@ cmd_remove_user (void)
   /* FIXME: skip step of key file removal in case key file does not
      exist (for whatever reasons).  */
 
-  /* Make sure to have the serial number.  */
+  /* Remove key file.  */
 
-  if (poldi_ctrl_opt.serialno)
-    serialno = poldi_ctrl_opt.serialno;
-  else
+  if (serialno)
     {
-      serialno = NULL;
-      err = usersdb_lookup_by_username (poldi_ctrl_opt.account, &serialno);
+      err = key_file_remove (serialno);
       if (err)
        {
-         log_error ("Error: failed to lookup serial number "
-                    "for username `%s': %s; thus cannot remove key file\n",
-                    poldi_ctrl_opt.account, gpg_strerror (err));
+         log_error ("Error: failed to remove key file for "
+                    "serial number `%s': %s\n",
+                    serialno, gpg_strerror (err));
          goto out;
        }
     }
 
-  /* Remove key file.  */
-
-  err = key_file_remove (serialno);
-  if (err)
-    {
-      log_error ("Error: failed to remove key file for "
-                "serial number `%s': %s\n",
-                serialno, gpg_strerror (err));
-      goto out;
-    }
-
  out:
 
   if (serialno != poldi_ctrl_opt.serialno)
index 8bcec13..4c12798 100644 (file)
@@ -1,3 +1,7 @@
+2005-10-29  Moritz Schulte  <moritz@g10code.com>
+
+       * pam_poldi.c: More verbose logging.
+
 2005-10-23  Moritz Schulte  <moritz@g10code.com>
 
        * pam_poldi.c: Remove Syslog logging macros, use jnlib logging
index 77d6ab9..4b18e58 100644 (file)
@@ -306,21 +306,39 @@ lookup_key (const char *username, gcry_sexp_t *key)
 
   err = usersdb_lookup_by_username (username, &serialno);
   if (err)
-    goto out;
+    {
+      log_error ("Error: failed to lookup serial number for user `%s': %s\n",
+                username, gpg_strerror (err));
+      goto out;
+    }
 
   err = key_filename_construct (&key_path, serialno);
   if (err)
-    goto out;
+    {
+      log_error ("Error: failed to construct key file path "
+                "for serial number `%s': %s\n",
+                serialno, gpg_strerror (err));
+      goto out;
+    }
 
   err = file_to_string (key_path, &key_string);
   if ((! err) && (! key_string))
     err = gpg_error (GPG_ERR_NO_PUBKEY);
   if (err)
-    goto out;
+    {
+      log_error ("Error: failed to retrieve key from key file `%s': %s\n",
+                key_path, gpg_strerror (err));
+      goto out;
+    }
 
   err = string_to_sexp (&key_sexp, key_string);
   if (err)
-    goto out;
+    {
+      log_error ("Error: failed to convert key "
+                "from `%s' into S-Expression: %s\n",
+                key_path, gpg_strerror (err));
+      goto out;
+    }
 
   *key = key_sexp;
 
@@ -354,12 +372,19 @@ wait_for_card (int slot, int require_card_switch,
     {
       if (gpg_err_code (err) == GPG_ERR_CARD_NOT_PRESENT)
        tell_user (conv, "Timeout inserting card");
+      else
+       log_error ("Error: failed to initialize card: %s\n",
+                  gpg_strerror (err));
       goto out;
     }
 
   err = card_info (slot, &serialno_new, NULL, NULL);
   if (err)
-    goto out;
+    {
+      log_error ("Error: failed to retrieve card information: %s\n",
+                gpg_strerror (err));
+      goto out;
+    }
 
   *serialno = serialno_new;
 
@@ -391,7 +416,7 @@ parse_argv (int argc, const char **argv)
        pam_poldi_opt.wait_timeout = atoi (argv[i] + 8);
       else
        {
-         log_error ("Unknown PAM argument: %s", argv[i]);
+         log_error ("Error: Unknown PAM argument: %s", argv[i]);
          err = gpg_error (GPG_ERR_UNKNOWN_NAME);
        }
 
@@ -424,22 +449,39 @@ do_auth (int slot, const struct pam_conv *conv, gcry_sexp_t key)
   /* Query user for PIN.  */
   err = ask_user (conv, POLDI_PIN2_QUERY_MSG, &pin);
   if (err)
-    goto out;
+    {
+      log_error ("Error: failed to retrieve PIN from user: %s\n",
+                gpg_strerror (err));
+      goto out;
+    }
 
   /* Send PIN to card.  */
   err = card_pin_provide (slot, 2, pin);
   if (err)
-    goto out;
+    {
+      log_error ("Error: failed to send PIN to card: %s\n",
+                gpg_strerror (err));
+      goto out;
+    }
 
   /* Generate challenge.  */
   err = challenge_generate (&challenge, &challenge_n);
   if (err)
-    goto out;
+    {
+      log_error ("Error: failed to generate challenge: %s\n",
+                gpg_strerror (err));
+      goto out;
+    }
 
   /* Let card sign the challenge.  */
   err = card_sign (slot, challenge, challenge_n, &response, &response_n);
   if (err)
-    goto out;
+    {
+      log_error ("Error: failed to retrieve challenge signature "
+                "from card: %s\n",
+                gpg_strerror (err));
+      goto out;
+    }
 
   /* Verify response.  */
   err = challenge_verify (key, challenge, challenge_n, response, response_n);
@@ -457,7 +499,8 @@ do_auth (int slot, const struct pam_conv *conv, gcry_sexp_t key)
 
 \f
 
-/* Uaaahahahh, ich will dir einloggen!  */
+/* Uaaahahahh, ich will dir einloggen!  PAM authentication entry
+   point.  */
 PAM_EXTERN int
 pam_sm_authenticate (pam_handle_t *pam_handle,
                     int flags, int argc, const char **argv)
@@ -482,12 +525,26 @@ pam_sm_authenticate (pam_handle_t *pam_handle,
   err = options_parse_conf  (pam_poldi_options_cb, NULL,
                             arg_opts, POLDI_CONF_FILE);
   if (err)
-    goto out;
+    {
+      log_error ("Error: failed to parse configuration file: %s\n",
+                gpg_strerror (err));
+      goto out;
+    }
 
   /* Parse argument vector provided by PAM.  */
   err = parse_argv (argc, argv);
   if (err)
-    goto out;
+    {
+      log_error ("Error: failed to parse PAM argument vector: %s\n",
+                gpg_strerror (err));
+      goto out;
+    }
+
+  /* Initialize logging: in case `logfile' has been set in the
+     configuration file, initialize jnlib-logging the traditional
+     file, loggin to the file (or socket special file) specified in
+     the configuration file; in case `logfile' has NOT been set in the
+     configuration file, log through Syslog.  */
 
   if (pam_poldi_opt.logfile)
     {
@@ -503,6 +560,7 @@ pam_sm_authenticate (pam_handle_t *pam_handle,
   else
     log_set_syslog ("poldi", LOG_AUTH);
 
+  /* Initialize libscd.  */
   scd_init (pam_poldi_opt.debug,
            pam_poldi_opt.debug_sc,
            pam_poldi_opt.verbose,
@@ -513,6 +571,10 @@ pam_sm_authenticate (pam_handle_t *pam_handle,
            pam_poldi_opt.disable_ccid,
            pam_poldi_opt.debug_ccid_driver);
 
+  /*
+   * Retrieve information from PAM.
+   */
+
   /* Ask PAM for username.  */
   ret = pam_get_item (pam_handle, PAM_USER, &username_void);
   if (ret != PAM_SUCCESS)
@@ -537,6 +599,10 @@ pam_sm_authenticate (pam_handle_t *pam_handle,
   if (err)
     goto out;
 
+  /*
+   * Process authentication request.
+   */
+
   if (username)
     {
       /* We got a username from PAM, thus we are waiting for a
@@ -561,14 +627,28 @@ pam_sm_authenticate (pam_handle_t *pam_handle,
        {
          /* Either the account could not be found or it is not the
             expected one -> fail.  */
-         tell_user (conv, "Serial no %s is not associated with %s",
-                    serialno, username);
+
          if (! err)
-           err = gpg_error (GPG_ERR_INV_NAME);
+           {
+             tell_user (conv, "Serial no %s is not associated with %s",
+                        serialno, username);
+             err = gpg_error (GPG_ERR_INV_NAME);
+           }
+         else
+           log_error ("Error: failed to lookup username for "
+                      "serial number `%s': %s\n",
+                      serialno, gpg_strerror (err));
        }
       else
-       /* Inform user about inserted card.  */
-       err = tell_user (conv, "Serial no: %s", serialno);
+       {
+         /* Inform user about inserted card.  */
+       
+         err = tell_user (conv, "Serial no: %s", serialno);
+         if (err)
+           log_error ("Error: failed to inform user about inserted card: %s\n",
+                      gpg_strerror (err));
+       }
+
       if (err)
        goto out;
 
@@ -592,17 +672,30 @@ pam_sm_authenticate (pam_handle_t *pam_handle,
       /* Inform user about inserted card.  */
       err = tell_user (conv, "Serial no: %s", serialno);
       if (err)
-       goto out;
+       {
+         log_error ("Error: failed to inform user about inserted card: %s\n",
+                    gpg_strerror (err));
+         goto out;
+       }
 
       /* Lookup account for inserted card.  */
       err = usersdb_lookup_by_serialno (serialno, &account);
       if (err)
-       goto out;
+       {
+         log_error ("Error: failed to lookup username for "
+                    "serial number `%s': %s\n",
+                    serialno, gpg_strerror (err));
+         goto out;
+       }
 
       /* Inform user about looked up account.  */
       err = tell_user (conv, "Account: %s", account);
       if (err)
-       goto out;
+       {
+         log_error ("Error: failed to inform user about inserted card: %s\n",
+                    gpg_strerror (err));
+         goto out;
+       }
 
       /* Lookup key for looked up account.  */
       err = lookup_key (account, &key);
@@ -648,6 +741,8 @@ pam_sm_authenticate (pam_handle_t *pam_handle,
   return err ? PAM_AUTH_ERR : PAM_SUCCESS;
 }
 
+
+/* PAM's `set-credentials' interface.  */
 PAM_EXTERN int
 pam_sm_setcred (pam_handle_t *pam_handle,
                int flags, int argc, const char **argv)