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

Run frr service as frr_t and add new bfd_multi port #547

Open
wants to merge 1 commit into
base: rawhide
Choose a base branch
from

Conversation

Koncpa
Copy link
Contributor

@Koncpa Koncpa commented Jan 22, 2021

Confined frr service as frr_t and create policy files and rules for frr service.
FRRouting (FRR) is an IP routing protocol which
includes protocol daemons for BGP, IS-IS, LDP, OSPF, PIM, and RIP.
Add new port called bfd_multi for bfdd daemon, which is use in frr service.

Copy link
Member

@WOnder93 WOnder93 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 to my untrained eye now, although @zpytela might have further comments.

Koncpa added a commit to Koncpa/selinux-policy that referenced this pull request Jan 29, 2021
Add new port called cmadmin for bfdd daemon, which is use in
frr service.
Related to PR: fedora-selinux#547
@Koncpa
Copy link
Contributor Author

Koncpa commented Jan 29, 2021

Add another permissions for frr policy, which were discover during testing a common selinux policy tests. The build test will fail, because in policy has added interface, which isn't yet in policy, but PR has already made: #556

@WOnder93
Copy link
Member

The build test will fail, because in policy has added interface, which isn't yet in policy, but PR has already made: #556

In that case why not just include the commit in this PR instead of opening a separate one?

Koncpa added a commit to Koncpa/selinux-policy that referenced this pull request Feb 1, 2021
Add new port called cmadmin for bfdd daemon, which is use in
frr service.
Related to PR: fedora-selinux#547
policy/modules.conf Outdated Show resolved Hide resolved
policy/modules/contrib/frr.fc Outdated Show resolved Hide resolved
/var/lock/subsys/ripd gen_context(system_u:object_r:frr_lock_t,s0)
/var/lock/subsys/ripngd gen_context(system_u:object_r:frr_lock_t,s0)
/var/lock/subsys/staticd gen_context(system_u:object_r:frr_lock_t,s0)
/var/lock/subsys/zebra gen_context(system_u:object_r:frr_lock_t,s0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are all of the subsys locks plain files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, It was tested.

Copy link
Contributor

Choose a reason for hiding this comment

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

For plain files the -- spec can be used.

policy/modules/contrib/frr.if Show resolved Hide resolved
policy/modules/contrib/frr.if Show resolved Hide resolved
policy/modules/contrib/frr.te Outdated Show resolved Hide resolved
policy/modules/contrib/frr.te Outdated Show resolved Hide resolved
policy/modules/contrib/frr.te Show resolved Hide resolved
policy/modules/contrib/frr.te Show resolved Hide resolved
files_pid_filetrans(frr_t, frr_var_run_t, { dir file lnk_file })

allow frr_t frr_exec_t:dir search_dir_perms;
read_lnk_files_pattern(frr_t, frr_exec_t, frr_exec_t)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there an example for executable symlink?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, here you have example.

Example : type=PROCTITLE msg=audit(02/02/2021 06:02:14.242:651) : proctitle=/usr/lib/frr/fabricd -d -A 127.0.0.1 type=PATH msg=audit(02/02/2021 06:02:14.242:651) : item=0 name=/usr/lib64/frr/libfrr.so.0 nametype=UNKNOWN cap_fp=none cap_fi=none cap_fe=0 cap_fver=0 cap_frootid=0 type=CWD msg=audit(02/02/2021 06:02:14.242:651) : cwd=/etc/frr type=SYSCALL msg=audit(02/02/2021 06:02:14.242:651) : arch=x86_64 syscall=openat success=no exit=EACCES(Permission denied) a0=0xffffff9c a1=0x7ffcae5eadb0 a2=O_RDONLY|O_CLOEXEC a3=0x0 items=1 ppid=35891 pid=35892 auid=unset uid=root gid=root euid=root suid=root fsuid=root egid=root sgid=root fsgid=root tty=(none) ses=unset comm=fabricd exe=/usr/lib/frr/fabricd subj=system_u:system_r:frr_t:s0 key=(null) type=AVC msg=audit(02/02/2021 06:02:14.242:651) : avc: denied { read } for pid=35892 comm=fabricd name=libfrr.so.0 dev="vda1" ino=35651802 scontext=system_u:system_r:frr_t:s0 tcontext=system_u:object_r:frr_exec_t:s0 tclass=lnk_file permissive=0

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I managed to find the previous discussion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you post the output of this command?

ls -laZ /usr/lib64/frr

Copy link
Member

Choose a reason for hiding this comment

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

This seems like a packaging bug in frr - the executable files currently in /usr/lib/frr should be placed in /usr/libexec/frr instead. Did you report it to the maintainer/upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@zpytela , matchpathcon say:
/usr/lib64/frr/libfrr.so.0 system_u:object_r:frr_exec_t:s0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This seems like a packaging bug in frr - the executable files currently in /usr/lib/frr should be placed in /usr/libexec/frr instead. Did you report it to the maintainer/upstream?

No didn't report that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is that in .fc there is /usr/lib/frr(/.*)? gen_context(system_u:object_r:frr_exec_t,s0), which breaks the labels in /usr/lib64/frr/..., since there is an equivalency between /usr/lib and /usr/lib64.

So issue is that this files should be in /usr/libexec/frr a also have different label?

Copy link
Contributor

Choose a reason for hiding this comment

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

Executable files which are not supposed to be executed from cmdline should be in /usr/libexec. Shared libraries should be in /usr/lib64 and have the common type lib_t unless there was a reason for having a private type - this matches the purpose of shared libraries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@WOnder93 So I have already reported this bug to frr maintainer.

@zpytela
Copy link
Contributor

zpytela commented Feb 2, 2021

@Koncpa, do you want to keep the 2 commit separate, like to distinguish between the initial policy and later enhancements, or will it make sense for you to squash them?

@Koncpa
Copy link
Contributor Author

Koncpa commented Feb 2, 2021

@Koncpa, do you want to keep the 2 commit separate, like to distinguish between the initial policy and later enhancements, or will it make sense for you to squash them?

What do you think that will be better? I made two commits because on previous commit made @WOnder93 review so I want distinguish later enchancements for next review.

@Koncpa
Copy link
Contributor Author

Koncpa commented Feb 4, 2021

Squash two commits into one.

zpytela pushed a commit that referenced this pull request Feb 4, 2021
Add new port called cmadmin for bfdd daemon, which is use in
frr service.
Related to PR: #547
@vmojzis
Copy link
Member

vmojzis commented Feb 10, 2021

@Koncpa Could you please rebase the PR to incoporate abf7fa2 ?

Koncpa added a commit to Koncpa/selinux-policy that referenced this pull request Feb 10, 2021
Add new port called cmadmin for bfdd daemon, which is use in
frr service.
Related to PR: fedora-selinux#547
@Koncpa
Copy link
Contributor Author

Koncpa commented Feb 10, 2021

@Koncpa Could you please rebase the PR to incoporate abf7fa2 ?

Can I rebase PR to incorporate commit in this way?

@Koncpa Koncpa changed the title Run frr service as frr_t Run frr service as frr_t and add new bfd_multi port Mar 18, 2021
@zpytela
Copy link
Contributor

zpytela commented Mar 18, 2021

Now it generally looks good.

Confined frr service as frr_t and create policy files and rules for frr
service.
FRRouting (FRR) is an IP routing protocol which
includes protocol daemons for BGP, IS-IS, LDP, OSPF, PIM, and RIP.
Add new port called bfd_multi for bfdd daemon, which is use in
frr service.
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.

None yet

4 participants