git client domain fixes #971
Conversation
aerusso
force-pushed
the
mrs/git-gpg
branch
2 times, most recently
from
696e000 to
dfdaa40
Compare
|
@0xC0ncord IIRC you made the client domain, I'd appreciate your thoughts. |
There was a problem hiding this comment.
I don't fully understand the various user_git_t domains, so I'm not sure if it makes sense to have user-specific temporary directories.
The primary reason I wrote it this way was because git executes a user's editor of choice for things like interactive rebasing, but it looks like I forgot to add that back-transition to the user domain in the policy when I wrote it. I addressed that in a below comment.
| allow $1_git_t $1_git_tmp_t:file mmap_manage_file_perms; | ||
| allow $1_git_t $1_git_tmp_t:lnk_file manage_lnk_file_perms; | ||
| files_tmp_filetrans($1_git_t, $1_git_tmp_t, {dir file}) | ||
|
|
||
| # allow userdomains to exec git hooks | ||
| exec_files_pattern($3, git_home_hook_t, git_home_hook_t) | ||
| # transition back to the user domain when executing git hooks | ||
| domtrans_pattern($1_git_t, git_home_t, $2) |
There was a problem hiding this comment.
Could you please add corecmd_bin_domtrans($1_git_t, $2) and see if that alleviates the need for a separate $2_git_tmp_t type?
I'm not opposed to a separate tmp type for the git client, but it introduces some additional complexity that would be nice to avoid if we can.
There was a problem hiding this comment.
Unfortunately, the temporary file is created by the git binary itself. For example, when you use git tag -v, verify_gpg_signed_buffer in gpg-interface.c makes a temp = mks_tempfile_t(".git_vtag_tmpXXXXXX"); call that creates a temporary file.
There may be other reasons to do transitions to other domains, but this cannot help the need to handle temporary file creation by the git binary itself.
There was a problem hiding this comment.
This should be fine then, but we still need corecmd_bin_domtrans($1_git_t, $2) to make sure the user's editor runs in the correct context.
There was a problem hiding this comment.
Also done! I was originally trying to provide some protection from a rogue (e.g.,) staff_git_t process, and this would trivially allow staff_t elevation.
I'm still trying to understand what exactly we're trying to stop/allow with each policy.
Allow gpg to read files of a specified type Signed-off-by: Antonio Enrico Russo <[email protected]>
git-core programs are located at /usr/lib/git rather than /usr/libexec/git on some distribution. Label those as well. Signed-off-by: Antonio Enrico Russo <[email protected]>
Appropriately transition the file types for git_xdg_config_t when a user creates such files. Signed-off-by: Antonio Enrico Russo <[email protected]>
Some git-core programs are shellscript. This allows them to run. Also, allow comands to run in the user domain e.g., the commit log editor. Signed-off-by: Antonio Enrico Russo <[email protected]>
git sometimes creates temporary files. This creates new per-user file domains of the form $user_git_tmp_t and automatically transitions to them. Signed-off-by: Antonio Enrico Russo <[email protected]>
git may under some circumstances want to run user binaries (e.g., git-bisect and custom git commands). Add a tunable to allow git to execute such user binaries. Signed-off-by: Antonio Enrico Russo <[email protected]>
git calls gpg when signing and validating commits, and needs to communicate with it through temporary files. Add a tunable to allow this domain transition, and for gpg to be able to read these temporary files. Signed-off-by: Antonio Enrico Russo <[email protected]>
| ## </summary> | ||
| ## </param> | ||
| # | ||
| interface(`gpg_read_files',` |
There was a problem hiding this comment.
This would be better with a name that isn't confused with an interface that gives access to gpg files. Something like gpg_encrypted_content(). It's a bit imprecise in this PR's case, since it is simply signing git commits, but it has clearer intent. The docs can be updated that it's for encryption and signing.
| userdom_user_tmp_file($1_git_tmp_t) | ||
|
|
||
| allow $2 $1_git_tmp_t:dir { manage_dir_perms relabel_dir_perms }; | ||
| allow $2 $1_git_tmp_t:file { exec_file_perms manage_file_perms relabel_file_perms }; |
There was a problem hiding this comment.
How is the exec used? This is an obvious path for arbitrary code execution.
Using the git client domain comes with some pain points. This series adds some tunables and defaults that make it much more functional.
I don't fully understand the various user_git_t domains, so I'm not sure if it makes sense to have user-specific temporary directories. (Though, given the ability to run user-controlled scripts, I suspect it does make sense to restrict inter-user temporary file reads).
I also had to add an interface to gpg, and I'm not sure it's idiomatic, so I suspect it may need to be adjusted to fit in with the overall project style.