libsemanage patch

libsemanage patch

[Daniel J Walsh]

[-- Attachment #1: Type: text/plain, Size: 296 bytes --]

If you have a homedir that ends in '/', genhomedircon gets confused.

# useradd -h /home2/dwalsh/ dwalsh
# genhomedircon

Check out the labeling.  genhomedircon thinks dwalsh is a toplevel home root.  

We should just get rid of this command...  :^)

Patch removes all trailing '/' from homedir.

[-- Attachment #2: libsemanage-rhat.patch --]
[-- Type: text/plain, Size: 597 bytes --]

diff --exclude-from=exclude -N -u -r nsalibsemanage/src/genhomedircon.c libsemanage-2.0.33/src/genhomedircon.c
--- nsalibsemanage/src/genhomedircon.c	2008-08-28 09:34:24.000000000 -0400
+++ libsemanage-2.0.33/src/genhomedircon.c	2009-07-15 10:32:20.000000000 -0400
@@ -304,6 +304,10 @@
 			continue;
 		if (!semanage_list_find(shells, pwbuf->pw_shell))
 			continue;
+		int len = strlen(pwbuf->pw_dir) -1;
+		for(; len > 0 && pwbuf->pw_dir[len]=='/'; len--) {
+			pwbuf->pw_dir[len]=0;
+		}
 		if (strcmp(pwbuf->pw_dir, "/") == 0)
 			continue;
 		if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)

Re: libsemanage patch

[Chad Sellers]

On 7/15/09 10:36 AM, "Daniel J Walsh" <dwalsh@redhat.com> wrote:

> If you have a homedir that ends in '/', genhomedircon gets confused.
> 
> # useradd -h /home2/dwalsh/ dwalsh
> # genhomedircon
> 
> Check out the labeling.  genhomedircon thinks dwalsh is a toplevel home root.
> 
> We should just get rid of this command...  :^)
> 
> Patch removes all trailing '/' from homedir.

Acked-by: Chad Sellers <csellers@tresys.com>


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: libsemanage patch

[Joshua Brindle]

Daniel J Walsh wrote:
> If you have a homedir that ends in '/', genhomedircon gets confused.
>
> # useradd -h /home2/dwalsh/ dwalsh
> # genhomedircon
>
> Check out the labeling.  genhomedircon thinks dwalsh is a toplevel home root.
>
> We should just get rid of this command...  :^)
>
> Patch removes all trailing '/' from homedir.

> diff --exclude-from=exclude -N -u -r nsalibsemanage/src/genhomedircon.c libsemanage-2.0.33/src/genhomedircon.c
> --- nsalibsemanage/src/genhomedircon.c	2008-08-28 09:34:24.000000000 -0400
> +++ libsemanage-2.0.33/src/genhomedircon.c	2009-07-15 10:32:20.000000000 -0400
> @@ -304,6 +304,10 @@
>  			continue;
>  		if (!semanage_list_find(shells, pwbuf->pw_shell))
>  			continue;
> +		int len = strlen(pwbuf->pw_dir) -1;
> +		for(; len > 0 && pwbuf->pw_dir[len]=='/'; len--) {
> +			pwbuf->pw_dir[len]=0;
> +		}
>  		if (strcmp(pwbuf->pw_dir, "/") == 0)
>  			continue;
>  		if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)

Why aren't you just doing:

len = strlen(pwbuf->pwdir);
if (pwbuf->pwdir[len] == '/')
	pwbuf->pwdir[len] = '\0';

?

Also, won't this fail if the homedir is set to '/' ? This check should probably 
go below the strcmp(pwbuf->pw_dir, "/") that is currently below it.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: libsemanage patch

[Daniel J Walsh]

