Bug 663445 - Hardcoded SELinux constants in pam_selinux
Hardcoded SELinux constants in pam_selinux
Product: Fedora
Classification: Fedora
Component: pam (Show other bugs)
Unspecified Unspecified
low Severity medium
: ---
: ---
Assigned To: Tomas Mraz
Fedora Extras Quality Assurance
Depends On:
  Show dependency treegraph
Reported: 2010-12-15 15:08 EST by Tomas Mraz
Modified: 2011-11-24 09:16 EST (History)
6 users (show)

See Also:
Fixed In Version: pam-1.1.4-1.fc15
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: 663193
Last Closed: 2011-11-24 09:16:33 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---

Attachments (Terms of Use)

  None (edit)
Description Tomas Mraz 2010-12-15 15:08:47 EST
Similarly to cronie pam_selinux contains hardcoded parameter values for security_compute_av() call.

--- Additional comment from tmraz@redhat.com on 2010-12-15 11:11:36 EST ---

OK if the values are fixed for each boot it makes sense to cache them even if the lookup is cheap, i'll do that.

I'm curious whether other code doesn't contain the hardcoded values as well - f.e. pam_selinux or openssh.

--- Additional comment from dwalsh@redhat.com on 2010-12-15 12:53:01 EST ---

Theoretically, although, this is only going to happen when an app is a policy enforcer.  openssh might do something around checking whether the range is "contained" within the system range.


We have the following code in pam_selinux:

static int mls_range_allowed(pam_handle_t *pamh, security_context_t src, security_context_t dst, int debug)
  struct av_decision avd;
  int retval;
  unsigned int bit = CONTEXT__CONTAINS;
  context_t src_context = context_new (src);
  context_t dst_context = context_new (dst);
  context_range_set(dst_context, context_range_get(src_context));
  if (debug)
    pam_syslog(pamh, LOG_NOTICE, "Checking if %s mls range valid for  %s", dst, context_str(dst_context));

  retval = security_compute_av(context_str(dst_context), dst, SECCLASS_CONTEXT, bit, &avd);
  if (retval || ((bit & avd.allowed) != bit))
    return 0;
  return 1;

How it should be fixed? I suppose the SECCLASS_CONTEXT and CONTEXT__CONTAINS should be un-hardcoded.
Comment 1 Daniel Walsh 2010-12-15 15:22:02 EST
	security_class_t tclass = string_to_security_class("context");
	if (!tclass) return 0;
	access_vector_t bit = string_to_av_perm(tclass, "contains");
	if (!bit) return 0;


  retval = security_compute_av(context_str(dst_context), dst, tclass,
bit, &avd);
Comment 2 Eric Paris 2010-12-16 10:17:20 EST
I told some people some lies earlier and need to make sure they are corrected.  these values can change on selinux policy update.  So you need to check the string_to_* functions before every call to security_compute_av.  The results should not be cached.

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