Bug 237249 - LSPP: polyinstantiation behavior correct and documented
LSPP: polyinstantiation behavior correct and documented
Status: CLOSED ERRATA
Product: Red Hat Enterprise Linux 5
Classification: Red Hat
Component: pam (Show other bugs)
5.0
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tomas Mraz
David Lawrence
: OtherQA
Depends On:
Blocks: RHEL5LSPPCertTracker
  Show dependency treegraph
 
Reported: 2007-04-20 09:28 EDT by Ted X Toth
Modified: 2009-06-19 12:46 EDT (History)
4 users (show)

See Also:
Fixed In Version: RHSA-2007-0555
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-11-07 10:40:36 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
method change patch (1.03 KB, patch)
2007-04-24 16:23 EDT, Ted X Toth
no flags Details | Diff

  None (edit)
Description Ted X Toth 2007-04-20 09:28:43 EDT
Description of problem:
Despite configuring a directory to be polyinstantiated at level sometimes it
gets polyinstantiated by user. So I looked at the code and
I see the reason for the behavior that had confused me partly because it
isn't documented and partly because I don't think it is desired. The code as
described in the following comment is overriding my specified method
if getexeccon fails.
/*
 * This function checks if the calling program has requested context
 * change by calling setexeccon(). If context change is not requested
 * then it does not make sense to polyinstantiate based on context.
 * The return value from this function is used when selecting the
 * polyinstantiation method. If context change is not requested then
 * the polyinstantiation method is set to USER, even if the configuration
 * file lists the method as "context" or "both".
 */
static int ctxt_based_inst_needed(void)

Why if getexeccon fails doesn't it make sense to polyinstantiate based
on context/level? Why not call getcon if getexeccon fails and use that
context instead of switching the method? 

At a minimum this needs to be documented so that users will understand the
behavior. 


Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:
Comment 1 Tomas Mraz 2007-04-20 11:25:28 EDT
It's really questionable what is the best behaviour in the case getexeccon
fails. It would perhaps also make sense to completely fail out.

But I completely agree that the behavior should be documented.
Comment 2 Steve Grubb 2007-04-20 16:26:35 EDT
I'll go for man page updates.
Comment 4 Tomas Mraz 2007-04-24 05:51:05 EDT
Behavior documented in pam-0.99.6.2-3.21.el5.
Comment 5 Steve Grubb 2007-04-24 13:59:16 EDT
Man pages were updated. Taking this off the LSPP tracker.
Comment 6 Ted X Toth 2007-04-24 16:23:10 EDT
Created attachment 153379 [details]
method change patch
Comment 7 Ted X Toth 2007-04-24 16:33:52 EDT
I think this needs more than a doc change so I created a patch. The patch calls
getcon in the event that getexeccon fails and it doesn't change the user
specified method.
Comment 8 Steve Grubb 2007-04-24 16:43:37 EDT
But if getexeccon fails, wouldn't that polyinstantiate based on the daemon's
context instead of the user's intended context? IOW,
system_u:system_r:xdm_t:SystemLow-SystemHigh. That would not be good either.
Comment 9 Tomas Mraz 2007-04-24 18:06:15 EDT
I'm curious if there is a valid application calling pam which wouldn't call
setexeccon and have the proper user's context assigned by other means?

