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

Fixes #37967 - Add missing rule for running web console. #11

Closed

Conversation

hao-yu
Copy link
Member

@hao-yu hao-yu commented Nov 1, 2024

Please refer to https://projects.theforeman.org/issues/37967 for full details

@ekohl ekohl requested a review from ehelms November 1, 2024 12:11
@@ -2,3 +2,4 @@ allow perm=open exe=/usr/bin/ruby : path=/etc/foreman/encryption_key.rb ftype=te
allow perm=open exe=/usr/bin/ruby : dir=/usr/share/gems ftype=text/x-ruby trust=0
allow perm=open exe=/usr/bin/ruby : dir=/usr/share/foreman ftype=text/x-java trust=0
allow perm=open exe=/usr/bin/ruby : dir=/usr/share/foreman ftype=text/x-ruby trust=0
allow perm=any exe=/usr/libexec/cockpit-ws : dir=/usr/share/gems/gems/ ftype=text/x-ruby trust=0
Copy link
Member

Choose a reason for hiding this comment

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

Two things:

I'd prefer if we were explicit in the perms, from your bug report that would mean execute and open.

From the report this is calling /usr/share/gems/gems/foreman_remote_execution-12.0.7/extra/cockpit/foreman-cockpit-session which seems odd that we call an executable at that path rather than putting it somewhere more standard. @ekohl do you think we should move this file instead?

Copy link
Member

Choose a reason for hiding this comment

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

We actually do ship it in /usr/sbin:
https://github.com/theforeman/foreman-packaging/blob/708efe199b7e34a5cfe988c979196ad650fc8728/packages/plugins/rubygem-foreman_remote_execution/rubygem-foreman_remote_execution.spec#L84

theforeman/foreman-packaging@f4ac262 changed it around, but the Redmine issue mentioned Satellite 6.15 which is Foreman 3.9. That commit only made it into 3.10 so I think this is fixed upstream, but needs a cherry pick.

Copy link
Member

Choose a reason for hiding this comment

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

Excellent, then I believe that is the correct fix and an additional rule should not be needed.

Copy link
Member Author

@hao-yu hao-yu Nov 1, 2024

Choose a reason for hiding this comment

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

IIRC, the default rules only allow shell script execution in /usr/sbin so extra rules might still be needed regardless. Need to test to confirm

Copy link
Member Author

Choose a reason for hiding this comment

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

I did a quick test by manually remove the symlink and move the foreman-cockpit-session to /usr/sbin and then run:

curl -k  https://satellite.example.com/webcon/cockpit+=example.client.com/login
< HTTP/2 500

fapolicyd debug log:

rule=21 dec=deny_audit perm=execute auid=-1 pid=82317 exe=/usr/libexec/cockpit-ws : path=/usr/sbin/foreman-cockpit-session ftype=text/x-ruby trust=0

Copy link
Member

Choose a reason for hiding this comment

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

This is not necessarily a fair test because there are rules about trusting files from RPMs that this is likely to fall under. I think this test needs to be performed on the latest Foreman or the next Satellite release.

Copy link
Member Author

Choose a reason for hiding this comment

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

So I need to add this rules to make it works.

allow perm=open exe=/usr/bin/ruby : path=/usr/sbin/foreman-cockpit-session ftype=text/x-ruby trust=0
allow perm=open exe=/usr/libexec/cockpit-ws : path=/usr/sbin/foreman-cockpit-session ftype=text/x-ruby trust=0
allow perm=execute exe=/usr/libexec/cockpit-ws : path=/usr/sbin/foreman-cockpit-session ftype=text/x-ruby trust=0

Copy link
Member Author

@hao-yu hao-yu Nov 1, 2024

Choose a reason for hiding this comment

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

You are right. no rules needed then. It appears to work in Satellite 6.16

Copy link
Member

Choose a reason for hiding this comment

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

You are right. no rules needed then. It appears to work in Satellite 6.16

As in, you were able to test this? Should I close this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup. I think we can close this. Thanks.

@hao-yu hao-yu closed this Nov 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants