Syncthing Policy Module #26
Conversation
| @@ -0,0 +1,8 @@ | |||
| # binary | |||
There was a problem hiding this comment.
These comments aren't needed. Its too obvious.
|
General note/query. I suspect that you have developed this policy using the RedHat reference policy fork. I might be wrong but if you did, then be aware that reference policy differs quite a bit from RedHat's fork, and that it differs in fundamental way's. |
naftulikay
force-pushed
the
feature/syncthing
branch
from
0777676
to
34247e9
Compare
|
@doverride I have fixed most of the issues you've mentioned above and amended my commit. Can you recommend an alternate for Also, I'm combing through the style guide to make sure that everything is in order. |
|
Still getting these denieds, probably due to invalid file transitions: As the following user: With the following home directory: I think it's being caused by Syncthing running in SystemD in user mode ie |
|
On 09/25/2016 07:04 AM, Naftuli Tzvi Kay wrote:
Something like this might do it userdom_user_home_dir_filetrans_user_home_content(syncthing_t, dir) Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 |
|
On 09/25/2016 07:04 AM, Naftuli Tzvi Kay wrote:
This is wrong: +# newly created dirs in home root will transition to user_home_t Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 |
|
Yes, if I run this outside of SystemD as unconfined_u, I get no more denieds and everything seems to work just fine. |
|
Okay, so everything works and I don't have any more denieds coming from syncthing. The only remaining problem is that when I run Syncthing with SystemD in user mode with the following unit: The Syncthing process is running as This causes the files that Syncthing creates to have the following contexts: It's interesting that SystemD in user mode is running as |
|
On 09/25/2016 07:04 PM, Naftuli Tzvi Kay wrote:
reference policy currently does not support systemd --user, so for now
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 |
| gen_require(` | ||
| type NetworkManager_var_run_t; | ||
| ') | ||
| search_dirs_pattern(syncthing_t, NetworkManager_var_run_t, NetworkManager_var_run_t) |
There was a problem hiding this comment.
@doverride Would you like me to submit this change upstream to the NetworkManager interface? IIRC I shouldn't be doing gen_require et al here.
|
On 09/25/2016 07:11 PM, Naftuli Tzvi Kay wrote:
networkmanager_read_pid_files(synchting_t) makes so remove: +# TODO no rule allows searching this dir type
also make networkmanager_read_pid_files(syncthing_t) optional: optional_policy(` Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 |
| @@ -0,0 +1,3 @@ | |||
| /usr/(local/)?bin/syncthing -- gen_context(system_u:object_r:syncthing_exec_t,s0) | |||
There was a problem hiding this comment.
/usr/bin/syncthing -- gen_context(system_u:object_r:syncthing_exec_t,s0)
| # Policy | ||
| domtrans_pattern($2, syncthing_exec_t, syncthing_t) | ||
|
|
||
| syncthing_relabel_home_config_files($2) |
There was a problem hiding this comment.
use this instead:
allow $2 syncthing_config_home_t:file { manage_file_perms relabel_file_perms };
allow $2 syncthing_config_home_t:dir { manage_dir_perms relabel_dir_perms };
allow $2 syncthing_config_home_t:lnk_file { manage_lnk_file_perms relabel_lnk_file_perms };
| # should only ever be execed non-root, but can be started by systemd --user as a service | ||
| init_daemon_domain(syncthing_t, syncthing_exec_t) | ||
| userdom_user_application_domain(syncthing_t, syncthing_exec_t) | ||
| userdom_use_user_ptys(syncthing_t) |
There was a problem hiding this comment.
use userdom_use_user_terminals(syncthing_t) instead or preferably userdom_use_inherited_user_terminals(syncthing_t) if that is available
There was a problem hiding this comment.
userdom_use_inherited_user_terminals doesn't exist in refpolicy.
I'm not sure how crucial this permission is to the runtime of Syncthing. Due to the warning present in userdom_use_user_terminals, I am hesitant about this permission:
## However, this also allows the applications to spy
## on user sessions or inject information into the
## user session. Thus, this access should likely
## not be allowed for non-interactive domains.
Is this access okay for a daemon-like program running in the background?
|
|
||
| userdom_read_user_home_content_files(syncthing_t) | ||
| userdom_read_user_home_content_symlinks(syncthing_t) | ||
|
|
There was a problem hiding this comment.
line 27 to 32 are redundant (already taken care of on like 23 to 25)
| # self rules | ||
| allow syncthing_t self:process getsched; | ||
| allow syncthing_t self:fifo_file { read write }; | ||
|
|
There was a problem hiding this comment.
use allow syncthing_t self:fifo_file rw_fifo_file_perms; instead
| allow syncthing_t self:udp_socket create_socket_perms; | ||
|
|
||
| allow syncthing_t syncthing_exec_t:file execute_no_trans; # presumably allowed to restart itself | ||
|
|
There was a problem hiding this comment.
use: can_exec(syncthing_t, syncthing_exec_t) instead
| kernel_read_system_state(syncthing_t) | ||
| kernel_read_net_sysctls(syncthing_t) | ||
|
|
||
| sysnet_dns_name_resolve(syncthing_t) |
| auth_use_nsswitch(syncthing_t) # /etc/nsswitch.conf | ||
|
|
||
| networkmanager_read_pid_files(syncthing_t) | ||
|
|
There was a problem hiding this comment.
should be optional:
optional_policy(`
# temporary hack until we make this part of sysnet_dns_name_resolve
networkmanager_read_pid_files(syncthing_t)
')
| gen_require(` | ||
| type NetworkManager_var_run_t; | ||
| ') | ||
| search_dirs_pattern(syncthing_t, NetworkManager_var_run_t, NetworkManager_var_run_t) |
There was a problem hiding this comment.
line 83 to 87 can be removed (is already allowed with the networkmanager_read_pid_files() call
There was a problem hiding this comment.
If I remove these lines, I see:
type=AVC msg=audit(1474865450.349:1770): avc: denied { search } for pid=12157 comm="syncthing" name="NetworkManager" dev="tmpfs" ino=16441 scontext=system_u:system_r:syncthing_t tcontext=system_u:object_r:NetworkManager_var_run_t tclass=dir permissive=1
The definition of this interface is:
interface(`networkmanager_read_pid_files',`
gen_require(`
type NetworkManager_var_run_t;
')
files_search_pids($1)
allow $1 NetworkManager_var_run_t:file read_file_perms;
')
| search_dirs_pattern(syncthing_t, NetworkManager_var_run_t, NetworkManager_var_run_t) | ||
|
|
||
| # file transitions | ||
| userdom_user_home_dir_filetrans(syncthing_t, syncthing_config_home_t, dir, "syncthing") |
There was a problem hiding this comment.
@doverride can you link me to the styles wiki on this topic?
|
On 09/26/2016 07:00 AM, Naftuli Tzvi Kay wrote:
That should have been: files_search_pids($1) So that interface should be adjusted Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 |
|
On 09/26/2016 08:16 AM, Naftuli Tzvi Kay wrote:
try userdom_use_user_term() or userdom_use_inherited_user_term() no need to hesitarw this makes it so that if you run syncthing manually
Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 |
|
On 09/26/2016 08:19 AM, Dominick Grift wrote:
Whoops i misread. userdom_use_user_terminals() should be okay to use. Even though it is a Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 |
|
On 09/26/2016 08:47 PM, Naftuli Tzvi Kay wrote:
https://github.com/TresysTechnology/refpolicy/wiki/StyleGuide Key fingerprint = 5F4D 3CDB D3F8 3652 FBD8 02D5 3B6C 5F1D 2C7B 6B02 |
|
@doverride I have updated according to the style guide. Please let me know if there are any other changes. I'm still getting a denied due to the NetworkManager bug in the policy, but I can submit a PR for that. |
|
|
|
If you make it look like the above then i think it should be pretty good. You should indeed also send a PR for adjusted networkmanager_read_var_run_files() Once you made these changes then please ask @pebenito to look at it and if possible commit it |
|
There is at least one thing missing though. Syncthing is currently not allowed to read its own synchthing_config_home_t files. |
|
oh now i see... this is wrong The above would only apply to ~/syncthing, but that is not accurate. Instead it should apply to ~/.config/syncthing So you instead want something like: userdom_user_home_content_filetrans(syncthing_t, syncthing_config_home_t, dir, "syncthing") |
Bug found in pull TresysTechnology#26 - permissions aren't granted for searching the NetworkManager_var_run_t directory, only to reading its files.
naftulikay
force-pushed
the
feature/syncthing
branch
from
20b06ec
to
2c8a149
Compare
|
@doverride I have submitted my PR for the NetworkManager fix at #27.
@doverride above, did you mean I should use: or am I misunderstanding? For now I have updated my PR verbatim with what you posted. |
naftulikay
force-pushed
the
feature/syncthing
branch
from
2c8a149
to
fd67fe1
Compare
Policy governing Syncthing - a file synchronization utility written in Go.
naftulikay
force-pushed
the
feature/syncthing
branch
from
fd67fe1
to
2898174
Compare
|
@pebenito This is almost ready for merge. I need to test the full workflow again, but it's just about there. If there's anything that sticks out, please let me know and I'll explain or fix. |
|
I don't see anything more than @doverride has already noted. |
|
@doverride @pebenito I have tested the full workflow of this PR, and everything checks out. Please merge refpolicy#37 if everything looks good, and then merge this PR as well. Please let me know if any other changes are necessary. |
Policy governing Syncthing - a file synchronization utility written in Go.
Dependent upon TresysTechnology/refpolicy#37.