[PATCH]Fix me add subj

[PATCH]Fix me add subj

[Chu Li]

Hi Steve,
  I have made a patch for "Fixme add subj" in auditd.c. This is for the latest
codes.

Signed-off-by: Chu Li <chul@cn.fujistu.com>
---
diff --git a/src/auditd.c b/src/auditd.c
index b74199f..3843245 100755
--- a/src/auditd.c
+++ b/src/auditd.c
@@ -53,6 +53,7 @@
 #define DMSG_SIZE (DEFAULT_BUF_SZ + 48)
 #define SUCCESS 0
 #define FAILURE 1
+#define SUBJ_LEN 4097

 /* Global Data */
 volatile int stop = 0;
@@ -65,11 +66,13 @@ static int init_pipe[2];
 static int do_fork = 1;
 static struct auditd_reply_list *rep = NULL;
 static int hup_info_requested = 0, usr1_info_requested = 0;
+static char subj[SUBJ_LEN];

 /* Local function prototypes */
 static void close_down(void);
 static void clean_exit(void);
 static int get_reply(int fd, struct audit_reply *rep, int seq);
+static char *getsubj(char *subj);

 enum startup_state {startup_disable=0, startup_enable, startup_nochange, 
startup_INVALID};
 static const char *startup_states[] = {"disable", "enable", "nochange"};
