tee_supplicant: Add necessary SELinux policy for tee_supplicant domain#1105
tee_supplicant: Add necessary SELinux policy for tee_supplicant domain#1105wenjz-qualcomm wants to merge 1 commit into
Conversation
c7d9e2a to
fc2e74a
Compare
|
Hi @wenjz-qualcomm , Can you please update the Commit message to explicitly state that we are adding these new interface for qtee-supplicant which requires more permissions than OPTEE's tee-supplicant. Also, please describe what all permissions (high level) we are adding and why. Please also fix the indentation on your commit message body. Finally, drop this "Upstream-Status: Inappropriate [embedded specific]" from the commit message. |
|
Hi @harshaldev27 and @dburgener, I already modified all mentioned comments, pls help me review the latest version. Thanks. |
e1b57ab to
61d20af
Compare
e8ffeaf to
31ceb11
Compare
No, |
Understood. I believe @wenjz-qualcomm has already accomodated this direction in the lastest pushed changes. |
| # Access tee_supplicant to read /var | ||
| files_list_var(tee_supplicant_t) | ||
|
|
||
| # Access qtee_supplicant to visit /var/lib | ||
| files_list_var_lib(tee_supplicant_t) | ||
|
|
||
| # Access qtee_supplicant to access compatible of device tree | ||
| dev_read_sysfs(tee_supplicant_t) | ||
|
|
||
| # Access qtee_supplicant to send logs to systemd journal | ||
| logging_send_syslog_msg(tee_supplicant_t) | ||
|
|
||
| # Access qtee_supplicant to access /proc/cmdline | ||
| kernel_read_system_state(tee_supplicant_t) | ||
|
|
||
| # Access qtee_supplicant to request sys_rawio capability | ||
| allow tee_supplicant_t self:capability sys_rawio; | ||
|
|
||
| # Access qtee_supplicant to write wake_lock | ||
| dev_write_sysfs(tee_supplicant_t) | ||
|
|
||
| # Allow qtee_supplicant to block system suspend (wake_lock) | ||
| allow tee_supplicant_t self:capability2 block_suspend; |
There was a problem hiding this comment.
Please order the lines in the tunable block according to the StyleGuide.
| files_list_var_lib(tee_supplicant_t) | ||
|
|
||
| # Access qtee_supplicant to access compatible of device tree | ||
| dev_read_sysfs(tee_supplicant_t) |
There was a problem hiding this comment.
Which files/directories in sysfs does this read? We are slowly adding more labeling in sysfs. Please put comments that summarize what is accessed in sysfs.
|
Hi @pebenito , could you please help me to do the further review? |
harshaldev27
left a comment
There was a problem hiding this comment.
This is looking good from my side.
| ') | ||
|
|
||
| optional_policy(` | ||
| tee_supplicant_var_lib_filetrans(initrc_t) |
There was a problem hiding this comment.
This is too broad. This will make any file or directory it creates in /var/lib as tee_supplicant_var_lib_t. The interface should be revised like
refpolicy/policy/modules/kernel/files.if
Line 2192 in 436d78b
There was a problem hiding this comment.
I think the interface just used for tee_supplicant. So I hard coding the filetrans_pattern in .if file.
| type tee_supplicant_var_lib_t; | ||
| ') | ||
|
|
||
| filetrans_pattern($1, var_lib_t, tee_supplicant_var_lib_t, dir, "qtee_supplicant") |
There was a problem hiding this comment.
Sorry I was unclear with my previous comment: You should still use files_var_lib_filetrans. The call should have all the same parameters, except var_lib_t.
There was a problem hiding this comment.
It's ok. I already update this place. Pls review them again.
This change is adding some interfaces for qtee_supplicant which requires more permissions than OPTEE’s tee‑supplicant. Overall, some necessary permissions for qtee_supplicant to accessing system resources have been added. Signed-off-by: Wenjia Zhang <[email protected]>
|
Policy looks good, one last change needed to pass the CI checks. You'll need to add refpolicy/testing/sechecker.ini Line 224 in 1275976 |
Define some new interfaces to access /var/lib/tee, /var/lib/qtee_supplicant
and /var/lib/tee/qtee_supplicant.
Grant the nescessary permission to tee_supplicant for resolving
AVC denials in enforcing mode.
Upstream-Status: Inappropriate [embedded specific]
In the commit write the Upstream-status just because yocto project request it, the status isn't the truly status, just to maintain a uniform format.