Re: pam_tty_audit icanon log switch

[Richard Guy Briggs <rgb@redhat.com>]

On Fri, Mar 22, 2013 at 08:19:31AM +0100, Tomas Mraz wrote:
> On Fri, 2013-03-22 at 01:46 -0400, Richard Guy Briggs wrote: 
> > Hi folks,
> > 
> > There's been a couple of requests to add a switch to pam_tty_audit to
> > *not* log passwords when logging user commands.
> > 
> > Most commands are entered one line at a time and processed as complete
> > lines in non-canonical mode.  Commands that interactively require a
> > password, enter canonical mode to do this.  This feature (icanon) can be
> > used to avoid logging passwords by audit while still logging the rest of
> > the command.
> > 
> > Adding a member to the struct audit_tty_status passed in by
> > pam_tty_audit allows control of canonical mode per task.
> > 
> 
> For the upstream inclusion of the pam_tty_audit patch you will need to
> add a detection of the new member of the struct audit_tty_status in the
> configure.in and #ifdef the code properly. The new option can be kept
> even in the case the new member is not available, but it should log a
> warning into the syslog with pam_syslog() when used. The documentation
> should reflect the fact that the option might not be available on old
> kernels as well.

Tomas,

Please have a look at this patch and see if this addresses the issues
you raised:

---
 configure.in                              |   15 +++++++++++++++
 modules/pam_tty_audit/Makefile.am         |    3 +++
 modules/pam_tty_audit/pam_tty_audit.8.xml |   14 ++++++++++++++
 modules/pam_tty_audit/pam_tty_audit.c     |   23 ++++++++++++++++++++++-
 4 files changed, 54 insertions(+), 1 deletions(-)

diff --git a/configure.in b/configure.in
index 515b301..c9c1c5f 100644
--- a/configure.in
+++ b/configure.in
@@ -386,6 +386,19 @@ if test x"$WITH_LIBAUDIT" != xno ; then
         fi
         if test ! -z "$HAVE_AUDIT_TTY_STATUS" ; then
             AC_DEFINE([HAVE_AUDIT_TTY_STATUS], 1, [Define to 1 if struct audit_tty_status exists.])
+
+            AC_CHECK_MEMBER(
+                            [struct audit_tty_status.log_icanon],
+                            [
+                             HAVE_AUDIT_TTY_STATUS_LOG_ICANON=yes
+                             AC_DEFINE([HAVE_AUDIT_TTY_STATUS_LOG_ICANON], 1, [Define to 1 if struct audit_tty_status.log_icanon exists.])
+                            ],
+                            [
+                             HAVE_AUDIT_TTY_STATUS_LOG_ICANON=""
+                             AC_MSG_WARN([The struct audit_tty_status.log_icanon member is needed for the log_icanon option.  The log_icanon option is disabled.])
+                            ],
+                            [[#include <libaudit.h>]]
+                            )
         fi
 else
 	LIBAUDIT=""
@@ -393,6 +406,8 @@ fi
 AC_SUBST(LIBAUDIT)
 AM_CONDITIONAL([HAVE_AUDIT_TTY_STATUS],
 	       [test "x$HAVE_AUDIT_TTY_STATUS" = xyes])
+AM_CONDITIONAL([HAVE_AUDIT_TTY_STATUS_LOG_ICANON],
+	       [test "x$HAVE_AUDIT_TTY_STATUS_LOG_ICANON" = xyes])
 
 AC_CHECK_HEADERS(xcrypt.h crypt.h)
 AS_IF([test "x$ac_cv_header_xcrypt_h" = "xyes"],
diff --git a/modules/pam_tty_audit/Makefile.am b/modules/pam_tty_audit/Makefile.am
index 6378483..b67d2e5 100644
--- a/modules/pam_tty_audit/Makefile.am
+++ b/modules/pam_tty_audit/Makefile.am
@@ -16,6 +16,9 @@ XMLS = README.xml pam_tty_audit.8.xml
 securelibdir = $(SECUREDIR)
 
 AM_CFLAGS = -I$(top_srcdir)/libpam/include -I$(top_srcdir)/libpamc/include
+if HAVE_AUDIT_TTY_STATUS_LOG_ICANON
+  AM_CFLAGS += -DHAVE_AUDIT_TTY_STATUS_LOG_ICANON
+endif
 AM_LDFLAGS = -no-undefined -avoid-version -module
 if HAVE_VERSIONING
   AM_LDFLAGS += -Wl,--version-script=$(srcdir)/../modules.map
diff --git a/modules/pam_tty_audit/pam_tty_audit.8.xml b/modules/pam_tty_audit/pam_tty_audit.8.xml
index 447b845..f451f45 100644
--- a/modules/pam_tty_audit/pam_tty_audit.8.xml
+++ b/modules/pam_tty_audit/pam_tty_audit.8.xml
@@ -77,6 +77,18 @@
           </para>
         </listitem>
       </varlistentry>
+      <varlistentry>
+        <term>
+          <option>log_icanon</option>
+        </term>
+        <listitem>
+          <para>
+           Log keystrokes in ICANON mode.  By default, keystrokes in ICANON
+           mode are not logged to avoid logging passwords.  This option may not
+           be available on older kernels (3.9?).
+          </para>
+        </listitem>
+      </varlistentry>
     </variablelist>
   </refsect1>
 
@@ -161,6 +173,8 @@ session	required pam_tty_audit.so disable=* enable=root
       <para>
         pam_tty_audit was written by Miloslav Trma&ccaron;
 	&lt;mitr@redhat.com&gt;.
+        The log_icanon option was added by Richard Guy Briggs
+        &lt;rgb@redhat.com&gt;.
       </para>
   </refsect1>
 
diff --git a/modules/pam_tty_audit/pam_tty_audit.c b/modules/pam_tty_audit/pam_tty_audit.c
index 080f495..7be914b 100644
--- a/modules/pam_tty_audit/pam_tty_audit.c
+++ b/modules/pam_tty_audit/pam_tty_audit.c
@@ -201,6 +201,9 @@ pam_sm_open_session (pam_handle_t *pamh, int flags, int argc, const char **argv)
   struct audit_tty_status *old_status, new_status;
   const char *user;
   int i, fd, open_only;
+#ifdef HAVE_AUDIT_TTY_STATUS_LOG_ICANON
+  int log_icanon;
+#endif
 
   (void)flags;
 
@@ -212,6 +215,9 @@ pam_sm_open_session (pam_handle_t *pamh, int flags, int argc, const char **argv)
 
   command = CMD_NONE;
   open_only = 0;
+#ifdef HAVE_AUDIT_TTY_STATUS_LOG_ICANON
+  log_icanon = 0;
+#endif
   for (i = 0; i < argc; i++)
     {
       if (strncmp (argv[i], "enable=", 7) == 0
@@ -237,6 +243,14 @@ pam_sm_open_session (pam_handle_t *pamh, int flags, int argc, const char **argv)
 	}
       else if (strcmp (argv[i], "open_only") == 0)
 	open_only = 1;
+      else if (strcmp (argv[i], "log_icanon") == 0)
+#ifdef HAVE_AUDIT_TTY_STATUS_LOG_ICANON
+        log_icanon = 1;
+#else
+        pam_syslog (pamh, LOG_WARNING,
+                    "pam_tty_audit: The log_icanon option was not available at compile time.");
+#warning "pam_tty_audit: The log_icanon option is not available.  Please upgrade your kernel."
+#endif
       else
 	{
 	  pam_syslog (pamh, LOG_ERR, "unknown option `%s'", argv[i]);
@@ -262,7 +276,14 @@ pam_sm_open_session (pam_handle_t *pamh, int flags, int argc, const char **argv)
     }
 
   new_status.enabled = (command == CMD_ENABLE ? 1 : 0);
-  if (old_status->enabled == new_status.enabled)
+#ifdef HAVE_AUDIT_TTY_STATUS_LOG_ICANON
+  new_status.log_icanon = log_icanon;
+#endif
+  if (old_status->enabled == new_status.enabled
+#ifdef HAVE_AUDIT_TTY_STATUS_LOG_ICANON
+      && old_status->log_icanon == new_status.log_icanon
+#endif
+     )
     {
       open_only = 1; /* to clean up old_status */
       goto ok_fd;
-- 
1.7.1

> Tomas Mraz

- RGB

--
Richard Guy Briggs <rbriggs@redhat.com>
Senior Software Engineer
AMER ENG Base Operating Systems
Remote, Canada, Ottawa
Voice: 1.647.777.2635
Internal: (81) 32635