GNOME Bugzilla – Bug 599066
Create a specific check for the gnomeweb user from l10n.gnome.org
Last modified: 2013-12-02 10:05:28 UTC
Our l10n infrastructure on l10n.gnome.org would like to be able to auto-commit files from the Web application Damned-Lies. For security concerns, it has been decided by the sysadmin team that this would require a specific git hook to check that only acceptable files are committed by this account. Basically, the hook should be like the snippet below. I've no idea if the committer name or email is forgeable though. #!/bin/bash check_gnomeweb() { path=$1 case "$path" in *.po|LINGUAS) ;; *) echo "Unauthorized file for gnomeweb user: $path" exit 1 esac } committer_name="$(git log $commit -1 --pretty=format:%cn)" if [ $committer_name = "Gnomeweb" ] ; then git diff-tree --name-only -r $oldrev $newrev | ( while read path ; do check_gnomeweb $path done ) fi
This of course is only valid for UI translations, not documentation where image files and Makefile.am have to be touched. But this could be done in a second step.
Git Hook created. Can you test please? Regards, Alexandro Silva (In reply to comment #0) > Our l10n infrastructure on l10n.gnome.org would like to be able to auto-commit > files from the Web application Damned-Lies. > For security concerns, it has been decided by the sysadmin team that this would > require a specific git hook to check that only acceptable files are committed > by this account. > > Basically, the hook should be like the snippet below. I've no idea if the > committer name or email is forgeable though. > > #!/bin/bash > > check_gnomeweb() { > path=$1 > > case "$path" in > *.po|LINGUAS) > ;; > *) > echo "Unauthorized file for gnomeweb user: $path" > exit 1 > esac > } > > committer_name="$(git log $commit -1 --pretty=format:%cn)" > if [ $committer_name = "Gnomeweb" ] ; then > git diff-tree --name-only -r $oldrev $newrev | ( > while read path ; do > check_gnomeweb $path > done > ) > fi
Sorry for not catching this earlier, but the above snippet is wrong. We aren't interested in the git commit email. We are interested in the Unix user pushing the commit to the server. Often, these are the same, but we don't and can't enforce that they are the same. Something like the above would the damned-lies to commit anything, just by pushing commits with a different Git commit name.
It was not supposed to be correct :-) It was just a first idea, hoping that it would help going forward.
Alexandro, Can you take another look at the code per Owen's comments? Please ping Claude if you have questions on the functionality needed. Thanks.
So to resume and maybe ger my hands dirty with it... What should do this git hook? - check that only po files are changed/added - check the user (as per Owen's comment) - anything else?
Created attachment 173457 [details] [review] create-auth patch This is pretty ugly, but there doesn't seem like a super clean way to do this. The create-auth script needs a bit of refactoring to not make me cry.
Ok so here is the plan of attack... 1.) Setup a password-less ssh key for gnomeweb@l10n.gnome.org. Make the private key readable by the gnomeweb _user_ only and not the group. l10n.gnome.org has fairly limited user access as is so the attack vector is lower than many other servers. 2.) Have create-auth[1] throw down a special ssh key[2] for the gnomeweb user including the host="boron.canonical.com,91.189.93.2" line when given the --gnomeweb-hack argument. This restricts ssh connections from that ssh key to only originate from l10n.gnome.org aka progress.gnome.org aka boron.canonical.com. The patch to do this is attached. Owen or someone else on the sysadmin team please review it to let me know if this is the right idea. create-auth is going to get a lot of love later on. Splinter is truncating the full length of the patch in my browser so look at it raw. 3.) On l10n.gnome.org, configure the git global user (and the d-l process that commits) to be "Damned-Lies Autocommit", and the global git client email to a mailinglist that emails all of the translators (if that list exists). This is for reply to go to the main l10n email list if someone wants to reply to an auto-checkin. 4.) Write a simple bourne shell git hook that runs these checks: a.) [ "$(/usr/bin/whoami)" = "gnomeweb" ] b.) [ "$(/usr/bin/id -u)" = "2184" ] c.) [ "$committer_name" = "Damned-Lies Autocommit" ] d.) [ "$committer_email" = "the email for the main l10n list" ] e.) If it works[2], logic similar to Claude's pseudocode would be perfect. I double-checked that whoami runs geteuid(2) (yay strace) so b isn't 100% necessary. The goal is max paranoia and gracefully die if anything is off. c and d are easy for anyone to circumvent with "git commit --author", but they are just an extra layer of sanity checking. e is to make sure that only translation files are being committed. 5.) Teach d-l how to commit translations to a local git repository and rebase ontop of changes (hello git.py). The sysadmin team will write a cronjob to periodically push commits to git.gnome.org as user gnomeweb. I'll address points 1-3 now and put this off to someone else until at very least after the Boston Summit. [1] http://git.gnome.org/sysadmin-bin/tree/create-auth [2] Example key when I tested this patch on label: command="/home/admin/bin/run-git-or-special-cmd",no-pty,no-port-forwarding,host="boron.canonical.com,91.189.93.2" ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAoP1vEyT0IiDzmedoe+NKpgJ0pe47pOiaX31/XAntQ5+WWJn2PJDZIGyxBmgSjO8z4pdk7TMV9Bf2ryJRwEnEJDNkAoz1HJM8WUCt0l2SYwS4Qrem2AYHqPJTESrSLkwtEkK4WZrrk00Mp8/dUUBAL3uM5lTKjQuRXZ2PFZFBg79KTP4mrakZ0eTuvvs/jA13Fa8g9q5Ho3A7pe8kpTWCYeqzVbsTMHd1u7s3hiZ5JZhiCHeEOrXN/APtMpSH16wnBjogershs4BzRyAGu2SGcJOs+5jII26tFC3RcFrqTYsaaaplDlZp1j0fKGdQBe+v+SmR6OWFPzlxnhmeQFpqow== gnomeweb@progress.gnome.org_l10n_autocommit_git_only_key
Created attachment 173458 [details] [review] Version 2 Forgot a "global GNOMEWEB" statement before. Not strictly necessary and works without, but it fits with the rest of the existing coding style.
(In reply to comment #8) > Splinter is truncating the full length of the > patch in my browser so look at it raw. I don't see this - the Splinter review view seems to show everything for me.
@owen: Must be a bleeding edge chromium issue. Any problems with me merging the patch to create-auth and adding an ssh key to gnomeweb? I tested it with ldapvi and it worked fine.
(In reply to comment #11) > @owen: Must be a bleeding edge chromium issue. Any problems with me merging the > patch to create-auth and adding an ssh key to gnomeweb? I tested it with ldapvi > and it worked fine. I didn't review the patch. (That's not the same as having objections, but some care with create-auth is important.) I can try to get to it later today, though I'm not necessarily the authoritative person for create-auth these days.
@owen: so who is the authoritative person now? :) I mean, let's try not to block it, it has been already a year, and now that Damned-Lies has to be faced with Transifex to do a review study on who is better for GNOME it would not be fair to have a half baked Damned-Lies. Thanks for your work!
@Gil: We are working on it. Please be patient.
Review of attachment 173458 [details] [review]: Main problem I have with this is that I don't think we should be recyclign the gnomeweb user for this. Otherwise comments are pretty superficial. ::: create-auth @@ +29,3 @@ GNOMECVS=0 GNOMEGIT=0 +GNOMEWEB=0 GNOMEWEB isn't at all like the other variables or options here, this would have to be GNOMEWEB_USER or something. But I don't like using the gnomeweb user for this. gnomeweb is about editing web config, viewing web logs, etc. Doesn't seem like it should be in the same permissions bucket as "committing translations changes to Git" .... I don't see any real connection other than that the process is running as gnomeweb (is it?) on l10n.gnome.org. I would expect a separate user 'translations' or something. @@ +130,3 @@ + if not GNOMEWEB and "gnomeweb" in users: + users.remove('gnomeweb') Probably harmless, but makes no "code sense" to me @@ +184,3 @@ #file.write ("command=\"/usr/bin/cvs server\",no-pty,no-port-forwarding ") + if GNOMEWEB and user['uid'] == "gnomeweb": + file.write ("command=\"/home/admin/bin/run-git-or-special-cmd\",no-pty,no-port-forwarding,host=\"boron.canonical.com,91.189.93.2\" ") Belt and suspenders doesn't make sense to me - either host or IP, why both? @@ +257,3 @@ + if '--gnomeweb-hack' in group_list: + GNOMEWEB=1 + group_list = filter (lambda x: x != '--gnomeweb-hack', group_list) What's a hack about it?
(In reply to comment #15) > Review of attachment 173458 [details] [review]: (...) > But I don't like using the gnomeweb user for this. gnomeweb is about editing > web config, viewing web logs, etc. Doesn't seem like it should be in the same > permissions bucket as "committing translations changes to Git" .... I don't see > any real connection other than that the process is running as gnomeweb (is it?) > on l10n.gnome.org. I would expect a separate user 'translations' or something. The question is whether we will authorize DL itself to push updates (gnomeweb user then makes sense), or if this will be done in a separate script (as Jeff suggested before). I'm not completely opposed to the latter, but it adds some more complexity (one more layer to maintain) and would add some delay between coordinator submission and arrival in real git branch. Note that especially on release time, this can be important. And I'm not sure it adds more security, does it?
(In reply to comment #8) > Ok so here is the plan of attack... > > 1.) Setup a password-less ssh key for gnomeweb@l10n.gnome.org. Make the private > key readable by the gnomeweb _user_ only and not the group. l10n.gnome.org has > fairly limited user access as is so the attack vector is lower than many other > servers. Is damned lies really running as gnomeweb? Thought we pretty much gave up on running web services under a non-apache user. Basically if you have commit access to l10n.gnome.org you can make the account do whatever you want, so I don't think locking down the key too hard has a point. Readable-as-web-service-user seems about as good as we can easily do. > 2.) Have create-auth[1] throw down a special ssh key[2] for the gnomeweb user > including the host="boron.canonical.com,91.189.93.2" line when given the > --gnomeweb-hack argument. This restricts ssh connections from that ssh key to > only originate from l10n.gnome.org aka progress.gnome.org aka > boron.canonical.com. > > The patch to do this is attached. Owen or someone else on the sysadmin team > please review it to let me know if this is the right idea. create-auth is going > to get a lot of love later on. Splinter is truncating the full length of the > patch in my browser so look at it raw. See separate review. > 3.) On l10n.gnome.org, configure the git global user (and the d-l process that > commits) to be "Damned-Lies Autocommit", and the global git client email to a > mailinglist that emails all of the translators (if that list exists). This is > for reply to go to the main l10n email list if someone wants to reply to an > auto-checkin. I think noreply@gnome.org would be better. (Check that that's what we use for bugzilla mail, etc.) Mail aliases imply accountability for responses. Also, I would make the name something meaningful - Damned-Lies is an implementation detail. > 4.) Write a simple bourne shell git hook that runs these checks: > a.) [ "$(/usr/bin/whoami)" = "gnomeweb" ] > b.) [ "$(/usr/bin/id -u)" = "2184" ] > c.) [ "$committer_name" = "Damned-Lies Autocommit" ] > d.) [ "$committer_email" = "the email for the main l10n list" ] > e.) If it works[2], logic similar to Claude's pseudocode would be perfect. > > I double-checked that whoami runs geteuid(2) (yay strace) so b isn't 100% > necessary. The goal is max paranoia and gracefully die if anything is off. c > and d are easy for anyone to circumvent with "git commit --author", but they > are just an extra layer of sanity checking. e is to make sure that only > translation files are being committed. Hmmm, not really a fan of belt-and-suspenders-and-duct-tape. Adding easily circumventable checks just adds potential breakage without security. I'd do a single check that we are confident in. /usr/bin/whoami = <translation user> seems good enough to me. If you can subvert that, you can subvert the operation of the shell script and the system to the point where that single check working doesn't matter. > 5.) Teach d-l how to commit translations to a local git repository and rebase > ontop of changes (hello git.py). The sysadmin team will write a cronjob to > periodically push commits to git.gnome.org as user gnomeweb. I'll address > points 1-3 now and put this off to someone else until at very least after the > Boston Summit. This part is all up to the damned lies maintainers - though that they already had code. Transifex certainly does.
(In reply to comment #17) > (In reply to comment #8) > > Ok so here is the plan of attack... > > > > 1.) Setup a password-less ssh key for gnomeweb@l10n.gnome.org. Make the private > > key readable by the gnomeweb _user_ only and not the group. l10n.gnome.org has > > fairly limited user access as is so the attack vector is lower than many other > > servers. > > Is damned lies really running as gnomeweb? Thought we pretty much gave up on > running web services under a non-apache user. We run django apps as user gnomeweb. mod_wsgi is very flexible and gives advantages to running things this way. The tomboy online webapp, snowy, runs as gnomeweb as well. > Basically if you have commit access to l10n.gnome.org you can make the account > do whatever you want, so I don't think locking down the key too hard has a > point. Readable-as-web-service-user seems about as good as we can easily do. Ok so the question is do we wand d-l to run the equivalent of git push directly to git.gnome.org? It does open things up a bit more, but in the worst possible case, someone reverts the commits and it is only language files. You seem to be of the opinion that it is ok to give users enough rope to hang themselves. I'm still working out the details of how things in gnome-land work :) > > 2.) Have create-auth[1] throw down a special ssh key[2] for the gnomeweb user > > including the host="boron.canonical.com,91.189.93.2" line when given the > > --gnomeweb-hack argument. This restricts ssh connections from that ssh key to > > only originate from l10n.gnome.org aka progress.gnome.org aka > > boron.canonical.com. > > > > The patch to do this is attached. Owen or someone else on the sysadmin team > > please review it to let me know if this is the right idea. create-auth is going > > to get a lot of love later on. Splinter is truncating the full length of the > > patch in my browser so look at it raw. > > See separate review. I agree with almost all of it, but can't respond indepth or do anything about it until later tomorrow or this weekend. That whole "real life" thing. > > 3.) On l10n.gnome.org, configure the git global user (and the d-l process that > > commits) to be "Damned-Lies Autocommit", and the global git client email to a > > mailinglist that emails all of the translators (if that list exists). This is > > for reply to go to the main l10n email list if someone wants to reply to an > > auto-checkin. > > I think noreply@gnome.org would be better. (Check that that's what we use for > bugzilla mail, etc.) Mail aliases imply accountability for responses. Also, I > would make the name something meaningful - Damned-Lies is an implementation > detail. That is all arbitrary and subject to change. > > 4.) Write a simple bourne shell git hook that runs these checks: > > a.) [ "$(/usr/bin/whoami)" = "gnomeweb" ] > > b.) [ "$(/usr/bin/id -u)" = "2184" ] > > c.) [ "$committer_name" = "Damned-Lies Autocommit" ] > > d.) [ "$committer_email" = "the email for the main l10n list" ] > > e.) If it works[2], logic similar to Claude's pseudocode would be perfect. > > > > I double-checked that whoami runs geteuid(2) (yay strace) so b isn't 100% > > necessary. The goal is max paranoia and gracefully die if anything is off. c > > and d are easy for anyone to circumvent with "git commit --author", but they > > are just an extra layer of sanity checking. e is to make sure that only > > translation files are being committed. > > Hmmm, not really a fan of belt-and-suspenders-and-duct-tape. Adding easily > circumventable checks just adds potential breakage without security. I'd do a > single check that we are confident in. /usr/bin/whoami = <translation user> > seems good enough to me. If you can subvert that, you can subvert the operation > of the shell script and the system to the point where that single check working > doesn't matter. History and smart people like Dan Walsh and Bruce Schneier have taught me security works best in layers. However, I'm still fairly new to how we handle things so this list will be shortened to a and e. > > 5.) Teach d-l how to commit translations to a local git repository and rebase > > ontop of changes (hello git.py). The sysadmin team will write a cronjob to > > periodically push commits to git.gnome.org as user gnomeweb. I'll address > > points 1-3 now and put this off to someone else until at very least after the > > Boston Summit. > > This part is all up to the damned lies maintainers - though that they already > had code. Transifex certainly does. Claude seems to be of the opinion d-l should do that. If you're ok with it, I'm ok with it.
(In reply to comment #18) > > Basically if you have commit access to l10n.gnome.org you can make the account > > do whatever you want, so I don't think locking down the key too hard has a > > point. Readable-as-web-service-user seems about as good as we can easily do. > > Ok so the question is do we wand d-l to run the equivalent of git push directly > to git.gnome.org? It does open things up a bit more, but in the worst possible > case, someone reverts the commits and it is only language files. You seem to be > of the opinion that it is ok to give users enough rope to hang themselves. I'm > still working out the details of how things in gnome-land work :) Yes, you get some extra security if: A) You run the git push as a different user so the gnomeweb user can't read the ssh key B) You run the git push with a fixed set of options, so exploiting holes in our setup with tricky git push options is harder But on the other hand automated commits to git seem hard enough to do without an extra layer. You can't just do something like cron/fishpoll something like: ( cd $gnomeweb/checkouts/some_directory && git push ) Because hooks in some_directory will run and you might as well have run the git push as the original user. So you'd need to figure out how to disable git hooks when running the push command. (And also worry about what affect git push options in .git/config have on the push.) If possible at all, then the solution might not be too bad, but I don't think it's that important and worth blocking getting something working on. The essential layers of security here are: - The security of the web app - The security of restricting commits to be to .po files in the po/ directory (shouldn't be able to commit to po/Makefile.*)
Created attachment 173543 [details] [review] Version 3 addressing all of owen's comments
I've created a translations user with the shell set to /sbin/nologin and tested the script: [root@label ~]# /tmp/create-auth --translation-user gnomeweb webusers bugzilla Added 1 user: translations [root@label ~]# cd /etc/sshd/users/translations/ [root@label translations]# cat authorized_keys command="/home/admin/bin/run-git-or-special-cmd",no-pty,no-port-forwarding,host="91.189.93.2" ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAoP1vEyT0IiDzmedoe+NKpgJ0pe47pOiaX31/XAntQ5+WWJn2PJDZIGyxBmgSjO8z4pdk7TMV9Bf2ryJRwEnEJDNkAoz1HJM8WUCt0l2SYwS4Qrem2AYHqPJTESrSLkwtEkK4WZrrk00Mp8/dUUBAL3uM5lTKjQuRXZ2PFZFBg79KTP4mrakZ0eTuvvs/jA13Fa8g9q5Ho3A7pe8kpTWCYeqzVbsTMHd1u7s3hiZ5JZhiCHeEOrXN/APtMpSH16wnBjogershs4BzRyAGu2SGcJOs+5jII26tFC3RcFrqTYsaaaplDlZp1j0fKGdQBe+v+SmR6OWFPzlxnhmeQFpqow== gnomeweb@progress.gnome.org_l10n_autocommit_git_only_key [root@label translations]# /tmp/create-auth gnomeweb webusers bugzilla Removed 1 user: translations Once this is in I'll update puppet and then write the git commit hook.
How do you handle the server side? Loads of Git repositories run under the gnomecvs user. Most have the common GNOME git hook, so you can do a pre-push check. However, not all have the common git hook, allowing this translation user to commit to them.
@Olav: The server side check will be handled via the common pre-receive check. Out of curiosity, I did a look to see what repositories don't have the common pre-receive hook: [root@git ~]# cd /git [root@git git]# ll */hooks/pre-receive | egrep -v '\/home\/admin\/gitadmin-bin\/gnome-pre-receive$' [root@git git]# EXCLUDES="^(second-import|backup|repositories\.(txt|doap)|cgit\.reposit.*|archive|empty-description)$" [root@git git]# ls -1 | sed -e 's/\.git$//g' | egrep -v "$EXCLUDES" | sort > /tmp/allrepos [root@git git]# ls -1 */hooks/pre-receive | sed -e 's:\.git/:/:g' | awk -F/ '{print $1}' | egrep -v "$EXCLUDES" | sort > /tmp/pre-rcvx [root@git git]# diff -u /tmp/allrepos /tmp/pre-rcvx --- /tmp/allrepos 2010-10-30 20:24:35.000000000 +0000 +++ /tmp/pre-rcvx 2010-10-30 20:24:38.000000000 +0000 @@ -195,7 +195,6 @@ giomm gio-strigi gir-repository -gitadmin-bin gitg giulia giv @@ -690,7 +689,6 @@ svn-migration svn-web swfdec-gnome -sysadmin-bin sysprof system-tools-backends-clone tasks The translations user shouldn't need to ever push to {git,sys}admin-bin so that seems like a perfect place to put it. My thought was to work on adding a function to pre-receive-check-po[1] which is called by gnome-pre-receive[2]. Time to work on that patch. [1] http://git.gnome.org/browse/gitadmin-bin/tree/pre-receive-check-po [2] http://git.gnome.org/browse/gitadmin-bin/tree/gnome-pre-receive#n74
Jeff: You only checked the repositories in /git? I thought we had repositories in other locations as well (not only git VM), but forgot details.
I looked at /etc/cgitrc and /git/cgit.repositories. In those files, there are no repositories under anywhere but /git. If there are, they won't need the translation user to commit files to them. Also, they need to be fixed and put under /git. If you can find any more by all means lets fix them and put them under /git. At very least, a symlink would do.
The point I would make (without investigating the matter in detail) - is that ideally the system would be robust if a git repository is somewhere we aren't thinking about or loses the hook. To deny commits to the translations user on such a repository.
The best way I can think to do that is to not put the translations user in the gnomecvs group. Instead, we can use acls to set write permissions to all current repositories under /git and set it as the default for any newly created directories under /git sans {git,sys}admin-bin. That way access will fail if there is some git repo not under /git. I am unaware of any others.
@Owen: It seems impossible given a user with write access to a repository to somehow deny them write access if the hook that checks their access is removed. Also, the only people capable of doing that are sysadmin or gitadmin team members. It seems like a reasonable enough tradeoff.
(In reply to comment #28) > @Owen: It seems impossible given a user with write access to a repository to > somehow deny them write access if the hook that checks their access is removed. > > Also, the only people capable of doing that are sysadmin or gitadmin team > members. It seems like a reasonable enough tradeoff. What I was thinking about, rather than a complicated acl scheme was doing a check in run-git-or-special-command ... the syntax of what can be executed by git is pretty limited, and has the directory easily extractable. (See man git-shell) That could actually check that the repository was in the expected location and had the expected hook before allowing the commit to proceed. (That script or what it calls needs fixing anyways .... otherwise the translation user could create repositories -- not the end of the world, but probably not intended.) Since we need to maintain security anyways ... no gnomecvs user is supposed to be able to commit to repositories without the standard email and reflogging hooks .. I don't see this check as mandatory. if it's hard, then we shouldn't bother
Hey guys, are there any plans on working on this feature during this (3.1/3.2) development cycle? Needless to say, GNOME translators would (still) really appreciate it. TIA
(In reply to comment #30) > Hey guys, are there any plans on working on this feature during this (3.1/3.2) > development cycle? Needless to say, GNOME translators would (still) really > appreciate it. TIA Definitely a +1. What's the status of the business? - the script is mostly done? - the security concerns are all addressed? - what's missing on damned-lies (the web app)? - what's missing on damned-lies server? - what's missing on GNOME git server? It would be really cool to have this in time for 3.2 so that the release notes itself can be pushed automatically on release-notes git repository from damned-lies.
I looked at the script, but it only gives a SSH key full access to the GNOME git repository. I thought we wanted to limit what could be committed? Anyway, I have very limited knowledge regarding Git. Owen: Thoughts?
ping. Anyone is working on this or at least knows the status? Where we have to send the $BEVERAGE to make it happen? :)
Nobody. I lack the knowledge for sure.
GUADEC is approaching fast, if you want your cup of $BEVERAGE it's the perfect time to finish this :) Also known as: friendly ping to Owen, Jeff, any other sysadmin able to look at it...
Created attachment 250984 [details] [review] Add a pre-receive hook that limits the translations user Before we allow the 'translations' user to commit, check that the change is something expect l10n.gnome.org - a change to a .po file, a LINGUAS file, or the addition of a language to a help Makefile.am.
Comment on attachment 250984 [details] [review] Add a pre-receive hook that limits the translations user We tried to install the hook with Andrea, but found several errors. We blocked on the git.config("key", _quiet=True) line, as git.config is not a method but a module. Was the idea to read the config of the specific git module?
(In reply to comment #37) > (From update of attachment 250984 [details] [review]) > We tried to install the hook with Andrea, but found several errors. We blocked > on the git.config("key", _quiet=True) line, as git.config is not a method but a > module. Was the idea to read the config of the specific git module? The git module that this is using is git.py in sysadmin-bin/git - in that context, I don't understand "git.config is not a method but a module" git.config("key", _quiet=True) Does have an error (didn't test the configuration part) - it should be key, not "key" - it's a parameter to the function. I'm away until the beginning of September but can set this up then.
(In reply to comment #38) > The git module that this is using is git.py in sysadmin-bin/git - in that > context, I don't understand "git.config is not a method but a module" This explain the error, thanks (we used the GitPython module instead).
Here's how the final setup is looking like: 1. the translations user was added into LDAP and an SSH key pair was generated for this user, the key is currently living in /usr/local/www/gnomeweb/.ssh/translations_rsa on progress.gnome.org. The translations user has its own switch on create-auth, and it's currently not part of the gnomevcs group. The gnomeweb user has access to the file in rw, the file is not group accessible. 2. create-auth is restricting access to the translations user making sure the user itself can only reach git.gnome.org from boron.canonical.com, in addition it can't get a pty allocated. More details at https://git.gnome.org/browse/sysadmin-bin/tree/create-auth#n40. Thanks Jeff for your past work on this. 3. Owen's hook has been committed to sysadmin-bin and enabled globally. The hook will make sure that the only committable files are: PO/help files, with the addition of the LINGUAS line on Makefile.am. 4. The only downside of the whole setup is the translations_rsa file being handled by gnomeweb, which is the user that is currently running the damned-lies service. I did ask Claude to properly implement a way to really use the translations user for making the commit. I personally don't see any grave security issue in this, we do have a lot of checks in place already and removing an offending key is a matter of a few seconds in case an attacker will gain access to the gnomeweb user but having a command that gets executed by an user != from gnomeweb itself would be indeed nice, that way even if an attacker will gain access to the gnomeweb user by hacking the damned-lies app, the ssh key won't be accessible at all given it being chowned to the translations:translations user in 0600 mode.
Owen, if you feel the whole setup is fine, we can go ahead and add the relevant switch on damned-lies web UI so that the whole process goes to production.
Claude, we were wondering if it would be actually possible to have the relevant damned-lies code to include a little script we would write to let the 'translations' user execute the push itself through a setuid call? can you think of a better solution? we're just throwing in some ideas :-)
Just call sudo with the right arguments, on need to a setuid script.
I don't see much point in having a separate user to do the push on the client side. The only point would be if the sudo'ed command tried to restrict exactly what was pushed - if you can push anything then there is no security improvement at all. And duplicating complicated checks on client and server seems like too much.
Created attachment 254268 [details] [review] create-auth: Fix up --translation-user option The handling of --translation-user had some quirks - for example, specifying --translation-user would silently force on inclusion of all GIT users. Rearrange option handling for --translation-user so that it works in a more expected way.
Created attachment 254271 [details] [review] run-git-or-special-cmd: check the git directory Check that the directory being accessed is in /git and has the correct hooks.
(In reply to comment #44) > I don't see much point in having a separate user to do the push on the client > side. The only point would be if the sudo'ed command tried to restrict exactly > what was pushed - if you can push anything then there is no security > improvement at all. And duplicating complicated checks on client and server > seems like too much. Thinking about it, there is one significant thing you do get out of a sudo setup which is preventing a directory traversal (or other read-only) vulnerability from exposing the private key for someone to take and try to do stuff on their own system. But hopefully locking the key to one IP *mostly* handles that. So there would be some advantage, but I don't see it as blocking getting something going.
The GNOME Infrastructure Team is currently migrating its bug / issue tracker away from Bugzilla to Request Tracker and therefore all the currently open bugs have been closed and marked as OBSOLETE. The following move will also act as a cleanup for very old and ancient tickets that were still living on Bugzilla. If your issue still hasn't been fixed as of today please report it again on the relevant RT queue. More details about the available queues you can report the bug against can be found at https://wiki.gnome.org/Sysadmin/RequestTracker. Thanks for your patience, the GNOME Infrastructure Team
For reference I sent this bugreport to the new system, it has been assigned an ID of [gnome.org #14045]. That would be https://rt.gnome.org/SelfService/Display.html?id=14045 but it's not public.