Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default file SELinux seltype is incorrect #9431

Closed
davejbax opened this issue Jul 26, 2024 · 4 comments · Fixed by #9448
Closed

Default file SELinux seltype is incorrect #9431

davejbax opened this issue Jul 26, 2024 · 4 comments · Fixed by #9448
Labels
bug Something isn't working triaged Jira issue has been created for this

Comments

@davejbax
Copy link
Contributor

Describe the Bug

The change from matchpathcon to selabel_lookup implemented in #9349 and released in Puppet 8.8.1 today has caused a regression in the SELinux contexts assigned to files by default.

For example, whereas previously the file /etc/httpd/conf.d/foo.conf would get the SELinux type httpd_config_t, it now gets the file type etc_t.

I think this is because the mode parameter of selabel_lookup is set to 0, and does not use the same lstat logic that can be seen in the old code path using matchpathcon. Setting the mode to 0 in this function call means that file context rules are not correctly matched by libselinux.

Going back to the example, we have the following fcontext rules:

/etc/httpd(/.*)?                                   all files          system_u:object_r:httpd_config_t:s0
/etc/httpd/.*                                      symbolic link      system_u:object_r:etc_t:s0

If mode was set to a value including S_IFREG, libselinux would know that the file was a regular file (not a symbolic link) and that the latter rule would not apply. However, when mode is 0, I think all rules are applied ignoring the file type; the latter rule then would take precedence, as it has a greater number of characters before its regular expression (precedence rules). Thus, we get etc_t instead of httpd_config_t.

The fix for this I think is fairly straightforward: we'd just need to determine a suitable mode as done here, and pass that to selabel_lookup. Having tested this directly with the selabel_lookup function, I think this works.

Expected Behavior

Default SELinux types on file resources should match those set by restorecon

Steps to Reproduce

Steps to reproduce the behavior:

  1. Install Apache on a Rocky Linux 9 system with SELinux enabled in enforcing/permissive mode
  2. Manage a file /etc/httpd/conf.d/foo.conf with Puppet: file { '/etc/httpd/conf.d/foo.conf': content => 'test' }
  3. Examine the SELinux type set by Puppet: ls -laZ /etc/httpd/conf.d/foo.conf
  4. Restore the SELinux context with restorecon: restorecon -vF /etc/httpd/conf.d/foo.conf and observe that the SELinux type changes

Environment

  • Puppet 8.8.1
  • Rocky Linux 9.4 x86_64
@davejbax davejbax added the bug Something isn't working label Jul 26, 2024
@davejbax davejbax changed the title Default SELinux seltype is incorrect Default file SELinux seltype is incorrect Jul 26, 2024
davejbax pushed a commit to davejbax/puppet that referenced this issue Jul 29, 2024
SELinux file contexts can be limited to files with a particular mode,
such as symbolic links only or directories only. Therefore, if we
specify no mode (a value of zero), our SELinux label lookup can return
an inaccurate result for the file, causing Puppet to set the wrong
SELinux type for a file. selabel_file(5) notes this:

> mode may be zero, however full matching may not occur.

This commit changes the behaviour of
get_selinux_default_context_with_handle to attempt to lstat(2) the file,
or otherwise rely on the `ensure` property to infer a suitable mode.

This should fix puppetlabs#9431.
davejbax added a commit to davejbax/puppet that referenced this issue Jul 29, 2024
SELinux file contexts can be limited to files with a particular mode,
such as symbolic links only or directories only. Therefore, if we
specify no mode (a value of zero), our SELinux label lookup can return
an inaccurate result for the file, causing Puppet to set the wrong
SELinux type for a file. selabel_file(5) notes this:

> mode may be zero, however full matching may not occur.

This commit changes the behaviour of
get_selinux_default_context_with_handle to attempt to lstat(2) the file,
or otherwise rely on the `ensure` property to infer a suitable mode.

This should fix puppetlabs#9431.
@joshcooper
Copy link
Contributor

Hi @davejbax thanks for your detailed report and pull request!

In 8.8.1, does puppet restore the SELinux type correctly the next time it runs? Or is it incorrect until you run restorecon?

Also does puppet manage the SELinux type correctly if you specify it explicitly in both cases where 1) puppet creates the file and 2) puppet modifies the seltype for an existing file?

file { '/etc/httpd/conf.d/foo.conf':
  content => 'test',
  seltype => 'etc_t',
}

@davejbax
Copy link
Contributor Author

In 8.8.1, does puppet restore the SELinux type correctly the next time it runs? Or is it incorrect until you run restorecon?

No: it appears to be incorrect until running restorecon. As a minimal example:

package { 'httpd': }
-> file { '/etc/httpd/conf.d/foo.conf':
  ensure => present,
}

The first puppet apply of this manifest results in:

$ ls -laZ /etc/httpd/conf.d
total 28
drwxr-xr-x. 3 root root system_u:object_r:httpd_config_t:s0 4096 Jul 31 10:25 .
drwxr-xr-x. 5 root root system_u:object_r:httpd_config_t:s0 4096 Jul 31 10:25 ..
-rw-r--r--. 1 root root system_u:object_r:httpd_config_t:s0 2916 Jul 23 15:56 autoindex.conf
-rw-r--r--. 1 root root system_u:object_r:etc_t:s0             0 Jul 31 10:25 foo.conf
-rw-r--r--. 1 root root system_u:object_r:httpd_config_t:s0  400 Jul 23 15:56 README
drwxr-xr-x. 2 root root system_u:object_r:httpd_config_t:s0 4096 Jul 30 11:12 ssl
-rw-r--r--. 1 root root system_u:object_r:httpd_config_t:s0 1252 Jul 23 15:53 userdir.conf
-rw-r--r--. 1 root root system_u:object_r:httpd_config_t:s0  653 Jul 23 15:53 welcome.conf

And subsequent applies have no change:

$ sudo -i puppet apply /tmp/test.pp
Notice: Compiled catalog for <hostname> in environment production in 0.31 seconds
Notice: Applied catalog in 0.12 seconds

restorecon then indicates that it would change the file type:

$ restorecon -n -v /etc/httpd/conf.d/foo.conf
Would relabel /etc/httpd/conf.d/foo.conf from system_u:object_r:etc_t:s0 to system_u:object_r:httpd_config_t:s0

Also does puppet manage the SELinux type correctly if you specify it explicitly in both cases where 1) puppet creates the file and 2) puppet modifies the seltype for an existing file?

If we specify seltype => 'httpd_config_t' explicitly, yes: Puppet correctly manages the SELinux type in both of these cases. If it is not specified explicitly, Puppet incorrectly manages the SELinux type in both of these cases.

@joshcooper joshcooper added the triaged Jira issue has been created for this label Jul 31, 2024
Copy link

Migrated issue to PUP-12066

@joshcooper
Copy link
Contributor

jenkins please test this on redhat9-64a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triaged Jira issue has been created for this
Projects
None yet
2 participants