After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 599066 - Create a specific check for the gnomeweb user from l10n.gnome.org
Create a specific check for the gnomeweb user from l10n.gnome.org
Status: RESOLVED OBSOLETE
Product: sysadmin
Classification: Infrastructure
Component: Git
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME Sysadmins
GNOME Sysadmins
Depends on:
Blocks:
 
 
Reported: 2009-10-20 16:16 UTC by Claude Paroz
Modified: 2013-12-02 10:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
create-auth patch (2.44 KB, patch)
2010-10-29 05:15 UTC, Jeff Schroeder
none Details | Review
Version 2 (2.64 KB, patch)
2010-10-29 05:32 UTC, Jeff Schroeder
needs-work Details | Review
Version 3 addressing all of owen's comments (2.91 KB, patch)
2010-10-30 00:40 UTC, Jeff Schroeder
none Details | Review
Add a pre-receive hook that limits the translations user (5.96 KB, patch)
2013-08-06 15:17 UTC, Owen Taylor
needs-work Details | Review
create-auth: Fix up --translation-user option (4.23 KB, patch)
2013-09-06 16:02 UTC, Owen Taylor
none Details | Review
run-git-or-special-cmd: check the git directory (3.26 KB, patch)
2013-09-06 16:05 UTC, Owen Taylor
none Details | Review

Description Claude Paroz 2009-10-20 16:16:19 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
Comment 1 Claude Paroz 2009-10-20 16:22:17 UTC
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.
Comment 2 Alexandro Silva 2010-07-06 14:20:08 UTC
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
Comment 3 Owen Taylor 2010-07-06 15:03:36 UTC
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.
Comment 4 Claude Paroz 2010-07-06 15:30:32 UTC
It was not supposed to be correct :-) It was just a first idea, hoping that it would help going forward.
Comment 5 Paul Cutler 2010-07-11 19:01:59 UTC
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.
Comment 6 Gil Forcada 2010-10-26 10:53:31 UTC
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?
Comment 7 Jeff Schroeder 2010-10-29 05:15:00 UTC
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.
Comment 8 Jeff Schroeder 2010-10-29 05:18:20 UTC
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
Comment 9 Jeff Schroeder 2010-10-29 05:32:01 UTC
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.
Comment 10 Owen Taylor 2010-10-29 14:34:34 UTC
(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.
Comment 11 Jeff Schroeder 2010-10-29 14:45:07 UTC
@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.
Comment 12 Owen Taylor 2010-10-29 14:48:24 UTC
(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.
Comment 13 Gil Forcada 2010-10-29 15:41:05 UTC
@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!
Comment 14 Jeff Schroeder 2010-10-29 16:03:27 UTC
@Gil: We are working on it. Please be patient.
Comment 15 Owen Taylor 2010-10-29 20:07:18 UTC
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?
Comment 16 Claude Paroz 2010-10-29 20:24:30 UTC
(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?
Comment 17 Owen Taylor 2010-10-29 20:27:32 UTC
(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.
Comment 18 Jeff Schroeder 2010-10-29 21:14:54 UTC
(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.
Comment 19 Owen Taylor 2010-10-29 21:36:16 UTC
(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.*)
Comment 20 Jeff Schroeder 2010-10-30 00:40:38 UTC
Created attachment 173543 [details] [review]
Version 3 addressing all of owen's comments
Comment 21 Jeff Schroeder 2010-10-30 00:42:36 UTC
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.
Comment 22 Olav Vitters 2010-10-30 13:02:25 UTC
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.
Comment 23 Jeff Schroeder 2010-10-30 20:42:56 UTC
@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
Comment 24 Olav Vitters 2010-10-30 22:10:10 UTC
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.
Comment 25 Jeff Schroeder 2010-10-30 23:09:11 UTC
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.
Comment 26 Owen Taylor 2010-10-30 23:31:30 UTC
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.
Comment 27 Jeff Schroeder 2010-11-04 01:41:56 UTC
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.
Comment 28 Jeff Schroeder 2010-11-04 01:52:26 UTC
@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.
Comment 29 Owen Taylor 2010-11-04 14:22:27 UTC
(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
Comment 30 Petr Kovar 2011-06-21 13:26:37 UTC
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
Comment 31 Gil Forcada 2011-08-10 14:02:54 UTC
(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.
Comment 32 Olav Vitters 2011-08-10 14:44:29 UTC
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?
Comment 33 Gil Forcada 2012-01-03 23:18:01 UTC
ping.

Anyone is working on this or at least knows the status? Where we have to send the $BEVERAGE to make it happen? :)
Comment 34 Olav Vitters 2012-01-04 08:34:54 UTC
Nobody. I lack the knowledge for sure.
Comment 35 Gil Forcada 2012-07-08 19:05:58 UTC
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...
Comment 36 Owen Taylor 2013-08-06 15:17:44 UTC
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 37 Claude Paroz 2013-08-23 18:27:28 UTC
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?
Comment 38 Owen Taylor 2013-08-24 06:55:55 UTC
(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.
Comment 39 Claude Paroz 2013-08-24 07:59:40 UTC
(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).
Comment 40 Andrea Veri 2013-08-24 12:44:34 UTC
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.
Comment 41 Andrea Veri 2013-08-24 12:50:56 UTC
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.
Comment 42 Andrea Veri 2013-08-28 21:00:35 UTC
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 :-)
Comment 43 Olav Vitters 2013-08-28 21:41:17 UTC
Just call sudo with the right arguments, on need to a setuid script.
Comment 44 Owen Taylor 2013-09-03 22:05:36 UTC
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.
Comment 45 Owen Taylor 2013-09-06 16:02:59 UTC
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.
Comment 46 Owen Taylor 2013-09-06 16:05:38 UTC
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.
Comment 47 Owen Taylor 2013-09-06 17:37:20 UTC
(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.
Comment 48 Andrea Veri 2013-11-21 14:57:00 UTC
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
Comment 49 Frederic Peters 2013-12-02 10:05:28 UTC
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.