On 09/04/2009 09:56 AM, Joshua Brindle wrote:
> Daniel J Walsh wrote:
>> If you have a homedir that ends in '/', genhomedircon gets confused.
>>
>> # useradd -h /home2/dwalsh/ dwalsh
>> # genhomedircon
>>
>> Check out the labeling.  genhomedircon thinks dwalsh is a toplevel
>> home root.
>>
>> We should just get rid of this command...  :^)
>>
>> Patch removes all trailing '/' from homedir.
> 
>> diff --exclude-from=exclude -N -u -r
>> nsalibsemanage/src/genhomedircon.c libsemanage-2.0.33/src/genhomedircon.c
>> --- nsalibsemanage/src/genhomedircon.c    2008-08-28
>> 09:34:24.000000000 -0400
>> +++ libsemanage-2.0.33/src/genhomedircon.c    2009-07-15
>> 10:32:20.000000000 -0400
>> @@ -304,6 +304,10 @@
>>              continue;
>>          if (!semanage_list_find(shells, pwbuf->pw_shell))
>>              continue;
>> +        int len = strlen(pwbuf->pw_dir) -1;
>> +        for(; len > 0 && pwbuf->pw_dir[len]=='/'; len--) {
>> +            pwbuf->pw_dir[len]=0;
>> +        }
>>          if (strcmp(pwbuf->pw_dir, "/") == 0)
>>              continue;
>>          if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
> 
> Why aren't you just doing:
> 
> len = strlen(pwbuf->pwdir);
> if (pwbuf->pwdir[len] == '/')
>     pwbuf->pwdir[len] = '\0';
> 
> ?
> 
What about /home/dwalsh//////
Which I believe is legal
> Also, won't this fail if the homedir is set to '/' ? This check should
> probably go below the strcmp(pwbuf->pw_dir, "/") that is currently below
> it.
Yes good point.

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: libsemanage patch

[Chad Sellers]

On 9/7/09 6:44 AM, "Daniel J Walsh" <dwalsh@redhat.com> wrote:

> On 09/04/2009 09:56 AM, Joshua Brindle wrote:
>> Daniel J Walsh wrote:
>>> If you have a homedir that ends in '/', genhomedircon gets confused.
>>> 
>>> # useradd -h /home2/dwalsh/ dwalsh
>>> # genhomedircon
>>> 
>>> Check out the labeling.  genhomedircon thinks dwalsh is a toplevel
>>> home root.
>>> 
>>> We should just get rid of this command...  :^)
>>> 
>>> Patch removes all trailing '/' from homedir.
>> 
>>> diff --exclude-from=exclude -N -u -r
>>> nsalibsemanage/src/genhomedircon.c libsemanage-2.0.33/src/genhomedircon.c
>>> --- nsalibsemanage/src/genhomedircon.c    2008-08-28
>>> 09:34:24.000000000 -0400
>>> +++ libsemanage-2.0.33/src/genhomedircon.c    2009-07-15
>>> 10:32:20.000000000 -0400
>>> @@ -304,6 +304,10 @@
>>>              continue;
>>>          if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>              continue;
>>> +        int len = strlen(pwbuf->pw_dir) -1;
>>> +        for(; len > 0 && pwbuf->pw_dir[len]=='/'; len--) {
>>> +            pwbuf->pw_dir[len]=0;
>>> +        }
>>>          if (strcmp(pwbuf->pw_dir, "/") == 0)
>>>              continue;
>>>          if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
>> 
>> Why aren't you just doing:
>> 
>> len = strlen(pwbuf->pwdir);
>> if (pwbuf->pwdir[len] == '/')
>>     pwbuf->pwdir[len] = '\0';
>> 
>> ?
>> 
> What about /home/dwalsh//////
> Which I believe is legal
>> Also, won't this fail if the homedir is set to '/' ? This check should
>> probably go below the strcmp(pwbuf->pw_dir, "/") that is currently below
>> it.
> Yes good point.
> 
I thought it worked fine with '/'. Since the loop condition is len > 0
(where len is the index of the last character, not the length of the string,
which is a bit confusing and should probably be changed), it would never
reset the first character. And don't you want this before the
strcmp(pwbuf->pw_dir, "/") in case the path is ///// (which would of course
be silly, but I believe is legal anyway)?

Chad


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: libsemanage patch

[Joshua Brindle]

[-- Attachment #1: Type: text/plain, Size: 2261 bytes --]



Chad Sellers wrote:
> On 9/7/09 6:44 AM, "Daniel J Walsh"<dwalsh@redhat.com>  wrote:
>
>    
>> On 09/04/2009 09:56 AM, Joshua Brindle wrote:
>>      
>>> Daniel J Walsh wrote:
>>>        
>>>> If you have a homedir that ends in '/', genhomedircon gets confused.
>>>>
>>>> # useradd -h /home2/dwalsh/ dwalsh
>>>> # genhomedircon
>>>>
>>>> Check out the labeling.  genhomedircon thinks dwalsh is a toplevel
>>>> home root.
>>>>
>>>> We should just get rid of this command...  :^)
>>>>
>>>> Patch removes all trailing '/' from homedir.
>>>>          
>>>> diff --exclude-from=exclude -N -u -r
>>>> nsalibsemanage/src/genhomedircon.c libsemanage-2.0.33/src/genhomedircon.c
>>>> --- nsalibsemanage/src/genhomedircon.c    2008-08-28
>>>> 09:34:24.000000000 -0400
>>>> +++ libsemanage-2.0.33/src/genhomedircon.c    2009-07-15
>>>> 10:32:20.000000000 -0400
>>>> @@ -304,6 +304,10 @@
>>>>               continue;
>>>>           if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>>               continue;
>>>> +        int len = strlen(pwbuf->pw_dir) -1;
>>>> +        for(; len>  0&&  pwbuf->pw_dir[len]=='/'; len--) {
>>>> +            pwbuf->pw_dir[len]=0;
>>>> +        }
>>>>           if (strcmp(pwbuf->pw_dir, "/") == 0)
>>>>               continue;
>>>>           if (semanage_str_count(pwbuf->pw_dir, '/')<= 1)
>>>>          
>>> Why aren't you just doing:
>>>
>>> len = strlen(pwbuf->pwdir);
>>> if (pwbuf->pwdir[len] == '/')
>>>      pwbuf->pwdir[len] = '\0';
>>>
>>> ?
>>>
>>>        
>> What about /home/dwalsh//////
>> Which I believe is legal
>>      
>>> Also, won't this fail if the homedir is set to '/' ? This check should
>>> probably go below the strcmp(pwbuf->pw_dir, "/") that is currently below
>>> it.
>>>        
>> Yes good point.
>>
>>      
> I thought it worked fine with '/'. Since the loop condition is len>  0
> (where len is the index of the last character, not the length of the string,
> which is a bit confusing and should probably be changed), it would never
> reset the first character. And don't you want this before the
> strcmp(pwbuf->pw_dir, "/") in case the path is ///// (which would of course
> be silly, but I believe is legal anyway)?
>
> Chad
>
>    

Yep, got it.

Acked-By: Joshua Brindle <method@manicmethod.com>

[-- Attachment #2: Type: text/html, Size: 3017 bytes --]

Re: libsemanage patch

[Joshua Brindle]

[-- Attachment #1: Type: text/plain, Size: 2769 bytes --]



Joshua Brindle wrote:
>
>
> Chad Sellers wrote:
>> On 9/7/09 6:44 AM, "Daniel J Walsh"<dwalsh@redhat.com>  wrote:
>>
>>    
>>> On 09/04/2009 09:56 AM, Joshua Brindle wrote:
>>>      
>>>> Daniel J Walsh wrote:
>>>>        
>>>>> If you have a homedir that ends in '/', genhomedircon gets confused.
>>>>>
>>>>> # useradd -h /home2/dwalsh/ dwalsh
>>>>> # genhomedircon
>>>>>
>>>>> Check out the labeling.  genhomedircon thinks dwalsh is a toplevel
>>>>> home root.
>>>>>
>>>>> We should just get rid of this command...  :^)
>>>>>
>>>>> Patch removes all trailing '/' from homedir.
>>>>>          
>>>>> diff --exclude-from=exclude -N -u -r
>>>>> nsalibsemanage/src/genhomedircon.c libsemanage-2.0.33/src/genhomedircon.c
>>>>> --- nsalibsemanage/src/genhomedircon.c    2008-08-28
>>>>> 09:34:24.000000000 -0400
>>>>> +++ libsemanage-2.0.33/src/genhomedircon.c    2009-07-15
>>>>> 10:32:20.000000000 -0400
>>>>> @@ -304,6 +304,10 @@
>>>>>               continue;
>>>>>           if (!semanage_list_find(shells, pwbuf->pw_shell))
>>>>>               continue;
>>>>> +        int len = strlen(pwbuf->pw_dir) -1;
>>>>> +        for(; len>  0&&  pwbuf->pw_dir[len]=='/'; len--) {
>>>>> +            pwbuf->pw_dir[len]=0;
>>>>> +        }
>>>>>           if (strcmp(pwbuf->pw_dir, "/") == 0)
>>>>>               continue;
>>>>>           if (semanage_str_count(pwbuf->pw_dir, '/')<= 1)
>>>>>          
>>>> Why aren't you just doing:
>>>>
>>>> len = strlen(pwbuf->pwdir);
>>>> if (pwbuf->pwdir[len] == '/')
>>>>      pwbuf->pwdir[len] = '\0';
>>>>
>>>> ?
>>>>
>>>>        
>>> What about /home/dwalsh//////
>>> Which I believe is legal
>>>      
>>>> Also, won't this fail if the homedir is set to '/' ? This check should
>>>> probably go below the strcmp(pwbuf->pw_dir, "/") that is currently below
>>>> it.
>>>>        
>>> Yes good point.
>>>
>>>      
>> I thought it worked fine with '/'. Since the loop condition is len>  0
>> (where len is the index of the last character, not the length of the string,
>> which is a bit confusing and should probably be changed), it would never
>> reset the first character. And don't you want this before the
>> strcmp(pwbuf->pw_dir, "/") in case the path is ///// (which would of course
>> be silly, but I believe is legal anyway)?
>>
>> Chad
>>
>>    
>
> Yep, got it.
>
> Acked-By: Joshua Brindle <method@manicmethod.com>

