On Wed, Jun 05, 2013 at 02:54:09AM +0400, Dmitry V. Levin wrote:
On Thu, May 23, 2013 at 10:29:59AM -0400, Richard Guy Briggs wrote:
> 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 with echo set to off to do this. This feature (icanon and
> !echo) 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 logging passwords per task.
Sorry for the long delay with review. Please see my comments below.
Ditto...
> This can be used with older kernels since it checks for the
needed structure
> members at compile time.
What would happen if pam_tty_audit compiled with fresh system headers is
attempted to be used on older kernels?
In older kernels:
audit_receive_skb() checks for message lengths and it is happy as long
as the received message length is equal to or longer than that which it
is expecting.
In audit_receive_msg(), in the case AUDIT_TTY_SET, the message length is
checked against the sizeof(struct audit_tty_status) only to make sure it
is at least long enough and not exactly so. The newer second element in
that struct will simply get ignored.
In newer kernels this latter check is generalized to avoid the reverse
problem.
> --- 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_passwd],
> + [
> + HAVE_AUDIT_TTY_STATUS_LOG_PASSWD=yes
> + AC_DEFINE([HAVE_AUDIT_TTY_STATUS_LOG_PASSWD], 1,
[Define to 1 if struct audit_tty_status.log_passwd exists.])
> + ],
> + [
> + HAVE_AUDIT_TTY_STATUS_LOG_PASSWD=""
> + AC_MSG_WARN([The struct audit_tty_status.log_passwd
member is needed for the log_passwd option. The log_passwd 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_PASSWD],
> + [test "x$HAVE_AUDIT_TTY_STATUS_LOG_PASSWD" = xyes])
There is a shorter way to express this idea:
AC_CHECK_MEMBER([struct audit_tty_status.log_passwd], [],
AC_MSG_WARN([audit_tty_status.log_passwd is not available, log_passwd option
disabled.],
[[#include <libaudit.h>]])
...
AM_CONDITIONAL([HAVE_AUDIT_TTY_STATUS_LOG_PASSWD],
[test "x$ac_cv_member_audit_tty_status_log_passwd" = xyes])
Ok, so $ac_cv_member_audit_tty_status_log_passwd is set by
AC_CHECK_MEMBER()?
> --- 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_PASSWD
> + AM_CFLAGS += -DHAVE_AUDIT_TTY_STATUS_LOG_PASSWD
> +endif
This shouldn't be needed because of the side effect of AC_CHECK_MEMBER.
I don't follow. I found I needed this latter one because
HAVE_AUDIT_TTY_STATUS_LOG_PASSWD wasn't being propagated to gcc when
compiling the C module.
> + else if (strcmp (argv[i], "log_passwd") == 0)
> +#ifdef HAVE_AUDIT_TTY_STATUS_LOG_PASSWD
> + log_passwd = 1;
> +#else /* HAVE_AUDIT_TTY_STATUS_LOG_PASSWD */
> + pam_syslog (pamh, LOG_WARNING,
> + "pam_tty_audit: The log_passwd option was not available at
compile time.");
No need to prefix syslog messages with the module name,
you can rely on pam_syslog.
Thanks.
> +#warning "pam_tty_audit: The log_passwd option is not
available. Please upgrade your kernel."
I'm not sure the wording is correct: it's headers not the kernel
that is subject of the configure check.
I pondered this wording. I originally used the header wording, but
thought it better to refer to the kernel, presuming the header would be
upgraded with a capable kernel.
--
ldv
- RGB
--
Richard Guy Briggs <rbriggs(a)redhat.com>
Senior Software Engineer
Kernel Security
AMER ENG Base Operating Systems
Remote, Ottawa, Canada
Voice: 1.647.777.2635
Internal: (81) 32635