The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

Bug #3999

undef values for the SELinux attributes of the file resource type don't work

Added by Cristian Ciupitu over 4 years ago. Updated over 3 years ago.

Status:ClosedStart date:06/12/2010
Priority:NormalDue date:
Assignee:Sean Millichamp% Done:

0%

Category:SELinux
Target version:2.6.7
Affected Puppet version:2.6.4 Branch:https://github.com/seanmil/puppet/tree/ticket/2.6.x/3999
Keywords:SELinux, undef

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com

This issue is currently not available for export. If you are experiencing the issue described below, please file a new ticket in JIRA. Once a new ticket has been created, please add a link to it that points back to this Redmine ticket.


Description

I’ve reported first this bug on Fedora’s bug tracker – https://bugzilla.redhat.com/show_bug.cgi?id=597285.

site.pp - /etc/puppet/manifests/site.pp (910 Bytes) Cristian Ciupitu, 06/12/2010 12:37 pm


Related issues

Related to Puppet - Feature #3449: exec type should support setting the project on Solaris Closed 03/29/2010

History

#1 Updated by Cristian Ciupitu over 4 years ago

The wiki formatting seems to be broken; here’s the correct link for the lazy ones: https://bugzilla.redhat.com/show_bug.cgi?id=597285

#2 Updated by James Turnbull over 4 years ago

  • Status changed from Unreviewed to Investigating
  • Assignee set to Sean Millichamp

Sean – no idea if you have any time/interest to look?

#3 Updated by Sean Millichamp over 4 years ago

I think I understand the problem. However, I am almost 2 years rusty on my Ruby and Puppet internals at this point and don’t have a ton of time.

I am happy to look at it, but I can’t promise an ETA.

One a first pass, it sounds like one (or both) of the following are correct approaches:

1) Test if a path is a mountpoint, if so, skip checking/applying SELinux permissions 2) Honor “undef” for the permissions, which should cause SELinux to the label on the filesystem and leave it as it is.

#4 Updated by Todd Zullinger over 4 years ago

Sean Millichamp wrote:

I think I understand the problem. However, I am almost 2 years rusty on my Ruby and Puppet internals at this point and don’t have a ton of time.

I am happy to look at it, but I can’t promise an ETA.

What? We need guarantees. This isn’t volunteer work… Oh wait, it is. ;)

Thanks for taking a look Sean, whenever you get some time for it.

One a first pass, it sounds like one (or both) of the following are correct approaches:

1) Test if a path is a mountpoint, if so, skip checking/applying SELinux permissions 2) Honor “undef” for the permissions, which should cause SELinux to the label on the filesystem and leave it as it is.

I think 2 makes sense.

I’m not so sure that special casing mount points does. Or at least, not skipping them unconditionally. Some folks may want to label a mount point. Perhaps defaulting to undef for mount points makes sense? I’m not at all sure of that though, just tossing it out for discussion.

#5 Updated by Sean Millichamp almost 4 years ago

I have finally had a chance to look into this a bit and while I think the #2 approach is the more correct one it won’t be quite that easy.

I’m still getting back up to speed with Puppet’s innards, but my reading of it is that any attribute not specified will internally be undef and subject to the “defaultto” in the type, which is where this value is getting automatically provided from in the first place, so explicitly setting them to undef will not help.

I believe the best way to handle this is to add a new attribute for the file type called something like “selinux_use_default_context” and if that is true (the default) then it will ask the system what the contexts should be if they are not specified and if it is false then no defaults will be provided internally in Puppet (and therefore should be left undef) and no changes should occur.

Cristian’s file section of the definition would then look like:

file { “${location}/${name}”: ensure => directory, owner => undef, group => undef, mode => undef, selinux_use_default_context => false, }

I don’t foresee this particular attribute being used too often, so I’m more okay with it being annoyingly long but I’m open to better names :)

At least, that is my current working plan… does anyone see any issues with it? I haven’t actually tried to implement this yet…

#6 Updated by Sean Millichamp almost 4 years ago

  • Status changed from Investigating to In Topic Branch Pending Review
  • Branch set to https://github.com/seanmil/puppet/tree/ticket/2.6.x/3999

I just submitted a proposed patch and test for this to the puppet-dev list and pushed the branch to my github.

#7 Updated by Sean Millichamp almost 4 years ago

I haven’t heard from either the original reporter or the Puppet crew on this patch. I sent it to the mailing list twice a while back with no feedback.

Should I be taking any next steps on this? I’d hate to see this just collect dust.

#8 Updated by Cristian Ciupitu almost 4 years ago

The selinux_use_default_context attribute sounds good enough for me, but please take note that I’m a newbie.

I’ll try to add the patch to Todd Zullinger’s RPM and test it.

#9 Updated by James Turnbull almost 4 years ago

Sean – I just escalated both tickets to our product manager. Will see what I can do! Sorry about the delay – we’ve been totally buried.

#10 Updated by Sean Millichamp almost 4 years ago

Thanks James! I completely understand being buried… I just wanted to make sure that they didn’t get lost/forgotten (or that you were waiting on me for something else).

#11 Updated by Cristian Ciupitu almost 4 years ago

Sean, I’ve applied your patch to puppet-2.6.5-0.5.rc5.fc14.noarch and selinux_ignore_defaults => true seems to work. Thank you for your patch and sorry for not giving some feedback earlier.

#12 Updated by Sean Millichamp almost 4 years ago

  • Affected Puppet version changed from 0.25.5 to 2.6.4

Cristian, Great news that the patch worked for you and seems to address your issue. Thank you for testing it.

#13 Updated by Jesse Wolfe almost 4 years ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release

Available in 2.6.next as of commit:96e9f8f4feab5d768fff304fdb129405596ba128 Thanks for the patch, Sean, and thanks for the testing, Cristian.

#14 Updated by James Turnbull over 3 years ago

  • Status changed from Merged - Pending Release to Closed
  • Target version set to 2.6.x

Pushed in 2.6.7

#15 Updated by James Turnbull over 3 years ago

  • Target version changed from 2.6.x to 2.6.7

Also available in: Atom PDF