Skip to content

git client domain fixes #971

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.

Sign up for GitHub

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

git client domain fixes #971

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c22e406
gpg_read_files interface
aerusso Jul 11, 2025
7803301
git: label git-core more reliably
aerusso Mar 16, 2025
fc9eff3
git: more git_xdg_config_t filetrans
aerusso Mar 16, 2025
139cab8
git: broaden execution permissions
aerusso Mar 16, 2025
e958739
git: allow temporary file creation
aerusso Jul 11, 2025
f2f6504
git_client_exec_user_bin tunable
aerusso Jul 11, 2025
e4565ac
git_client_use_gpg tunable
aerusso Jul 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions policy/modules/apps/gpg.if
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,6 +516,25 @@ interface(`gpg_list_user_secrets',`
userdom_search_user_home_dirs($1)
')

########################################
## <summary>
## Allow gpg to read a file type.
## </summary>
## <param name="domain">
## <summary>
## Domain gpg is allowed access to.
## </summary>
## </param>
#
interface(`gpg_read_files',`
Copy link
Member

Choose a reason for hiding this comment

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

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.

gen_require(`
type gpg_t;
')

allow gpg_t $1:file read_file_perms;
allow gpg_t $1:dir search_dir_perms;
')

########################################
## <summary>
## Do not audit attempt to search gpg user secrets dirs.
Expand Down
4 changes: 3 additions & 1 deletion policy/modules/services/git.fc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ HOME_DIR/\.git-credentials -- gen_context(system_u:object_r:git_xdg_config_t,s0)
/usr/bin/git-[^/]+ -- gen_context(system_u:object_r:git_exec_t,s0)
/usr/bin/git2_cli -- gen_context(system_u:object_r:git_exec_t,s0)

/usr/lib/git-core/git-daemon -- gen_context(system_u:object_r:gitd_exec_t,s0)
/usr/lib/git-core/git -- gen_context(system_u:object_r:git_exec_t,s0)
/usr/lib/git-core/git-[^/]+ -- gen_context(system_u:object_r:git_exec_t,s0)
/usr/lib/git-core/git-daemon -- gen_context(system_u:object_r:gitd_exec_t,s0)

/usr/libexec/git-core/git -- gen_context(system_u:object_r:git_exec_t,s0)
/usr/libexec/git-core/git-[^/]+ -- gen_context(system_u:object_r:git_exec_t,s0)
Expand Down
28 changes: 28 additions & 0 deletions policy/modules/services/git.if
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ template(`git_client_role_template',`
gen_require(`
attribute git_client_domain;
type git_exec_t, git_home_t, git_home_hook_t;
type git_xdg_config_t;
')

########################################
Expand All @@ -117,16 +118,43 @@ template(`git_client_role_template',`
allow $2 git_home_hook_t:dir { manage_dir_perms relabel_dir_perms };
allow $2 git_home_hook_t:file { exec_file_perms manage_file_perms relabel_file_perms };
filetrans_pattern($2, git_home_t, git_home_hook_t, dir, "hooks")
xdg_config_filetrans($2, git_xdg_config_t, dir, "git")
userdom_user_home_dir_filetrans($2, git_xdg_config_t, file, ".gitconfig")
userdom_user_home_dir_filetrans($2, git_xdg_config_t, file, ".git-credentials")

allow $3 $1_git_t:process { ptrace signal_perms };
ps_process_pattern($3, $1_git_t)

auth_use_nsswitch($1_git_t)

type $1_git_tmp_t;
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 };
Copy link
Member

Choose a reason for hiding this comment

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

How is the exec used? This is an obvious path for arbitrary code execution.

allow $2 $1_git_tmp_t:lnk_file { manage_lnk_file_perms relabel_lnk_file_perms };
allow $1_git_t $1_git_tmp_t:dir manage_dir_perms;
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)
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@aerusso aerusso Jul 19, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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.

# execute shell scripts
corecmd_exec_shell($1_git_t)
# execute user utilities, e.g., editor
corecmd_bin_domtrans($1_git_t, $2)

optional_policy(`
tunable_policy(`git_client_use_gpg', `
gpg_domtrans($1_git_t)
dev_read_urand($1_git_t)

gpg_read_files($1_git_tmp_t)
')
')

# transition to ssh client domain when performing ssh operations
optional_policy(`
Expand Down
20 changes: 20 additions & 0 deletions policy/modules/services/git.te
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,22 @@ gen_tunable(git_system_use_nfs, false)
## </desc>
gen_tunable(git_client_manage_all_user_home_content, false)

## <desc>
## <p>
## Determine whether Git client domains
## can run user binaries.
## </p>
## </desc>
gen_tunable(git_client_exec_user_bin, false)

## <desc>
## <p>
## Determine whether Git client domains
## can access gpg.
## </p>
## </desc>
gen_tunable(git_client_use_gpg, false)

attribute git_daemon;
attribute_role git_session_roles;

Expand Down Expand Up @@ -347,3 +363,7 @@ tunable_policy(`git_client_manage_all_user_home_content',`
userdom_manage_all_user_home_content(git_client_domain)
userdom_map_all_user_home_content_files(git_client_domain)
')

tunable_policy(`git_client_exec_user_bin',`
userdom_exec_user_bin_files(git_client_domain)
')
Loading