----- Original Message -----
> 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.

> Here are two patches, the first to pam to add the switch to
> the pam_tty_audit module.  The second is to the kernel to add the
> necessary bits in audit and tty:

Patches as attachments are little harder to comment on.  So inline preferred.

>From 110971ad92ce8669f6dc18db9e6369e92afdd03e Mon Sep 17 00:00:00 2001
From: Richard Guy Briggs <[email protected]>
Date: Thu, 21 Mar 2013 00:52:37 -0400
Subject: [PATCH] tty: add an option to control logging of passwords with 
pam_tty_audit
To: [email protected]

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.

Signed-off-by: Richard Guy Briggs <[email protected]>
---
 drivers/tty/tty_audit.c    |    8 ++++++++
 include/linux/sched.h      |    1 +
 include/uapi/linux/audit.h |    3 ++-
 kernel/audit.c             |    5 ++++-
 4 files changed, 15 insertions(+), 2 deletions(-)

[snip]

--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -369,7 +369,8 @@ struct audit_status {
 };
 
 struct audit_tty_status {
-       __u32           enabled; /* 1 = enabled, 0 = disabled */
+       __u32           enabled;        /* 1 = enabled, 0 = disabled */
+       __u32           log_icanon;     /* 1 = enabled, 0 = disabled */
 };
 
 /* audit_rule_data supports filter rules with both integer and string
diff --git a/kernel/audit.c b/kernel/audit.c
index d596e53..98d43c6 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -873,6 +873,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct 
nlmsghdr *nlh)
 
                spin_lock_irq(&tsk->sighand->siglock);
                s.enabled = tsk->signal->audit_tty != 0;
+               s.log_icanon = tsk->signal->audit_tty_log_icanon != 0;
                spin_unlock_irq(&tsk->sighand->siglock);
 
                audit_send_reply(NETLINK_CB(skb).portid, seq,
@@ -886,11 +887,13 @@ static int audit_receive_msg(struct sk_buff *skb, struct 
nlmsghdr *nlh)
                if (nlh->nlmsg_len < sizeof(struct audit_tty_status))
                        return -EINVAL;
                s = data;


what happens if this comes from an old pam stack that didn't set/send 
log_icanon?  We'd just be reading past the end of the allocation, right?  Maybe 
something like

unsigned log_icanon
if (nlmsg_len(nlh) < sizeof(struct audit_tty_status))
        log_icanon = 0;
else
        log_icanon = s->log_icanon

-               if (s->enabled != 0 && s->enabled != 1)
+               if (s->enabled != 0 && s->enabled != 1
+                       && s->log_icanon != 0 && s->log_icanon != 1)


Shouldn't this be
if ((s->enabled != 0 && s->enabled != 1) || log_icanon != 0 && log_icanon != 1))

 
                        return -EINVAL;
 
                spin_lock_irq(&tsk->sighand->siglock);
                tsk->signal->audit_tty = s->enabled != 0;
+               tsk->signal->audit_tty_log_icanon = s->log_icanon != 0;
                spin_unlock_irq(&tsk->sighand->siglock);
                break;
        }
-- 
1.7.1

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to