[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [RFC] linux-2.6.10-auditfs-tc1.patch



[much easier to comment on patches if they are inline, and shorter lines
to keep linewrap down]

* Darrel Goeddel (dgoeddel trustedcs com) wrote:
> Serge Hallyn wrote:
> > Perhaps we should print out current->cap_effective?  Or is that
> > overkill?  Or perhaps an actual "security_identify_process(task, buf,
> > len)" hook would be useful, where commoncap could print out the
> > capabilities, and selinux could print out the context.  Maybe that's
> > closer to debug info...
> > 
> 
> This hook, and a similar security_identify_inode(...), hook will be necessary 
> for an LSM to go through a LSPP evaluation.  The label information is required 
> to be included in the audit record for all subjects/objects/information involved 
> in the event.  I have a quick-and-dirty patch that implemented this 
> functionality.  Note that this patch uses pre-allocated 1K buffers (limits info 
> and sucks up a lot of memory).  A proper memory allocation scheme needs to be 
> worked up and the patch probably needs to be rebased to newer code.  I planned 
> on getting back to this in the near future.  If someone else is working on this 
> functionality, please let me know, otherwise I can bump this up on my TODO list.
> 
> This patch also includes uid/gid/mode for filesystem objects.  I felt that this 
> was a needed addition to determine the cause of filesystem related denials.  Do 
> others agree with this addition to the records, and is there anything else that 
> we could possibly want?

This would be redundant to the audit info that Tim's trying to push out.
Security label should stand alone, and be a simple string.


> -			    nd->dentry->d_inode->i_ino,
> -			    nd->dentry->d_inode->i_rdev);
> +		audit_inode(name, nd->dentry->d_inode);

Makes sense.  But this is only for path lookup.  Doesn't account, for
example, for the audit msg partway through failed path resolution --
failed for security reason, for example.

>  	return retval;
>  }
>  
> Index: include/linux/audit.h
> ===================================================================
> RCS file: /source/cvsroots/fedora-cd/fedora-cd/src/linux-2.6/include/linux/audit.h,v
> retrieving revision 1.1.1.1
> diff -u -p -r1.1.1.1 audit.h
> --- include/linux/audit.h	26 May 2004 18:05:58 -0000	1.1.1.1
> +++ include/linux/audit.h	12 Jan 2005 19:48:40 -0000
> @@ -147,7 +147,9 @@ extern void audit_syscall_entry(struct t
>  extern void audit_syscall_exit(struct task_struct *task, int return_code);
>  extern void audit_getname(const char *name);
>  extern void audit_putname(const char *name);
> -extern void audit_inode(const char *name, unsigned long ino, dev_t rdev);
> +
> +struct inode;
> +extern void audit_inode(const char *name, struct inode *inode);
>  
>  				/* Private API (for audit.c only) */
>  extern int  audit_receive_filter(int type, int pid, int uid, int seq,
> @@ -162,7 +164,7 @@ extern int  audit_set_loginuid(struct au
>  #define audit_syscall_exit(t,r) do { ; } while (0)
>  #define audit_getname(n) do { ; } while (0)
>  #define audit_putname(n) do { ; } while (0)
> -#define audit_inode(n,i,d) do { ; } while (0)
> +#define audit_inode(n,i) do { ; } while (0)
>  #endif
>  
>  #ifdef CONFIG_AUDIT
> Index: include/linux/security.h
> ===================================================================
> RCS file: /source/cvsroots/fedora-cd/fedora-cd/src/linux-2.6/include/linux/security.h,v
> retrieving revision 1.35
> diff -u -p -r1.35 security.h
> --- include/linux/security.h	11 Jan 2005 19:10:15 -0000	1.35
> +++ include/linux/security.h	12 Jan 2005 19:48:40 -0000
> @@ -413,6 +413,11 @@ struct open_request;
>   *	is specified by @buffer_size.  @buffer may be NULL to request
>   *	the size of the buffer required.
>   *	Returns number of bytes used/required on success.
> + * @inode_audit_augment:
> + *	Copy a NULL terminated string representing @inode's security relevant
> + *	data into @buffer.  @buffer_size is the size of buffer that is being
> + *	written to.  You only have this much space and this call can not return
> + *	an error, so manage the space wisely...
>   *
>   * Security hooks for file operations
>   *
> @@ -632,6 +637,11 @@ struct open_request;
>   * 	security attributes, e.g. for /proc/pid inodes.
>   *	@p contains the task_struct for the task.
>   *	@inode contains the inode structure for the inode.
> + * @task_audit_augment:
> + *	Copy a NULL terminated string representing @p's security relevant
> + *	data into @buffer.  @buffer_size is the size of buffer that is being
> + *	written to.  You only have this much space and this call can not return
> + *	an error, so manage the space wisely...

This should simply get back a char*, and the caller is responsible for
freeing, or something like that.  Also, this is needed for _every_ label,
not just inode and task.  SELinux already has this function internally,
see getprocattr->security_sid_to_context.  Perhaps a better solution is
to make the name be part of a label.

thanks,
-chris
-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]