12:27 p.m.
Hi,
I might be mis-reading the code, but it appears that the current
audit_alloc() code is not preserving the loginuid of the parent process
for the child process upon a fork or clone. Instead, it is checking to
see if the just-created child has an existing audit context and if so,
copies the loginuid from it, which seems nonsensical, especially as it
does nothing to free any existing audit context if one exists already.
Does this patch look correct? In this context, tsk == the child, and
current == the parent. The child has not yet started execution at this
point; it is still being setup by the fork/clone code.
--- linux-2.6.10/kernel/auditsc.c.orig 2005-01-06 13:11:51.000000000 -0500
+++ linux-2.6.10/kernel/auditsc.c 2005-01-06 13:14:28.000000000 -0500
@@ -549,8 +549,8 @@ int audit_alloc(struct task_struct *tsk)
/* Preserve login uid */
context->loginuid = -1;
- if (tsk->audit_context)
- context->loginuid = tsk->audit_context->loginuid;
+ if (current->audit_context)
+ context->loginuid = current->audit_context->loginuid;
tsk->audit_context = context;
set_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
--
Stephen Smalley <sds(a)epoch.ncsc.mil>
National Security Agency
3:18 p.m.
Yes, that has to have been a typo... The patch certainly looks correct.
(will test later today)
thanks,
-serge
On Thu, 2005-01-06 at 13:27 -0500, Stephen Smalley wrote:
Hi,
I might be mis-reading the code, but it appears that the current
audit_alloc() code is not preserving the loginuid of the parent process
for the child process upon a fork or clone. Instead, it is checking to
see if the just-created child has an existing audit context and if so,
copies the loginuid from it, which seems nonsensical, especially as it
does nothing to free any existing audit context if one exists already.
Does this patch look correct? In this context, tsk == the child, and
current == the parent. The child has not yet started execution at this
point; it is still being setup by the fork/clone code.
--- linux-2.6.10/kernel/auditsc.c.orig 2005-01-06 13:11:51.000000000 -0500
+++ linux-2.6.10/kernel/auditsc.c 2005-01-06 13:14:28.000000000 -0500
@@ -549,8 +549,8 @@ int audit_alloc(struct task_struct *tsk)
/* Preserve login uid */
context->loginuid = -1;
- if (tsk->audit_context)
- context->loginuid = tsk->audit_context->loginuid;
+ if (current->audit_context)
+ context->loginuid = current->audit_context->loginuid;
tsk->audit_context = context;
set_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
--
Serge Hallyn <serue(a)us.ibm.com>
6:42 p.m.
This patch works for me. Odd thing is I would have thought I would have
noticed this behavior before.
I'm testing it by hacking auditctl.c to do an execl("/bin/sh",
"/bin/sh"); if it was called on a -L. Before the "auditctl -L 25,ab"
my
loginuid is -1, after (on, say, a "auditctl -m cd") it is 25. Without
the patch, it is always -1.
Is that (-1 default loginuid until PAM sets it) the desired behavior?
-serge
On Thu, 2005-01-06 at 13:27 -0500, Stephen Smalley wrote:
Hi,
I might be mis-reading the code, but it appears that the current
audit_alloc() code is not preserving the loginuid of the parent process
for the child process upon a fork or clone. Instead, it is checking to
see if the just-created child has an existing audit context and if so,
copies the loginuid from it, which seems nonsensical, especially as it
does nothing to free any existing audit context if one exists already.
Does this patch look correct? In this context, tsk == the child, and
current == the parent. The child has not yet started execution at this
point; it is still being setup by the fork/clone code.
--- linux-2.6.10/kernel/auditsc.c.orig 2005-01-06 13:11:51.000000000 -0500
+++ linux-2.6.10/kernel/auditsc.c 2005-01-06 13:14:28.000000000 -0500
@@ -549,8 +549,8 @@ int audit_alloc(struct task_struct *tsk)
/* Preserve login uid */
context->loginuid = -1;
- if (tsk->audit_context)
- context->loginuid = tsk->audit_context->loginuid;
+ if (current->audit_context)
+ context->loginuid = current->audit_context->loginuid;
tsk->audit_context = context;
set_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
--
Serge Hallyn <serue(a)us.ibm.com>
7:27 p.m.
On Thursday 06 January 2005 19:42, Serge Hallyn wrote:
I'm testing it by hacking auditctl.c to do an
execl("/bin/sh",
"/bin/sh"); if it was called on a -L. Before the "auditctl -L 25,ab"
my
loginuid is -1, after (on, say, a "auditctl -m cd") it is 25. Without
the patch, it is always -1.
That's what I noticed and made me say I didn't think it was working right.
Is that (-1 default loginuid until PAM sets it) the desired behavior?
If there is no logins, there's nothing to *set* the user ID. The number
returned needs to be fictional so its not mistaken for root or a real user.
-Steve Grubb
5:29 a.m.
On Thu, 2005-01-06 at 20:27 -0500, Steve Grubb wrote:
On Thursday 06 January 2005 19:42, Serge Hallyn wrote:
> I'm testing it by hacking auditctl.c to do an execl("/bin/sh",
> "/bin/sh"); if it was called on a -L. Before the "auditctl -L
25,ab" my
> loginuid is -1, after (on, say, a "auditctl -m cd") it is 25. Without
> the patch, it is always -1.
That's what I noticed and made me say I didn't think it was working right.
> Is that (-1 default loginuid until PAM sets it) the desired behavior?
If there is no logins, there's nothing to *set* the user ID. The number
returned needs to be fictional so its not mistaken for root or a real user.
But -1 isn't a fictional UID. There is no number within the uid_t space
which is reserved for your purposes to mean 'no user specified'.
It's been suggested that perhaps we could use the kernel's key
management facilities for this. By attaching a key of a certain type to
the session keyring, we get to set it _synchronously_, and we also have
an unambiguous indication of its _absence_.
David, can a process ever override its session keyring and elect to use
another? That could be a problem -- this has to be immutable after it's
set at login time, I believe.
--
dwmw2
8:29 a.m.
On Fri, 2005-01-07 at 06:29, David Woodhouse wrote:
But -1 isn't a fictional UID. There is no number within the uid_t
space
which is reserved for your purposes to mean 'no user specified'.
Are we limited to the uid_t space? I know that the current code uses
uid_t for loginuid, and it has to be at least as large as uid_t, but
nothing says that every loginuid has to be useable as an ordinary uid;
the loginuid is only for use by the audit subsystem.
--
Stephen Smalley <sds(a)epoch.ncsc.mil>
National Security Agency
9:43 a.m.
Stephen Smalley <sds(a)epoch.ncsc.mil> wrote:
On Fri, 2005-01-07 at 06:29, David Woodhouse wrote:
> But -1 isn't a fictional UID. There is no number within the uid_t space
> which is reserved for your purposes to mean 'no user specified'.
Are we limited to the uid_t space? I know that the current code uses
uid_t for loginuid, and it has to be at least as large as uid_t, but
nothing says that every loginuid has to be useable as an ordinary uid;
the loginuid is only for use by the audit subsystem.
With reference to my key management system keys, the amount of data they can
hold is more or less arbitrary. For example, the user-defined key type just
holds a string of bytes of up to the user's quota in length.
David
9:41 a.m.
David Woodhouse <dwmw2(a)infradead.org> wrote:
It's been suggested that perhaps we could use the kernel's
key
management facilities for this. By attaching a key of a certain type to
the session keyring, we get to set it _synchronously_, and we also have
an unambiguous indication of its _absence_.
This facility has been in the vanilla Linux kernel since 2.6.10. See:
Documentation/keys.txt
David, can a process ever override its session keyring and elect to
use
another? That could be a problem -- this has to be immutable after it's
set at login time, I believe.
Unfortunately, yes. There's a keyctl() op that allows a process to change the
session it belongs to.
However, since the key management stuff is not entirely graven in stone yet,
it can be extended. I can deal with the problem Dave mentioned by adding a
root-privileged keyring to each process that can't just be shuffled aside so
easily for the purpose of keeping track of auditing and security info of this
type. This keyring would be inherited across fork.
Either I can have it so that it is searched first, thus the search order would
become:
Audit (or System, Security or whatever)
Thread
Process
Session
Either that or it could just require explicit searching and not be on the
standard search path.
I'd make the "Audit" keyring owned by root, with modify privileges only for
root and stick keys of the type you'd be dealing with owned by root and only
modifyable by root so that the user can't change them. Furthermore, if you
don't provide an update method for the key type, then the keys are immutable
once instantiated.
David
7289
days inactive
7290
days old
linux-audit@lists.linux-audit.osci.io
7 comments
5 participants
participants (5)
-
David Howells -
David Woodhouse -
Serge Hallyn -
Stephen Smalley -
Steve Grubb