If some app like this exists I think the best solution would be to add an option
which would make pam_namespace to simply call getcon instead of getexeccon. I'm
not really sure we want to change the current behavior unconditionally.
Comment 10 Ted X Toth 2007-04-25 12:16:30 EDT
(In reply to comment #8)
> But if getexeccon fails, wouldn't that polyinstantiate based on the daemon's
> context instead of the user's intended context? IOW,
> system_u:system_r:xdm_t:SystemLow-SystemHigh. That would not be good either.

You're right so I took a crack at the patch. Instead of a straight getcon if
getexeccon fails I'm getting the users default context and using that instead. 

--- Linux-PAM-0.99.7.0/modules/pam_namespace/pam_namespace.c	2007-04-25
10:45:52.000000000 -0500
+++ Linux-PAM-0.99.7.0.new/modules/pam_namespace/pam_namespace.c	2007-04-25
10:45:37.000000000 -0500
@@ -251,21 +251,15 @@
 
     poly.method = NONE;
     if (strcmp(method, "user") == 0) 
-	    poly.method = USER;
+        poly.method = USER;
 
 #ifdef WITH_SELINUX
     if (strcmp(method, "level") == 0) {
-        if (idata->flags & PAMNS_CTXT_BASED_INST)
-            poly.method = LEVEL;
-	else
-            poly.method = USER;
+        poly.method = LEVEL;
     }
 
     if (strcmp(method, "context") == 0) {
-        if (idata->flags & PAMNS_CTXT_BASED_INST)
-            poly.method = CONTEXT;
-	else
-            poly.method = USER;
+        poly.method = CONTEXT;
     }
 
 #endif
@@ -447,6 +441,7 @@
 	int rc = PAM_SUCCESS;
 	security_context_t scon = NULL;
 	security_class_t tclass;
+	char *selinuxuser, *level;
 
 	/*
 	 * Get the security context of the directory to polyinstantiate.
@@ -462,9 +457,29 @@
 
 	rc = getexeccon(&scon);
 	if (rc < 0 || scon == NULL) {
+	    
+	  rc = getseuserbyname(idata->user, &selinuxuser, &level);
+	  if (rc < 0) {
 		pam_syslog(idata->pamh, LOG_ERR, 
-			   "Error getting exec context, %m");
+			   "Error getting selinux user, %m");
 		return PAM_SESSION_ERR;
+	  }
+	  if (idata->flags & PAMNS_DEBUG)
+	    pam_syslog(idata->pamh, LOG_DEBUG,
+		       "selinux user %s level %s", selinuxuser, level);
+	  
+	  rc = get_default_context_with_level(selinuxuser, level, NULL, &scon);
+	  free(selinuxuser);
+	  free(level);
+	  if (rc < 0) {
+		pam_syslog(idata->pamh, LOG_ERR, 
+			   "Error getting selinux user default context, %m");
+		return PAM_SESSION_ERR;
+	  }
+	  if (idata->flags & PAMNS_DEBUG)
+	    pam_syslog(idata->pamh, LOG_DEBUG,
+		       "selinux user default context %s", scon);
+	  
 	}
 
 	/*
@@ -515,6 +530,10 @@
 			pam_syslog(idata->pamh, LOG_ERR, "Unable to set MLS Componant of context");
 			goto fail;
 		}
+		if (idata->flags & PAMNS_DEBUG)
+		  pam_syslog(idata->pamh, LOG_DEBUG,
+			     "context_range_set %s %s", context_str(fcontext), context_str(scontext));
+
 		*i_context=strdup(context_str(fcontext));
 		if (! *i_context) {
 			pam_syslog(idata->pamh, LOG_ERR, "out of memory");
Comment 11 Tomas Mraz 2007-04-25 12:53:13 EDT
That's debatable as well. I'd just like to know why the getexeccon could fail
and if there is any real application using pam_namespace where the getexeccon
fails and it is reasonable to continue. Because in the standard login/sshd/su...
cases the getexeccon simply shouldn't fail.

Comment 12 Ted X Toth 2007-04-25 13:09:47 EDT
gdm which I've configured to use pam_namespace doesn't ever calls setexeccon.
Also I don't believe that su calls setexeccon but I could be wrong.
Comment 13 Tomas Mraz 2007-04-25 13:25:30 EDT
They call it through pam_selinux.
Comment 14 Tomas Mraz 2007-04-25 13:27:58 EDT
Oops... I should have looked at the pam configs first. You're right that they
don't  have pam_selinux in configuration. But probably that should be changed?
Comment 15 Tomas Mraz 2007-04-25 13:42:18 EDT
And looking at the gdm sources it calls setexeccon on its own but after
pam_open_session is called so that's why pam_namespace doesn't see it.

Well then the patch from comment 10 could make sense, however I'd prefer it
being optional.

Thee su case is different because it doesn't change the context at all. So in
its case it would make more sense to get the context using getcon. Again that
could be a different option.

Comment 16 Ted X Toth 2007-04-25 15:18:52 EDT
(In reply to comment #15)
> And looking at the gdm sources it calls setexeccon on its own but after
> pam_open_session is called so that's why pam_namespace doesn't see it.
> 
> Well then the patch from comment 10 could make sense, however I'd prefer it
> being optional.
> 
> Thee su case is different because it doesn't change the context at all. So in
> its case it would make more sense to get the context using getcon. Again that
> could be a different option.
> 
> 
What are you suggesting be the behavior if the option to use the users default
context is not set and in the case of su if the option to use getcon is not set?
Comment 17 Ted X Toth 2007-04-25 15:28:59 EDT
(In reply to comment #15)
> And looking at the gdm sources it calls setexeccon on its own but after
> pam_open_session is called so that's why pam_namespace doesn't see it.
> 
> Well then the patch from comment 10 could make sense, however I'd prefer it
> being optional.
> 
> Thee su case is different because it doesn't change the context at all. So in
> its case it would make more sense to get the context using getcon. Again that
> could be a different option.
> 
> 
What are you suggesting be the behavior if the option to use the users default
context is not set and in the case of su if the option to use getcon is not set?
Comment 18 Tomas Mraz 2007-04-25 15:57:36 EDT
Either the current fallback to just username based polyinst. or returning
PAM_SESSION_ERROR.
Comment 19 Tomas Mraz 2007-04-25 16:02:55 EDT
This bug is about documenting the current behavior for RHEL5. I've created bug
237876 for the future development.
Comment 23 errata-xmlrpc 2007-11-07 10:40:36 EST
An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on the solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2007-0555.html

Note You need to log in before you can comment on or make changes to this bug.