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

Refs #30803: Allow Apache to connect to Unix socket #113

Merged
merged 1 commit into from
Oct 13, 2020

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Sep 10, 2020

No description provided.

@lzap
Copy link
Member

lzap commented Sep 21, 2020

Thanks for the patch. While this will probably work, we should follow good practice.

  • Create a new type named foreman_var_run_t in foreman.te
  • Create a rule in foreman.fc for /run/foreman.sock file and give it foreman_var_run_t type
  • Add this patch into the relabel script (if not present yet)
  • Relabel the system
  • Call sepolgen-ifgen to generate reverse macro database (if you haven't)
  • Reload the policy
  • Switch to permissive temporarily
  • Restart both apache and foreman services
  • Now use audit2allow -Ral to generate the required rules, they will be slightly different

@ehelms
Copy link
Member Author

ehelms commented Oct 1, 2020

I updated this with the suggestions. Went through the workflow above and I keep running into:

[root@centos7-foreman-nightly foreman-selinux]# audit2allow -Ral

require {
	type httpd_t;
	type foreman_rails_t;
	class unix_stream_socket connectto;
}

#============= httpd_t ==============

#!!!! The file '/run/foreman.sock' is mislabeled on your system.  
#!!!! Fix with $ restorecon -R -v /run/foreman.sock
#!!!! This avc can be allowed using the boolean 'daemons_enable_cluster_mode'
allow httpd_t foreman_rails_t:unix_stream_socket connectto;

I am not clear what I need to do to resolve that.

@lzap
Copy link
Member

lzap commented Oct 7, 2020

Sorry for the delay, how do I reproduce.

@ehelms
Copy link
Member Author

ehelms commented Oct 7, 2020

You need this PR -- theforeman/puppet-foreman#883
If you use Forklift you can create a box in vagrant/boxes/99-local.yml:

socket:
  box: centos7-foreman-nightly
  ansible:
    variables:
      foreman_installer_module_prs:
        - theforeman/foreman/883

Copy link
Member

@lzap lzap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the sock file was in /run/foreman directory none of these would be necessary as files do get their parent directories labels automatically. In order to have the file in /run we need to add an extra (transition) rule: files_pid_filetrans.

foreman.fc Outdated Show resolved Hide resolved
foreman.te Outdated
# Allow Apache access to the Unix socket
allow foreman_rails_t httpd_var_run_t:dir search;
allow httpd_t foreman_rails_t:unix_stream_socket { connectto getattr read write };

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace this with with:

# Socket and PID files transition
files_pid_filetrans(foreman_rails_t, foreman_var_run_t, { file dir sock_file })

# Allow Apache access to the Unix socket
files_search_pids(httpd_t)
stream_connect_pattern(httpd_t, foreman_var_run_t, foreman_var_run_t, foreman_rails_t)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put the first rule above next to Foreman rules, this is a rule for Foreman app to create the SOCK file in /run with the correct label. And the remaining two rules can be added next to Apache rules.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given #113 (comment) (the last part), does the Foreman app need a rule to create the SOCK file? I'm trying to parse the syntax here in the SELinux world with respect to who needs the rule.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The files_pid_filetrans(foreman_rails_t, foreman_var_run_t, { file dir sock_file }) rule says "when an app running in foreman_rails_t domain creates a pid or a socket in var_run_t give it foreman_var_run_t". I tested this on my install, when I restart "foreman" systemd unit it creates the /var/foreman.sock now with the correct label,

Note that foreman.fc only defines regular expressions for restorecon. You still need those transition rules.

Does that put some light on how this works?

@ehelms
Copy link
Member Author

ehelms commented Oct 10, 2020

Yea, we discussed where to store it. The tricky part is that the socket is owned by Apache and not Foreman which is what allows Apache to send traffic via the reverse proxy and lock down what system users can communicate over the socket (https://github.com/theforeman/puppet-foreman/pull/883/files#diff-d6542e555b7c0207c63c969d650e1036R5). This is the same pattern that gunicorn takes (https://docs.gunicorn.org/en/stable/deploy.html#systemd):

# Our service won't need permissions for the socket, since it
# inherits the file descriptor by socket activation
# only the nginx daemon will need access to the socket
User=www-data

@ekohl
Copy link
Member

ekohl commented Oct 10, 2020

The tricky part is that the socket is owned by Apache and not Foreman

More importantly, Apache needs to be able to read/write to the socket. We now keep /run/foreman as owned by Foreman and Apache can't access that directory. Using /run/foreman/foreman.sock (or similar) would mean Apache needs to be added to the Foreman group which reduces security. Maybe we could place it in /run/httpd (or equivalent on Debian) but I don't know if you can depend on /run/httpd to exist already when foreman.socket starts.

@ehelms
Copy link
Member Author

ehelms commented Oct 12, 2020

@lzap Updated with your recommendations.. not sure if I got them in the right sections of the foreman.te file though.

Copy link
Member

@lzap lzap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Let's give this a spin.

@lzap lzap merged commit 7a8c5ab into theforeman:develop Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants