Dave,
I think Steve and I have agreed on a solution, I'll put together a patch
right now based on what is currently in net-2.6 (i.e. the existing
NetLabel audit patch) and submit it to the lists in a few hours.
Steve Grubb wrote:
> On Friday 29 September 2006 14:09, Paul Moore wrote:
>
>>>type field is already taken for another purpose, it needs to be renamed.
>>
>>If we can't have duplicate field names I would propose prefixing both
>>these fields (and doing similar things with the other NetLabel specific
>>fields) with a "cipso_" making them "cipso_doi" and "cipso_type".
>
>
> That would be fine. This limits future field name collisions.
>
>
>>>>+/**
>>>>+ * netlbl_unlabel_acceptflg_set - Set the unlabeled accept flag
>>>>+ * @value: desired value
>>>>+ * @audit_secid: the LSM secid to use in the audit message
>>>>+ *
>>>>+ * Description:
>>>>+ * Set the value of the unlabeled accept flag to @value.
>>>>+ *
>>>>+ */
>>>>+static void netlbl_unlabel_acceptflg_set(u8 value, u32 audit_secid)
>>>>+{
>>>>+ atomic_set(&netlabel_unlabel_accept_flg, value);
>>>>+ netlbl_audit_nomsg((value ?
>>>>+ AUDIT_MAC_UNLBL_ACCEPT : AUDIT_MAC_UNLBL_DENY),
>>>>+ audit_secid);
>>>
>>>Looking at how this is being used, I think only 1 message type should be
>>>used. There are places in the audit system where we set a flag to 1 or 0,
>>>but only have 1 message type. We record the old and new value. So, you'd
>>>need to pass that to the logger.
>>
>>With that in mind I would probably change the message type to
>>AUDIT_MAC_UNLBL_ALLOW and use a "unlbl_accept" field; is that okay?
>
>
> That would be fine. Just a quick note...we have generally been "old " to
> indicate the previous value. Example, "backlog=512 old=256".
>
>
>>>>+/**
>>>>+ * netlbl_audit_start_common - Start an audit message
>>>>+ * @type: audit message type
>>>>+ * @secid: LSM context ID
>>>>+ *
>>>>+ * Description:
>>>>+ * Start an audit message using the type specified in @type and fill the
>>>>audit + * message with some fields common to all NetLabel audit messages.
>>>>Returns + * a pointer to the audit buffer on success, NULL on failure.
>>>>+ *
>>>>+ */
>>>>+struct audit_buffer *netlbl_audit_start_common(int type, u32 secid)
>>>>+{
>>>
>>>Generally, logging functions are moved into auditsc.c where the context
>>>and other functions are defined.
>>
>>How about leaving this for a future revision?
>
>
> Come to think of it, you don't need to move it. The reason to move it is to
> access the context and use helper functions related to it. But I found that
> you were using "current" which may not always be the sender. So if you cannot
> use current, most of the stuff you are logging can't be, so the event being
> logged becomes simpler and you don't need to move it.
>
> I have not traced through all the code, but if you do any security checks
> before taking the rules, be careful not to use current.
>
>
>>>>+ audit_log_format(audit_buf,
>>>>+ "netlabel: auid=%u uid=%u tty=%s pid=%d",
>>>>+ audit_loginuid,
>>>>+ current->uid,
>>>>+ audit_tty,
>>>>+ current->pid);
>>>
>>>Why are you logging all this? When we add audit rules, all that we log is
>>>the auid, and subj. If we need to log all this, we should probably have a
>>>helper function that gets called by other config change loggers.
>>
>>If I drop the uid, tty, and pid fields will this be acceptable?
>
>
> and comm & exe, yes. Anything you were basing off of current has to go. The
> audit rule logging was reduced to the credentials that are carried along in
> the netlink packet since that's all you can trust. The sending process could
> be gone by the time you get to this point in the code.
>
>
>>>>+ audit_log_format(audit_buf, " comm=");
>>>>+ audit_log_untrustedstring(audit_buf, audit_comm);
>>>>+ if (current->mm) {
>>>>+ down_read(¤t->mm->mmap_sem);
>>>>+ vma = current->mm->mmap;
>>>>+ while (vma) {
>>>>+ if ((vma->vm_flags & VM_EXECUTABLE) &&
>>>>+ vma->vm_file) {
>>>>+ audit_log_d_path(audit_buf,
>>>>+ " exe=",
>>>>+ vma->vm_file->f_dentry,
>>>>+ vma->vm_file->f_vfsmnt);
>>>>+ break;
>>>>+ }
>>>>+ vma = vma->vm_next;
>>>>+ }
>>>>+ up_read(¤t->mm->mmap_sem);
>>>>+ }
>>>>+
>>>
>>>If this function was moved inside auditsc.c you could use a function
>>>there that does this. But the question remains why all this data?
>>
>>In the ideal world would you prefer this to be removed?
>
>
> Yes.
>
> -Steve
--
paul moore
linux security @ hp
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