@@ -347,11 +350,15 @@ static void netlink_handler( struct ev_loop *loop, struct 
ev_io *io, int revents
 	if (rep == NULL) {
 		if ((rep = malloc(sizeof(*rep))) == NULL) {
 			char emsg[DEFAULT_BUF_SZ];
-			snprintf(emsg, sizeof(emsg),
-				 "auditd error halt, auid=%u pid=%d res=failed",
-				 audit_getloginuid(), getpid());
+			if(subj)
+				snprintf(emsg, sizeof(emsg),
+					"auditd error halt, auid=%u pid=%d subj=%s res=failed",
+					audit_getloginuid(), getpid(), subj);
+			else
+				snprintf(emsg, sizeof(emsg),
+					 "auditd error halt, auid=%u pid=%d res=failed",
+					 audit_getloginuid(), getpid());
 			EV_STOP ();
-			//FIXME add subj
 			send_audit_event(AUDIT_DAEMON_ABORT, emsg);
 			audit_msg(LOG_ERR,
 				  "Cannot allocate audit reply, exiting");
@@ -579,12 +586,18 @@ int main(int argc, char *argv[])
 			tell_parent(FAILURE);
 			return 1;
 		}
-//FIXME add SUBJ
-		snprintf(start, sizeof(start),
-		    "auditd start, ver=%s format=%s "
-		    "kernel=%.56s auid=%u pid=%d res=success",
-		     VERSION, fmt, ubuf.release,
-		     audit_getloginuid(), getpid());
+		if(getsubj(subj))
+			snprintf(start, sizeof(start),
+				"auditd start, ver=%s format=%s "
+				"kernel=%.56s auid=%u pid=%d subj=%s res=success",
+				VERSION, fmt, ubuf.release,
+				audit_getloginuid(), getpid(), subj);
+		else
+			snprintf(start, sizeof(start),
+				"auditd start, ver=%s format=%s "
+				"kernel=%.56s auid=%u pid=%d res=success",
+				VERSION, fmt, ubuf.release,
+				audit_getloginuid(), getpid());
 		if (send_audit_event(AUDIT_DAEMON_START, start)) {
         		audit_msg(LOG_ERR, "Cannot send start message");
 			if (pidfile)
@@ -604,11 +617,15 @@ int main(int argc, char *argv[])
 	/* Tell the kernel we are alive */
 	if (audit_set_pid(fd, getpid(), WAIT_YES) < 0) {
 		char emsg[DEFAULT_BUF_SZ];
-		snprintf(emsg, sizeof(emsg),
-			"auditd error halt, auid=%u pid=%d res=failed",
-			audit_getloginuid(), getpid());
+		if(subj)
+			snprintf(emsg, sizeof(emsg),
+				"auditd error halt, auid=%u pid=%d subj=%s res=failed",
+				audit_getloginuid(), getpid(), subj);
+		else
+			snprintf(emsg, sizeof(emsg),
+				"auditd error halt, auid=%u pid=%d res=failed",
+				audit_getloginuid(), getpid());
 		stop = 1;
-//FIXME add subj
 		send_audit_event(AUDIT_DAEMON_ABORT, emsg);
 		audit_msg(LOG_ERR, "Unable to set audit pid, exiting");
 		close_down();
@@ -626,11 +643,15 @@ int main(int argc, char *argv[])
 	if (opt_startup != startup_nochange &&
 	    audit_set_enabled(fd, (int)opt_startup) < 0) {
 		char emsg[DEFAULT_BUF_SZ];
-		snprintf(emsg, sizeof(emsg),
-			"auditd error halt, auid=%u pid=%d res=failed",
-			audit_getloginuid(), getpid());
+		if(subj)
+			snprintf(emsg, sizeof(emsg),
+				"auditd error halt, auid=%u pid=%d subj=%s res=failed",
+				audit_getloginuid(), getpid(), subj);
+		else
+			snprintf(emsg, sizeof(emsg),
+				"auditd error halt, auid=%u pid=%d res=failed",
+				audit_getloginuid(), getpid());
 		stop = 1;
-//FIXME add subj
 		send_audit_event(AUDIT_DAEMON_ABORT, emsg);
 		audit_msg(LOG_ERR,
 			"Unable to set intitial audit startup state to '%s', exiting",
@@ -780,3 +801,25 @@ static int get_reply(int fd, struct audit_reply *rep, int 
seq)
 	return -1;
 }

+//get the subj of the daemon
+static char *getsubj(char *subj)
+{
+	pid_t pid = getpid();
+	char filename[48];
+	ssize_t num_read;
+	int fd;
+
+	snprintf(filename, sizeof(filename), "/proc/%u/attr/current", pid);
+	fd = open(filename, O_RDONLY);
+	if(fd == -1)
+		return NULL;
+	do {
+		num_read = read(fd, subj, SUBJ_LEN-1);
+	} while (num_read < 0 && errno == EINTR);
+	close(fd);
+	if(num_read <= 0)
+		return NULL;
+	subj[num_read] = '\0';
+	return subj;
+}
+

Regards
Chu Li

Re: [PATCH]Fix me add subj

[Steve Grubb]

On Monday 18 August 2008 06:04:25 Chu Li wrote:
> I have made a patch for "Fixme add subj" in auditd.c. This is for the
> latest codes.

Thanks for the patch. I am deferring this until next release to apply. I want 
to get audit-1.7.5 out within the next day.

Thanks,
-Steve

Re: [PATCH]Fix me add subj

[Steve Grubb]

On Monday 18 August 2008 06:04:25 Chu Li wrote:
>   I have made a patch for "Fixme add subj" in auditd.c. This is for the
> latest codes.

Now that the audit svn is open for new work...I started to apply this patch. 
But then I got to thinking about SMACK. It probably does not like us to get 
selinux labels. I was wondering if we need to try to get its label, too? And 
I was wondering if both SE Linux and SMACK could be running at the same time? 
If they can, do we collect both labels? 

-Steve

Re: [PATCH]Fix me add subj

[Stephen Smalley]

On Tue, 2008-08-26 at 15:34 -0400, Steve Grubb wrote:
> On Monday 18 August 2008 06:04:25 Chu Li wrote:
> >   I have made a patch for "Fixme add subj" in auditd.c. This is for the
> > latest codes.
> 
> Now that the audit svn is open for new work...I started to apply this patch. 
> But then I got to thinking about SMACK. It probably does not like us to get 
> selinux labels. I was wondering if we need to try to get its label, too? And 
> I was wondering if both SE Linux and SMACK could be running at the same time? 
> If they can, do we collect both labels? 

They are exclusive of one another, and they both provide the process
label via /proc/pid/attr/current.  libselinux wraps that kernel
interface with getcon() (for current context) and getpidcon() (for
context of a given pid), which internally handle the allocation of the
buffer and will deal with label translation if using mcstransd.

So if you want the code to work with either, you'd directly
read /proc/pid/attr/current and display the resulting string.  If you
want to be SELinux-specific and include functionality like MLS label
translation, you'd use getpidcon(3).

-- 
Stephen Smalley
National Security Agency

Re: [PATCH]Fix me add subj

[Steve Grubb]

On Tuesday 26 August 2008 15:55:51 Stephen Smalley wrote:
> On Tue, 2008-08-26 at 15:34 -0400, Steve Grubb wrote:
> > On Monday 18 August 2008 06:04:25 Chu Li wrote:
> > >   I have made a patch for "Fixme add subj" in auditd.c. This is for the
> > > latest codes.
> >
> > Now that the audit svn is open for new work...I started to apply this
> > patch. But then I got to thinking about SMACK. It probably does not like
> > us to get selinux labels. I was wondering if we need to try to get its
> > label, too? And I was wondering if both SE Linux and SMACK could be
> > running at the same time? If they can, do we collect both labels?
>
> They are exclusive of one another, and they both provide the process
> label via /proc/pid/attr/current.  libselinux wraps that kernel
> interface with getcon() (for current context) and getpidcon() (for
> context of a given pid), which internally handle the allocation of the
> buffer and will deal with label translation if using mcstransd.
>
> So if you want the code to work with either, you'd directly
> read /proc/pid/attr/current and display the resulting string.  If you
> want to be SELinux-specific and include functionality like MLS label
> translation, you'd use getpidcon(3).

Thanks, that's very helpful. I think we want the raw data and then do context 
translations later in the parsing library if someone asks for it.

-Steve

Re: [PATCH]Fix me add subj

[Matt Anderson]

On Tue, Aug 26, 2008 at 04:08:35PM -0400, Steve Grubb wrote:
> On Tuesday 26 August 2008 15:55:51 Stephen Smalley wrote:
> > So if you want the code to work with either, you'd directly
> > read /proc/pid/attr/current and display the resulting string. ??If you
> > want to be SELinux-specific and include functionality like MLS label
> > translation, you'd use getpidcon(3).
> 
> Thanks, that's very helpful. I think we want the raw data and then do context 
> translations later in the parsing library if someone asks for it.

Can we be sure the delayed translation will be correct?  Maybe I'm
misinterpreting you, but it sounds like your saying that the context
would only be resolved when a user was scanning the audit log.  It seems
to me that by then the policy or the translation could have changed and
although you may have an audit of that event you wouldn't necessarily be
able to reconstruct the context that should appear in the log.

-matt

Re: [PATCH]Fix me add subj

[Steve Grubb]

On Wednesday 27 August 2008 12:04:26 Matt Anderson wrote:
> On Tue, Aug 26, 2008 at 04:08:35PM -0400, Steve Grubb wrote:
> > On Tuesday 26 August 2008 15:55:51 Stephen Smalley wrote:
> > > So if you want the code to work with either, you'd directly
> > > read /proc/pid/attr/current and display the resulting string. ??If you
> > > want to be SELinux-specific and include functionality like MLS label
> > > translation, you'd use getpidcon(3).
> >
> > Thanks, that's very helpful. I think we want the raw data and then do
> > context translations later in the parsing library if someone asks for it.
>
> Can we be sure the delayed translation will be correct?

I don't plan to add translations any time soon. We also don't have time to do 
a translation while logging. So, we will just have raw data for a while.


> It seems to me that by then the policy or the translation could have changed
> and although you may have an audit of that event you wouldn't necessarily be
> able to reconstruct the context that should appear in the log.

True and something that will need to be worked around.

-Steve

Re: [PATCH]Fix me add subj

[Casey Schaufler]

Steve Grubb wrote:
> On Tuesday 26 August 2008 15:55:51 Stephen Smalley wrote:
>   
>> On Tue, 2008-08-26 at 15:34 -0400, Steve Grubb wrote:
>>     
>>> On Monday 18 August 2008 06:04:25 Chu Li wrote:
>>>       
>>>>   I have made a patch for "Fixme add subj" in auditd.c. This is for the
>>>> latest codes.
>>>>         
>>> Now that the audit svn is open for new work...I started to apply this
>>> patch. But then I got to thinking about SMACK. It probably does not like
>>> us to get selinux labels. I was wondering if we need to try to get its
>>> label, too? And I was wondering if both SE Linux and SMACK could be
>>> running at the same time? If they can, do we collect both labels?
>>>       
>> They are exclusive of one another, and they both provide the process
>> label via /proc/pid/attr/current.  libselinux wraps that kernel
>> interface with getcon() (for current context) and getpidcon() (for
>> context of a given pid), which internally handle the allocation of the
>> buffer and will deal with label translation if using mcstransd.
>>
>> So if you want the code to work with either, you'd directly
>> read /proc/pid/attr/current and display the resulting string.  If you
>> want to be SELinux-specific and include functionality like MLS label
>> translation, you'd use getpidcon(3).
>>     
>
> Thanks, that's very helpful. I think we want the raw data and then do context 
> translations later in the parsing library if someone asks for it

That works for Smack.

Thank you.

Re: [PATCH]Fix me add subj

[Steve Grubb]

On Monday 18 August 2008 06:04:25 Chu Li wrote:
>   I have made a patch for "Fixme add subj" in auditd.c. This is for the
> latest codes.

Applied. Thanks for the patch!

-Steve