Sorry, premature ack.

I'm not seeing the expected results here. I added a user with a homedir 
of /home/method///// and the file_contexts.homedirs still has:

/home/method//////.+ staff_u:object_r:user_home_t:s0


Also, you should probably use '\0' rather than 0 when truncating the 
path. And does this get rid of the need of:

                 semanage_rtrim(path, '/');

a few lines below your patch?



[-- Attachment #2: Type: text/html, Size: 3845 bytes --]

Re: libsemanage patch

[Daniel J Walsh]

[-- Attachment #1: Type: text/plain, Size: 42 bytes --]

Updated patch.  Need check in two places.

[-- Attachment #2: libsemanage-genhomedir.patch --]
[-- Type: text/plain, Size: 826 bytes --]

--- nsalibsemanage/src/genhomedircon.c	2008-08-28 09:34:24.000000000 -0400
+++ libsemanage-2.0.37/src/genhomedircon.c	2009-09-16 13:25:43.000000000 -0400
@@ -304,6 +304,10 @@
 			continue;
 		if (!semanage_list_find(shells, pwbuf->pw_shell))
 			continue;
+		int len = strlen(pwbuf->pw_dir) -1;
+		for(; len > 0 && pwbuf->pw_dir[len] == '/'; len--) {
+			pwbuf->pw_dir[len] = '\0';
+		}
 		if (strcmp(pwbuf->pw_dir, "/") == 0)
 			continue;
 		if (semanage_str_count(pwbuf->pw_dir, '/') <= 1)
@@ -788,6 +792,11 @@
 			continue;
 		}
 
+		int len = strlen(pwent->pw_dir) -1;
+		for(; len > 0 && pwent->pw_dir[len] == '/'; len--) {
+			pwent->pw_dir[len] = '\0';
+		}
+
 		if (strcmp(pwent->pw_dir, "/") == 0) {
 			/* don't relabel / genhomdircon checked to see if root
 			 * was the user and if so, set his home directory to

Re: libsemanage patch

[Joshua Brindle]

Daniel J Walsh wrote:
> Updated patch.  Need check in two places.

merged in libsemanage 2.0.38

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

libsemanage patch

[Daniel J Walsh]

[-- Attachment #1: Type: text/plain, Size: 531 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

genhomedircon includes the "\n" in /etc/shells so no shells in the
/etc/passwd match.

Rawhide Policy includes policy without a user_context_tpl

swig causes a doublefree if I don't allocate memory when specifying a
alternate store.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFHVFmZrlYvE4MpobMRArfoAJ460UxLWEFjgmQx6CewOcTSGxivywCdGcGL
GDS/6bPpBJRQfiVlOwWPvBI=
=cvuG
-----END PGP SIGNATURE-----

[-- Attachment #2: libsemanage-rhat.patch --]
[-- Type: text/x-patch, Size: 2304 bytes --]

diff --exclude-from=exclude -N -u -r nsalibsemanage/src/genhomedircon.c libsemanage-2.0.14/src/genhomedircon.c
--- nsalibsemanage/src/genhomedircon.c	2007-10-05 13:09:53.000000000 -0400
+++ libsemanage-2.0.14/src/genhomedircon.c	2007-12-03 14:20:34.000000000 -0500
@@ -135,6 +135,7 @@
 	if (!shells)
 		return default_shell_list();
 	while (getline(&temp, &buff_len, shells) >= 0) {
+		temp[strlen(temp)-1]=0;
 		if (strcmp(temp, PATH_NOLOGIN_SHELL)) {
 			if (semanage_list_push(&list, temp)) {
 				free(temp);
@@ -790,7 +791,7 @@
 	homedir_context_tpl = make_template(s, &HOME_DIR_PRED);
 	homeroot_context_tpl = make_template(s, &HOME_ROOT_PRED);
 	user_context_tpl = make_template(s, &USER_CONTEXT_PRED);
-	if (!homedir_context_tpl || !homeroot_context_tpl || !user_context_tpl) {
+	if (!homedir_context_tpl || !homeroot_context_tpl) {
 		retval = STATUS_ERR;
 		goto done;
 	}
@@ -828,16 +829,18 @@
 
 		ustr_sc_free(&temp);
 	}
-	if (write_user_context(s, out, user_context_tpl,
-			       ".*", s->fallback_user,
-			       s->fallback_user_prefix) != STATUS_SUCCESS) {
-		retval = STATUS_ERR;
-		goto done;
-	}
+	if (user_context_tpl) {
+		if (write_user_context(s, out, user_context_tpl,
+				       ".*", s->fallback_user,
+				       s->fallback_user_prefix) != STATUS_SUCCESS) {
+			retval = STATUS_ERR;
+			goto done;
+		}
 
-	if (write_gen_home_dir_context(s, out, user_context_tpl,
-				       homedir_context_tpl) != STATUS_SUCCESS) {
-		retval = STATUS_ERR;
+		if (write_gen_home_dir_context(s, out, user_context_tpl,
+					       homedir_context_tpl) != STATUS_SUCCESS) {
+			retval = STATUS_ERR;
+		}
 	}
 
       done:
diff --exclude-from=exclude -N -u -r nsalibsemanage/src/handle.c libsemanage-2.0.14/src/handle.c
--- nsalibsemanage/src/handle.c	2007-08-20 19:15:37.000000000 -0400
+++ libsemanage-2.0.14/src/handle.c	2007-11-10 06:21:33.000000000 -0500
@@ -27,6 +27,7 @@
 #include <assert.h>
 #include <stdlib.h>
 #include <stdio.h>
+#include <string.h>
 #include <sys/time.h>
 
 #include "direct_api.h"
@@ -131,7 +132,7 @@
 
 	/* This just sets the storename to what the user requests, no 
 	   verification of existance will be done until connect */
-	sh->conf->store_path = storename;
+	sh->conf->store_path = strdup(storename);
 	sh->conf->store_type = storetype;
 
 	return;

[-- Attachment #3: libsemanage-rhat.patch.sig --]
[-- Type: application/octet-stream, Size: 65 bytes --]

RE: libsemanage patch

[Todd Miller]

Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> genhomedircon includes the "\n" in /etc/shells so no shells in the
> /etc/passwd match.

Isn't this going to cause problems if the last line in /etc/shells has
no newline?

Instead of:
    temp[strlen(temp)-1]=0;

I would use:
    temp[strcspn(temp, "\n")] = '\0';

That will overwrite the first newline with a NUL or, if there is no
newline, the terminating NUL will be overwritten with another NUL, which
is harmless.  It is a useful idiom...

 - todd


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

RE: libsemanage patch

[Stephen Smalley]

On Mon, 2007-12-03 at 14:51 -0500, Todd Miller wrote:
> Daniel J Walsh wrote:
> > -----BEGIN PGP SIGNED MESSAGE-----
> > Hash: SHA1
> > 
> > genhomedircon includes the "\n" in /etc/shells so no shells in the
> > /etc/passwd match.
> 
> Isn't this going to cause problems if the last line in /etc/shells has
> no newline?
> 
> Instead of:
>     temp[strlen(temp)-1]=0;
> 
> I would use:
>     temp[strcspn(temp, "\n")] = '\0';
> 
> That will overwrite the first newline with a NUL or, if there is no
> newline, the terminating NUL will be overwritten with another NUL, which
> is harmless.  It is a useful idiom...

Given that getline() returns the length read (not to be confused with
the buffer length), why not just:
	while ((len = getline(&temp, &buff_len, shells)) > 0) {
		if (temp[len-1] == '\n') temp[len-1] = 0;

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

RE: libsemanage patch

[Todd Miller]

Stephen Smalley wrote:
> Given that getline() returns the length read (not to be confused with
> the buffer length), why not just:
> 	while ((len = getline(&temp, &buff_len, shells)) > 0) {
> 		if (temp[len-1] == '\n') temp[len-1] = 0;

That is probably the simplest fix.

 - todd


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: libsemanage patch

[Daniel J Walsh]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Todd Miller wrote:
> Stephen Smalley wrote:
>> Given that getline() returns the length read (not to be confused with
>> the buffer length), why not just:
>> 	while ((len = getline(&temp, &buff_len, shells)) > 0) {
>> 		if (temp[len-1] == '\n') temp[len-1] = 0;
> 
> That is probably the simplest fix.
> 
>  - todd
Fine with me,  Do you want another patch.


-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFHVGJJrlYvE4MpobMRApYvAJ9IVdhR+SwUqZVdaTSayldB0LwQoACgyK4y
JovC/Zp6veX/jbP2m+qPbv0=
=5Azc
-----END PGP SIGNATURE-----

--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.

Re: libsemanage patch

[Daniel J Walsh]

[-- Attachment #1: Type: text/plain, Size: 1174 bytes --]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Stephen Smalley wrote:
> On Mon, 2007-12-03 at 14:51 -0500, Todd Miller wrote:
>> Daniel J Walsh wrote:
>>> -----BEGIN PGP SIGNED MESSAGE-----
>>> Hash: SHA1
>>>
>>> genhomedircon includes the "\n" in /etc/shells so no shells in the
>>> /etc/passwd match.
>> Isn't this going to cause problems if the last line in /etc/shells has
>> no newline?
>>
>> Instead of:
>>     temp[strlen(temp)-1]=0;
>>
>> I would use:
>>     temp[strcspn(temp, "\n")] = '\0';
>>
>> That will overwrite the first newline with a NUL or, if there is no
>> newline, the terminating NUL will be overwritten with another NUL, which
>> is harmless.  It is a useful idiom...
> 
> Given that getline() returns the length read (not to be confused with
> the buffer length), why not just:
> 	while ((len = getline(&temp, &buff_len, shells)) > 0) {
> 		if (temp[len-1] == '\n') temp[len-1] = 0;
> 
Second try.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFHVGvnrlYvE4MpobMRAhxfAJ4vuvC1uijEUDqFdj8Ju+cTJQIqoQCeMVqP
086ZRyvnHu9fsF7V8hLeEyI=
=jyGo
-----END PGP SIGNATURE-----

[-- Attachment #2: libsemanage-rhat.patch --]
[-- Type: text/x-patch, Size: 2512 bytes --]

diff --exclude-from=exclude -N -u -r nsalibsemanage/src/genhomedircon.c libsemanage-2.0.14/src/genhomedircon.c
--- nsalibsemanage/src/genhomedircon.c	2007-10-05 13:09:53.000000000 -0400
+++ libsemanage-2.0.14/src/genhomedircon.c	2007-12-03 15:47:19.000000000 -0500
@@ -130,11 +130,13 @@
 	char *temp = NULL;
 	semanage_list_t *list = NULL;
 	size_t buff_len = 0;
+	int len;
 
 	shells = fopen(PATH_SHELLS_FILE, "r");
 	if (!shells)
 		return default_shell_list();
-	while (getline(&temp, &buff_len, shells) >= 0) {
+	while ((len = getline(&temp, &buff_len, shells)) > 0) {
+		if (temp[len-1] == '\n') temp[len-1] = 0;
 		if (strcmp(temp, PATH_NOLOGIN_SHELL)) {
 			if (semanage_list_push(&list, temp)) {
 				free(temp);
@@ -790,7 +792,7 @@
 	homedir_context_tpl = make_template(s, &HOME_DIR_PRED);
 	homeroot_context_tpl = make_template(s, &HOME_ROOT_PRED);
 	user_context_tpl = make_template(s, &USER_CONTEXT_PRED);
-	if (!homedir_context_tpl || !homeroot_context_tpl || !user_context_tpl) {
+	if (!homedir_context_tpl || !homeroot_context_tpl) {
 		retval = STATUS_ERR;
 		goto done;
 	}
@@ -828,16 +830,18 @@
 
 		ustr_sc_free(&temp);
 	}
-	if (write_user_context(s, out, user_context_tpl,
-			       ".*", s->fallback_user,
-			       s->fallback_user_prefix) != STATUS_SUCCESS) {
-		retval = STATUS_ERR;
-		goto done;
-	}
+	if (user_context_tpl) {
+		if (write_user_context(s, out, user_context_tpl,
+				       ".*", s->fallback_user,
+				       s->fallback_user_prefix) != STATUS_SUCCESS) {
+			retval = STATUS_ERR;
+			goto done;
+		}
 
-	if (write_gen_home_dir_context(s, out, user_context_tpl,
-				       homedir_context_tpl) != STATUS_SUCCESS) {
-		retval = STATUS_ERR;
+		if (write_gen_home_dir_context(s, out, user_context_tpl,
+					       homedir_context_tpl) != STATUS_SUCCESS) {
+			retval = STATUS_ERR;
+		}
 	}
 
       done:
diff --exclude-from=exclude -N -u -r nsalibsemanage/src/handle.c libsemanage-2.0.14/src/handle.c
--- nsalibsemanage/src/handle.c	2007-08-20 19:15:37.000000000 -0400
+++ libsemanage-2.0.14/src/handle.c	2007-11-10 06:21:33.000000000 -0500
@@ -27,6 +27,7 @@
 #include <assert.h>
 #include <stdlib.h>
 #include <stdio.h>
+#include <string.h>
 #include <sys/time.h>
 
 #include "direct_api.h"
@@ -131,7 +132,7 @@
 
 	/* This just sets the storename to what the user requests, no 
 	   verification of existance will be done until connect */
-	sh->conf->store_path = storename;
+	sh->conf->store_path = strdup(storename);
 	sh->conf->store_type = storetype;
 
 	return;

[-- Attachment #3: libsemanage-rhat.patch.sig --]
[-- Type: application/octet-stream, Size: 65 bytes --]

Re: libsemanage patch

[Stephen Smalley]

On Mon, 2007-12-03 at 15:49 -0500, Daniel J Walsh wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Stephen Smalley wrote:
> > On Mon, 2007-12-03 at 14:51 -0500, Todd Miller wrote:
> >> Daniel J Walsh wrote:
> >>> -----BEGIN PGP SIGNED MESSAGE-----
> >>> Hash: SHA1
> >>>
> >>> genhomedircon includes the "\n" in /etc/shells so no shells in the
> >>> /etc/passwd match.
> >> Isn't this going to cause problems if the last line in /etc/shells has
> >> no newline?
> >>
> >> Instead of:
> >>     temp[strlen(temp)-1]=0;
> >>
> >> I would use:
> >>     temp[strcspn(temp, "\n")] = '\0';
> >>
> >> That will overwrite the first newline with a NUL or, if there is no
> >> newline, the terminating NUL will be overwritten with another NUL, which
> >> is harmless.  It is a useful idiom...
> > 
> > Given that getline() returns the length read (not to be confused with
> > the buffer length), why not just:
> > 	while ((len = getline(&temp, &buff_len, shells)) > 0) {
> > 		if (temp[len-1] == '\n') temp[len-1] = 0;
> > 
> Second try.

Merged with two changes (type on len => ssize_t, assert on store_path
after strdup so that we don't continue if allocation fails).

-- 
Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to majordomo@tycho.nsa.gov with
the words "unsubscribe selinux" without quotes as the message.