Re: git submodule: update=!command
A little late to this thread On Wed, Mar 18, 2015 at 8:50 AM, Jeff King p...@peff.net wrote: On Tue, Mar 17, 2015 at 03:28:57PM -0400, Ryan Lortie wrote: The first is a question about git's basic policy with respect to things like this. I hope that it's safe to assume that running 'git' commands on repositories downloaded from potentially-hostile places will never result in the authors of those repositories being able to run code on my machine. Definitely, our policy is that downloading a git repository should not result in arbitrary code being run. If there is a case of that, it would be a serious security bug. I am not an expert on submodules, but I think the security module there is: 1. You can do whatever you like in submodule.*.update entries in .git/config, including arbitrary code. Nobody but the user can write to it. Which was always the intention of the !command feature. It's for users who want to use additional git porcelains that need some help dealing with submodule updates (e.g stgit). 2. The submodule code may migrate entries from .gitmodules into .git/config, but does so with an allow-known-good whitelist (see git-submodule.sh lines 622-637). So AFAICT there's no bug here, and the system is working as designed. It might be worth mentioning that restriction in the submodule documentation, if only to prevent non-malicious people from wondering why adding !foo does not work in .gitmodules. At the time the !command feature and copying of update config from .gitmodules slid past each other on the list. But out of that I think we got a much better handling that provides security and version compatibility. If that is true then, the second request would be to spell this out more explicitly in the relevant documentation. I'm happy to write a patch to do that, if it is deemed appropriate. Yeah, spelling out the security model more explicitly would be good. There is also some subtlety around hooks. Doing: git clone user@host:/path/to/repo.git local should never run code controlled by repo.git as user@host. But doing: ssh user@host 'cd /path/to/repo.git git log' will respect the .git/config in repo.git, which may include arbitrary commands. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule: update=!command
On Wed, Mar 18, 2015 at 10:05 AM, Junio C Hamano gits...@pobox.com wrote: Ryan Lortie de...@desrt.ca writes: On Tue, Mar 17, 2015, at 16:49, Junio C Hamano wrote: With more recent versions of Git, namely, the versions after 30a52c1d (Merge branch 'ms/submodule-update-config-doc' into maint, 2015-03-13), the documentation pages already have updated descriptions around this area. sigh. That's what I get for forgetting to type 'git pull' before writing a patch. Sorry for the noise! Nothing to apologise or sigh about. You re-confirmed that the old documentation was lacking, which led to an earlier discussion which in turn led to Michal to update the documentation. If you check the output from git diff 30a52c1d^ 30a52c1d and find it appropriately address the problem you originally had, that would be wonderful, and if you can suggest further improvement, that is equally good. I think 30a52c1d could be improved with the following snippet from Ryans patch. For security reasons, this feature is not supported from the `.gitmodules` file Or something along those lines. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule: update=!command
On Wed, Mar 18, 2015 at 8:43 PM, Chris Packham judge.pack...@gmail.com wrote: On Wed, Mar 18, 2015 at 10:05 AM, Junio C Hamano gits...@pobox.com wrote: Ryan Lortie de...@desrt.ca writes: On Tue, Mar 17, 2015, at 16:49, Junio C Hamano wrote: With more recent versions of Git, namely, the versions after 30a52c1d (Merge branch 'ms/submodule-update-config-doc' into maint, 2015-03-13), the documentation pages already have updated descriptions around this area. sigh. That's what I get for forgetting to type 'git pull' before writing a patch. Sorry for the noise! Nothing to apologise or sigh about. You re-confirmed that the old documentation was lacking, which led to an earlier discussion which in turn led to Michal to update the documentation. If you check the output from git diff 30a52c1d^ 30a52c1d and find it appropriately address the problem you originally had, that would be wonderful, and if you can suggest further improvement, that is equally good. I think 30a52c1d could be improved with the following snippet from Ryans patch. For security reasons, this feature is not supported from the `.gitmodules` file Or something along those lines. Which is actually down the bottom if I take the time to read the whole diff. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule: update=!command
Ryan Lortie de...@desrt.ca writes: On Tue, Mar 17, 2015, at 16:49, Junio C Hamano wrote: With more recent versions of Git, namely, the versions after 30a52c1d (Merge branch 'ms/submodule-update-config-doc' into maint, 2015-03-13), the documentation pages already have updated descriptions around this area. sigh. That's what I get for forgetting to type 'git pull' before writing a patch. Sorry for the noise! Nothing to apologise or sigh about. You re-confirmed that the old documentation was lacking, which led to an earlier discussion which in turn led to Michal to update the documentation. If you check the output from git diff 30a52c1d^ 30a52c1d and find it appropriately address the problem you originally had, that would be wonderful, and if you can suggest further improvement, that is equally good. Thanks for participating in our effort to collectively make Git better. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule: update=!command
kara, On Tue, Mar 17, 2015, at 17:05, Junio C Hamano wrote: If you check the output from git diff 30a52c1d^ 30a52c1d and find it appropriately address the problem you originally had, that would be wonderful, and if you can suggest further improvement, that is equally good. Indeed, the new version of the docs looks much better. I'm particularly happy about the change to the format to make it easier to visually scan for the possible update modes. Cheers -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule: update=!command
On Tue, Mar 17, 2015, at 16:49, Junio C Hamano wrote: With more recent versions of Git, namely, the versions after 30a52c1d (Merge branch 'ms/submodule-update-config-doc' into maint, 2015-03-13), the documentation pages already have updated descriptions around this area. sigh. That's what I get for forgetting to type 'git pull' before writing a patch. Sorry for the noise! Cheers -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule: update=!command
On Tue, Mar 17, 2015 at 03:28:57PM -0400, Ryan Lortie wrote: The first is a question about git's basic policy with respect to things like this. I hope that it's safe to assume that running 'git' commands on repositories downloaded from potentially-hostile places will never result in the authors of those repositories being able to run code on my machine. Definitely, our policy is that downloading a git repository should not result in arbitrary code being run. If there is a case of that, it would be a serious security bug. I am not an expert on submodules, but I think the security module there is: 1. You can do whatever you like in submodule.*.update entries in .git/config, including arbitrary code. Nobody but the user can write to it. 2. The submodule code may migrate entries from .gitmodules into .git/config, but does so with an allow-known-good whitelist (see git-submodule.sh lines 622-637). So AFAICT there's no bug here, and the system is working as designed. It might be worth mentioning that restriction in the submodule documentation, if only to prevent non-malicious people from wondering why adding !foo does not work in .gitmodules. If that is true then, the second request would be to spell this out more explicitly in the relevant documentation. I'm happy to write a patch to do that, if it is deemed appropriate. Yeah, spelling out the security model more explicitly would be good. There is also some subtlety around hooks. Doing: git clone user@host:/path/to/repo.git local should never run code controlled by repo.git as user@host. But doing: ssh user@host 'cd /path/to/repo.git git log' will respect the .git/config in repo.git, which may include arbitrary commands. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule: update=!command
Ryan Lortie de...@desrt.ca writes: 'man git-submodule' contains mention (in one place) that: Setting the key submodule.$name.update to !command will cause command to be run. This is not documented in 'man gitmodules' (which documents the other possible values for the 'update' key) Yes, that is deliberate, because you cannot use !command in .gitmodules that is tracked for security reasons. With more recent versions of Git, namely, the versions after 30a52c1d (Merge branch 'ms/submodule-update-config-doc' into maint, 2015-03-13), the documentation pages already have updated descriptions around this area. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
git submodule: update=!command
karaj, 'man git-submodule' contains mention (in one place) that: Setting the key submodule.$name.update to !command will cause command to be run. This is not documented in 'man gitmodules' (which documents the other possible values for the 'update' key) nor in 'man git-config' which also mentions the 'update' key (but refers readers to the two other pages). This feature is scary. The idea that arbitrary code could be executed on my machine when I run innocent-looking git commands, based on the content of the .gitmodules file is enough to give pause to anybody. Fortunately, it seems that (for now?) this is not really the case. 'git submodule init' will copy the values of the 'update' key from .gitmodules to your local git config, but only if they are one of none, checkout, merge or rebase. So, I guess I'm asking two things. The first is a question about git's basic policy with respect to things like this. I hope that it's safe to assume that running 'git' commands on repositories downloaded from potentially-hostile places will never result in the authors of those repositories being able to run code on my machine. If that is true then, the second request would be to spell this out more explicitly in the relevant documentation. I'm happy to write a patch to do that, if it is deemed appropriate. Thanks in advance. Cheers -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git submodule: update=!command
On Tue, Mar 17, 2015, at 15:50, Jeff King wrote: Yeah, spelling out the security model more explicitly would be good. Please see the attached patch. Cheers 0001-docs-clarify-command-submodule-update-policy.patch Description: Binary data
