Red Hat's passwd

Red Hat's passwd

[Joshua Brindle]

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

The attached patch removes the selinux code from Red Hat's passwd.c and 
uses the libselinux function checkPasswdAccess. Aside from eliminating 
some duplication this will make it easier for passwd to use the 
userspace security server with dynamic object classes since it will only 
require a libselinux change and both shadows passwd and Red Hats passwd 
will benefit.

Let me know if there are problems merging this into mainline.

Joshua

[-- Attachment #2: passwd-0.70-checkaccess.patch --]
[-- Type: text/plain, Size: 4363 bytes --]

diff -purN passwd-0.70/passwd.c passwd-0.70-new/passwd.c
--- passwd-0.70/passwd.c	2005-06-17 05:53:50.000000000 -0400
+++ passwd-0.70-new/passwd.c	2005-08-05 11:03:11.213620856 -0400
@@ -54,7 +54,6 @@
 #include <selinux/selinux.h>
 #include <selinux/context.h>
 #include <selinux/av_permissions.h>
-#include "selinux_utils.h"
 #endif
 
 #ifdef WITH_AUDIT
@@ -366,7 +365,8 @@ main(int argc, const char **argv)
 #ifdef WITH_SELINUX
 	if ((is_selinux_enabled() > 0) &&
 	    (getuid() == 0) &&
-	    (check_selinux_access(username, pwd->pw_uid, PASSWD__PASSWD) != 0)) {
+	    (pwd->pw_uid != getuid ()) &&
+	    (checkPasswdAccess(PASSWD__PASSWD) != 0)) {
 		security_context_t user_context;
 		if (getprevcon(&user_context) < 0) {
 			user_context = strdup(_("Unknown user context"));
diff -purN passwd-0.70/selinux_utils.c passwd-0.70-new/selinux_utils.c
--- passwd-0.70/selinux_utils.c	2005-01-25 17:00:49.000000000 -0500
+++ passwd-0.70-new/selinux_utils.c	1969-12-31 19:00:00.000000000 -0500
@@ -1,82 +0,0 @@
-/*
- * Copyright Red Hat, Inc., 2003,2004.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, and the entire permission notice in its entirety,
- *    including the disclaimer of warranties.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- * 3. The name of the author may not be used to endorse or promote
- *    products derived from this software without specific prior
- *    written permission.
- *
- * ALTERNATIVELY, this product may be distributed under the terms of
- * the GNU Public License, in which case the provisions of the GPL are
- * required INSTEAD OF the above restrictions.  (This clause is
- * necessary due to a potential bad interaction between the GPL and
- * the restrictions contained in a BSD-style copyright.)
- *
- * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
- * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
- * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
- * DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT,
- * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
- * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
- * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
- * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
- * OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-/* Written by Daniel Walsh <dwalsh@redhat.com> */
-
-#include "config.h"
-#include <sys/types.h>
-#include <stdio.h>
-#include <string.h>
-#include <selinux/selinux.h>
-#include <selinux/flask.h>
-#include <selinux/av_permissions.h>
-#include <selinux/context.h>
-#include "selinux_utils.h"
-
-int
-check_selinux_access(const char *change_user, int change_uid, unsigned int access)
-{
-	int status = -1;
-	security_context_t user_context;
-	const char *user;
-
-	if (security_getenforce() == 0) {
-		status = 0;
-	} else {
-		if (getprevcon(&user_context) == 0) {
-			context_t c;
-			c = context_new(user_context);
-			user = context_user_get(c);
-			if (change_uid != 0 && strcmp(change_user, user) == 0) {
-				status = 0;
-			} else {
-				struct av_decision avd;
-				int retval;
-				retval = security_compute_av(user_context,
-							     user_context,
-							     SECCLASS_PASSWD,
-							     access,
-							     &avd);
-				if ((retval == 0) && 
-				    ((access & avd.allowed) == access)) {
-					status = 0;
-				}
-			}
-			context_free(c);
-			freecon(user_context);
-		}
-	}
-	return status;
-}
diff -purN passwd-0.70/selinux_utils.h passwd-0.70-new/selinux_utils.h
--- passwd-0.70/selinux_utils.h	2005-01-25 17:00:49.000000000 -0500
+++ passwd-0.70-new/selinux_utils.h	1969-12-31 19:00:00.000000000 -0500
@@ -1,4 +0,0 @@
-#ifndef __RH_SELINUX_UTILS_H
-#define __RH_SELINUX_UTILS_H
-int check_selinux_access(const char *change_user, int change_uid, unsigned int access);
-#endif

Re: Red Hat's passwd

[Colin Walters]

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

On Fri, 2005-08-05 at 12:14 -0400, Joshua Brindle wrote:
> The attached patch removes the selinux code from Red Hat's passwd.c and 
> uses the libselinux function checkPasswdAccess. 

This reminds me...there was some griping from people maintaining modules
which use libselinux about the name "checkPasswdAccess" in a shared
library targeted for wide use; could we get some namespacing here?  This
is a good thing for preventing symbol collisions and also for aesthetic
and consistency reasons.  Perhaps avc_check_passwd_permission or
security_check_passwd_permission or something?

Renaming the function would be an API/ABI break, but it doesn't seem
like a function that could be applicable for more than a few programs at
most.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Red Hat's passwd

[Joshua Brindle]

Colin Walters wrote:

>On Fri, 2005-08-05 at 12:14 -0400, Joshua Brindle wrote:
>  
>
>>The attached patch removes the selinux code from Red Hat's passwd.c and 
>>uses the libselinux function checkPasswdAccess. 
>>    
>>
>
>This reminds me...there was some griping from people maintaining modules
>which use libselinux about the name "checkPasswdAccess" in a shared
>library targeted for wide use; could we get some namespacing here?  This
>is a good thing for preventing symbol collisions and also for aesthetic
>and consistency reasons.  Perhaps avc_check_passwd_permission or
>security_check_passwd_permission or something?
>
>  
>
yea, thats a bad name, and it has caps!@

>Renaming the function would be an API/ABI break, but it doesn't seem
>like a function that could be applicable for more than a few programs at
>most.
>  
>
yea, the problem is really upstreaming the change in whatever packages 
use it (shadow, and hopefully RH passwd)
and some of Red Hats patches for things like vixie cron will also need 
to be fixed.

Personally I'd rather not deal with a major version change needed for an 
api change. Granted it shouldn't be named that but it isn't crucial to 
fix it. We probably need to be more careful about what functions are 
exported in the future.

Also, util-linux needs to be patched to use checkPasswdAccess (or 
whatever) rather than internally (which looks surprisingly similar to 
checkPasswdAccess) and there are probably more apps I'm missing.




--
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: Red Hat's passwd

[Colin Walters]

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

On Fri, 2005-08-05 at 14:16 -0400, Joshua Brindle wrote:

> yea, the problem is really upstreaming the change in whatever packages 
> use it (shadow, and hopefully RH passwd)
>
> and some of Red Hats patches for things like vixie cron will also need 
> to be fixed.

Ah, hm.  I can see how that is potentially harder.  Are those patches
already really upstreamed (i.e. in their tarballs) though?  Looking in
Fedora it looks like the vixie-cron SELinux support is still a separate
patch for example.

> Personally I'd rather not deal with a major version change needed for an 
> api change. 

Well I guess I was arguing that we should be evil and not bump the
soname for it and just fix the few apps that are using it :)  We know
what they are and are in control of the code.

If we are talking about bumping soname, there are other changes we
should probably make too.

> We probably need to be more careful about what functions are 
> exported in the future.

Yes.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Red Hat's passwd

[Daniel J Walsh]

Joshua Brindle wrote:

> Colin Walters wrote:
>
>> On Fri, 2005-08-05 at 12:14 -0400, Joshua Brindle wrote:
>>  
>>
>>> The attached patch removes the selinux code from Red Hat's passwd.c 
>>> and uses the libselinux function checkPasswdAccess.   
>>
>>
>> This reminds me...there was some griping from people maintaining modules
>> which use libselinux about the name "checkPasswdAccess" in a shared
>> library targeted for wide use; could we get some namespacing here?  This
>> is a good thing for preventing symbol collisions and also for aesthetic
>> and consistency reasons.  Perhaps avc_check_passwd_permission or
>> security_check_passwd_permission or something?
>>
>>  
>>
> yea, thats a bad name, and it has caps!@
>
>> Renaming the function would be an API/ABI break, but it doesn't seem
>> like a function that could be applicable for more than a few programs at
>> most.
>>  
>>
> yea, the problem is really upstreaming the change in whatever packages 
> use it (shadow, and hopefully RH passwd)
> and some of Red Hats patches for things like vixie cron will also need 
> to be fixed.
>
> Personally I'd rather not deal with a major version change needed for 
> an api change. Granted it shouldn't be named that but it isn't crucial 
> to fix it. We probably need to be more careful about what functions 
> are exported in the future.
>
> Also, util-linux needs to be patched to use checkPasswdAccess (or 
> whatever) rather than internally (which looks surprisingly similar to 
> checkPasswdAccess) and there are probably more apps I'm missing.
>
Lets add Colins' name and then just leave checkPasswdAccess to call it.  
Then remove any reference to the function.

>
>
>
> -- 
> 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.



-- 



--
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: Red Hat's passwd

[Joshua Brindle]

Daniel J Walsh wrote:

> Joshua Brindle wrote:
>
>> Colin Walters wrote:
>>
>>> On Fri, 2005-08-05 at 12:14 -0400, Joshua Brindle wrote:
>>>  
>>>
>>>> The attached patch removes the selinux code from Red Hat's passwd.c 
>>>> and uses the libselinux function checkPasswdAccess.   
>>>
>>>
>>>
>>> This reminds me...there was some griping from people maintaining 
>>> modules
>>> which use libselinux about the name "checkPasswdAccess" in a shared
>>> library targeted for wide use; could we get some namespacing here?  
>>> This
>>> is a good thing for preventing symbol collisions and also for aesthetic
>>> and consistency reasons.  Perhaps avc_check_passwd_permission or
>>> security_check_passwd_permission or something?
>>>
>>>  
>>>
>> yea, thats a bad name, and it has caps!@
>>
>>> Renaming the function would be an API/ABI break, but it doesn't seem
>>> like a function that could be applicable for more than a few 
>>> programs at
>>> most.
>>>  
>>>
>> yea, the problem is really upstreaming the change in whatever 
>> packages use it (shadow, and hopefully RH passwd)
>> and some of Red Hats patches for things like vixie cron will also 
>> need to be fixed.
>>
>> Personally I'd rather not deal with a major version change needed for 
>> an api change. Granted it shouldn't be named that but it isn't 
>> crucial to fix it. We probably need to be more careful about what 
>> functions are exported in the future.
>>
>> Also, util-linux needs to be patched to use checkPasswdAccess (or 
>> whatever) rather than internally (which looks surprisingly similar to 
>> checkPasswdAccess) and there are probably more apps I'm missing.
>>
> Lets add Colins' name and then just leave checkPasswdAccess to call 
> it.  Then remove any reference to the function.
>
Sounds good to me, trivial patch to do this.. Also, I assume you will 
change the previous patch for passwd.c to the new function.

Index: checkAccess.c
===================================================================
--- checkAccess.c       (revision 920)
+++ checkAccess.c       (working copy)
@@ -6,7 +6,7 @@
 #include <selinux/flask.h>
 #include <selinux/av_permissions.h>

-int checkPasswdAccess(access_vector_t requested) {
+int selinux_check_passwd_access(access_vector_t requested) {
         int status=-1;
         security_context_t user_context;
         if (is_selinux_enabled() == 0)
@@ -31,3 +31,7 @@

         return status;
 }
+
+int checkPasswdAccess(access_vector_t requested) {
+       return selinux_check_passwd_access(requested);
+}


--
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: Red Hat's passwd

[Colin Walters]

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

On Fri, 2005-08-05 at 15:07 -0400, Joshua Brindle wrote:

> Sounds good to me, trivial patch to do this.. Also, I assume you will 
> change the previous patch for passwd.c to the new function.

Shouldn't we also remove checkPasswdAccess from the header?  Otherwise
what's the point?

I should note this approach addresses the consistency and namespacing
from the developer point of view, but doesn't address possible symbol
collisions.  Possibly that could wait until a soname bump, just thought
I'd point it out.




[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Red Hat's passwd

[Joshua Brindle]

Colin Walters wrote:

>On Fri, 2005-08-05 at 15:07 -0400, Joshua Brindle wrote:
>
>  
>
>>Sounds good to me, trivial patch to do this.. Also, I assume you will 
>>change the previous patch for passwd.c to the new function.
>>    
>>
>
>Shouldn't we also remove checkPasswdAccess from the header?  Otherwise
>what's the point?
>
>  
>
no but we need to add the other..

The point is to allow some transition time so that both are available 
until everything is fixed, then we can remove the other without having 
any problems.

--- selinux.h   (revision 920)
+++ selinux.h   (working copy)
@@ -290,6 +290,7 @@

 /* Check a permission in the passwd class.
    Return 0 if granted or -1 otherwise. */
+extern int selinux_check_passwd_access(access_vector_t requested);
 extern int checkPasswdAccess(access_vector_t requested);

 /* Set the path to the selinuxfs mount point explicitly.

>I should note this approach addresses the consistency and namespacing
>from the developer point of view, but doesn't address possible symbol
>collisions.  Possibly that could wait until a soname bump, just thought
>I'd point it out.
>  
>


--
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: Red Hat's passwd

[Stephen Smalley]

The patches to rename checkPasswdAccess (while temporarily retaining the
old symbol for compatibility) were merged as of libselinux 1.25.1.  So
if/when people convert all users of it over to the new name, we can wait
a bit and then drop the old symbol altogether.

-- 
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: Red Hat's passwd

[Joshua Brindle]

Stephen Smalley wrote:

>The patches to rename checkPasswdAccess (while temporarily retaining the
>old symbol for compatibility) were merged as of libselinux 1.25.1.  So
>if/when people convert all users of it over to the new name, we can wait
>a bit and then drop the old symbol altogether.
>  
>
Dan, are you going to be able to merge those changes into passwd?

We need to eliminate as many hard coded class references as possible to 
make transitioning to the userspace security server easier.

--
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: Red Hat's passwd

[Daniel J Walsh]

Joshua Brindle wrote:

> Stephen Smalley wrote:
>
>> The patches to rename checkPasswdAccess (while temporarily retaining the
>> old symbol for compatibility) were merged as of libselinux 1.25.1.  So
>> if/when people convert all users of it over to the new name, we can wait
>> a bit and then drop the old symbol altogether.
>>  
>>
> Dan, are you going to be able to merge those changes into passwd?
>
> We need to eliminate as many hard coded class references as possible 
> to make transitioning to the userspace security server easier.

yes

-- 



--
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.