From 4669eab596c8d90de0cf9f9d359ad8dd8f48edb5 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 23 Oct 2008 16:29:50 -0400 Subject: more work on untrusted committers Wired up check_canedit and check_canremove, still need to deal with check_canattach, and test. --- IkiWiki/Receive.pm | 101 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 101 insertions(+) create mode 100644 IkiWiki/Receive.pm (limited to 'IkiWiki/Receive.pm') diff --git a/IkiWiki/Receive.pm b/IkiWiki/Receive.pm new file mode 100644 index 000000000..63944bb81 --- /dev/null +++ b/IkiWiki/Receive.pm @@ -0,0 +1,101 @@ +#!/usr/bin/perl + +package IkiWiki::Receive; + +use warnings; +use strict; +use IkiWiki; + +sub getuser () { #{{{ + my $user=(getpwuid($<))[0]; + if (! defined $user) { + error("cannot determine username for $<"); + } + return $user; +} #}}} + +sub trusted () { #{{{ + my $user=getuser(); + return ! ref $config{untrusted_committers} || + ! grep { $_ eq $user } @{$config{untrusted_committers}}; +} #}}} + +sub test () { #{{{ + exit 0 if trusted(); + IkiWiki::rcs_test_receive(); + + # Dummy up a cgi environment to use when calling check_canedit + # and friends. + eval q{use CGI}; + error($@) if $@; + my $cgi=CGI->new; + require IkiWiki::CGI; + my $session=IkiWiki::cgi_getsession($cgi); + my $user=getuser(); + $session->param("name", $user); + $ENV{REMOTE_ADDR}='unknown' unless exists $ENV{REMOTE_ADDR}; + + lockwiki(); + loadindex(); + + my %newfiles; + + foreach my $change (IkiWiki::rcs_receive()) { + # This untaint is safe because we check file_pruned and + # wiki_file_regexp. + my $file=$change->{file}=~/$config{wiki_file_regexp}/; + $file=possibly_foolish_untaint($file); + if (! defined $file || ! length $file || + IkiWiki::file_pruned($file, $config{srcdir})) { + error(gettext("bad file name")); + } + + my $type=pagetype($file); + my $page=pagename($file) if defined $type; + + if ($change->{action} eq 'add') { + $newfiles{$file}=1; + } + + if ($change->{action} eq 'change' || + $change->{action} eq 'add') { + if (defined $page) { + if (IkiWiki->can("check_canedit") && + IkiWiki::check_canedit($page, $cgi, $session)) { + next; + } + } + else { + # TODO + #if (IkiWiki::Plugin::attachment->can("check_canattach") && + # IkiWiki::Plugin::attachment::check_canattach($session, $file, $path)) { + # next; + #} + } + } + elsif ($change->{action} eq 'remove') { + # check_canremove tests to see if the file is present + # on disk. This will fail is a single commit adds a + # file and then removes it again. Avoid the problem + # by not testing the removal in such pairs of changes. + # (The add is still tested, just to make sure that + # no data is added to the repo that a web edit + # could add.) + next if $newfiles{$file}; + + if (IkiWiki::Plugin::remove->can("check_canremove") && + IkiWiki::Plugin::remove::check_canremove(defined $page ? $page : $file, $cgi, $session)) { + next; + } + } + else { + error "unknown action ".$change->{action}; + } + + error sprintf(gettext("you are not allowed to change %s"), $file); + } + + exit 0; +} #}}} + +1 -- cgit v1.2.3 From ad9e443f22a139c71f0cd05885cda3e418f27567 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 23 Oct 2008 16:56:40 -0400 Subject: check_canattach hooked up --- IkiWiki/Plugin/git.pm | 33 ++++++++++++++++++++++++++------- IkiWiki/Receive.pm | 9 ++++----- doc/plugins/write.mdwn | 5 ++++- 3 files changed, 34 insertions(+), 13 deletions(-) (limited to 'IkiWiki/Receive.pm') diff --git a/IkiWiki/Plugin/git.pm b/IkiWiki/Plugin/git.pm index 234e7af2e..bdac6f7a1 100644 --- a/IkiWiki/Plugin/git.pm +++ b/IkiWiki/Plugin/git.pm @@ -9,6 +9,7 @@ use open qw{:utf8 :std}; my $sha1_pattern = qr/[0-9a-fA-F]{40}/; # pattern to validate Git sha1sums my $dummy_commit_msg = 'dummy commit'; # message to skip in recent changes +my $no_chdir=0; sub import { #{{{ hook(type => "checkconfig", id => "git", call => \&checkconfig); @@ -127,8 +128,10 @@ sub safe_git (&@) { #{{{ if (!$pid) { # In child. # Git commands want to be in wc. - chdir $config{srcdir} - or error("Cannot chdir to $config{srcdir}: $!"); + if (! $no_chdir) { + chdir $config{srcdir} + or error("Cannot chdir to $config{srcdir}: $!"); + } exec @cmdline or error("Cannot exec '@cmdline': $!"); } # In parent. @@ -606,13 +609,20 @@ sub rcs_receive () { #{{{ while (<>) { chomp; my ($oldrev, $newrev, $refname) = split(' ', $_, 3); - + # only allow changes to gitmaster_branch if ($refname !~ /^refs\/heads\/\Q$config{gitmaster_branch}\E$/) { error sprintf(gettext("you are not allowed to change %s"), $refname); } - - foreach my $ci (git_commit_info($oldrev."..".$newrev)) { + + # Avoid chdir when running git here, because the changes + # are in the master git repo, not the srcdir repo. + # The pre-recieve hook already puts us in the right place. + $no_chdir=1; + my @changes=git_commit_info($oldrev."..".$newrev); + $no_chdir=0; + + foreach my $ci (@changes) { foreach my $detail (@{ $ci->{'details'} }) { my $file = $detail->{'file'}; @@ -623,8 +633,7 @@ sub rcs_receive () { #{{{ error sprintf(gettext("you are not allowed to change %s"), $file); } - my $action; - my $mode; + my ($action, $mode, $path); if ($detail->{'status'} =~ /^[M]+\d*$/) { $action="change"; $mode=$detail->{'mode_to'}; @@ -632,6 +641,15 @@ sub rcs_receive () { #{{{ elsif ($detail->{'status'} =~ /^[AM]+\d*$/) { $action="add"; $mode=$detail->{'mode_to'}; + if (! pagetype($file)) { + eval q{use File::Temp}; + die $@ if $@; + my $fh; + ($fh, $path)=tempfile("XXXXXXXXXX", UNLINK => 1); + if (system("git show ".$detail->{sha1_to}." > '$path'") != 0) { + error("failed writing temp file"); + } + } } elsif ($detail->{'status'} =~ /^[DAM]+\d*/) { $action="remove"; @@ -654,6 +672,7 @@ sub rcs_receive () { #{{{ push @rets, { file => $file, action => $action, + path => $path, }; } } diff --git a/IkiWiki/Receive.pm b/IkiWiki/Receive.pm index 63944bb81..4d437cf78 100644 --- a/IkiWiki/Receive.pm +++ b/IkiWiki/Receive.pm @@ -66,11 +66,10 @@ sub test () { #{{{ } } else { - # TODO - #if (IkiWiki::Plugin::attachment->can("check_canattach") && - # IkiWiki::Plugin::attachment::check_canattach($session, $file, $path)) { - # next; - #} + if (IkiWiki::Plugin::attachment->can("check_canattach") && + IkiWiki::Plugin::attachment::check_canattach($session, $file, $change->{path})) { + next; + } } } elsif ($change->{action} eq 'remove') { diff --git a/doc/plugins/write.mdwn b/doc/plugins/write.mdwn index 9f096e4f7..7fbe4bd57 100644 --- a/doc/plugins/write.mdwn +++ b/doc/plugins/write.mdwn @@ -829,13 +829,16 @@ sense to implement for all RCSs. It should examine the incoming changes, and do any sanity checks that are appropriate for the RCS to limit changes to safe file adds, -removes, and renames. If something bad is found, it should exit +removes, and changes. If something bad is found, it should exit nonzero, to abort the push. Otherwise, it should return a list of files that were changed, in the form: { file => # name of file that was changed action => # either "add", "change", or "remove" + path => # temp file containing the new file content, only + # needed for "add", and only if the file is an + # attachment, not a page } The list will then be checked to make sure that each change is one that -- cgit v1.2.3 From f349e4ef36d0653c779872d9eb8dca94e6c15d90 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Thu, 23 Oct 2008 18:05:12 -0400 Subject: fix calls to check_* These throw errors, do not have useful return codes. --- IkiWiki/Receive.pm | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'IkiWiki/Receive.pm') diff --git a/IkiWiki/Receive.pm b/IkiWiki/Receive.pm index 4d437cf78..33f548a14 100644 --- a/IkiWiki/Receive.pm +++ b/IkiWiki/Receive.pm @@ -22,7 +22,6 @@ sub trusted () { #{{{ sub test () { #{{{ exit 0 if trusted(); - IkiWiki::rcs_test_receive(); # Dummy up a cgi environment to use when calling check_canedit # and friends. @@ -31,20 +30,19 @@ sub test () { #{{{ my $cgi=CGI->new; require IkiWiki::CGI; my $session=IkiWiki::cgi_getsession($cgi); - my $user=getuser(); - $session->param("name", $user); + $session->param("name", getuser()); $ENV{REMOTE_ADDR}='unknown' unless exists $ENV{REMOTE_ADDR}; - lockwiki(); - loadindex(); + IkiWiki::lockwiki(); + IkiWiki::loadindex(); my %newfiles; foreach my $change (IkiWiki::rcs_receive()) { # This untaint is safe because we check file_pruned and # wiki_file_regexp. - my $file=$change->{file}=~/$config{wiki_file_regexp}/; - $file=possibly_foolish_untaint($file); + my ($file)=$change->{file}=~/$config{wiki_file_regexp}/; + $file=IkiWiki::possibly_foolish_untaint($file); if (! defined $file || ! length $file || IkiWiki::file_pruned($file, $config{srcdir})) { error(gettext("bad file name")); -- cgit v1.2.3 From fbcb8553df1f6150f2cfb2fd5d81a65e93074ac5 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Fri, 24 Oct 2008 13:29:30 -0400 Subject: really fix calls to check_can* --- IkiWiki/Receive.pm | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'IkiWiki/Receive.pm') diff --git a/IkiWiki/Receive.pm b/IkiWiki/Receive.pm index 33f548a14..c69911a7c 100644 --- a/IkiWiki/Receive.pm +++ b/IkiWiki/Receive.pm @@ -58,15 +58,15 @@ sub test () { #{{{ if ($change->{action} eq 'change' || $change->{action} eq 'add') { if (defined $page) { - if (IkiWiki->can("check_canedit") && - IkiWiki::check_canedit($page, $cgi, $session)) { - next; + if (IkiWiki->can("check_canedit")) { + IkiWiki::check_canedit($page, $cgi, $session); + next; } } else { - if (IkiWiki::Plugin::attachment->can("check_canattach") && - IkiWiki::Plugin::attachment::check_canattach($session, $file, $change->{path})) { - next; + if (IkiWiki::Plugin::attachment->can("check_canattach")) { + IkiWiki::Plugin::attachment::check_canattach($session, $file, $change->{path}); + next; } } } @@ -80,15 +80,15 @@ sub test () { #{{{ # could add.) next if $newfiles{$file}; - if (IkiWiki::Plugin::remove->can("check_canremove") && - IkiWiki::Plugin::remove::check_canremove(defined $page ? $page : $file, $cgi, $session)) { + if (IkiWiki::Plugin::remove->can("check_canremove")) { + IkiWiki::Plugin::remove::check_canremove(defined $page ? $page : $file, $cgi, $session); next; } } else { error "unknown action ".$change->{action}; } - + error sprintf(gettext("you are not allowed to change %s"), $file); } -- cgit v1.2.3 From 1a883b3c504a543c8f4a5b5bb9687e7770f28a4f Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Fri, 24 Oct 2008 13:44:03 -0400 Subject: include temp file for attachment change too --- IkiWiki/Plugin/git.pm | 23 +++++++++++++---------- IkiWiki/Receive.pm | 2 +- doc/plugins/write.mdwn | 4 ++-- 3 files changed, 16 insertions(+), 13 deletions(-) (limited to 'IkiWiki/Receive.pm') diff --git a/IkiWiki/Plugin/git.pm b/IkiWiki/Plugin/git.pm index 3a8476e7d..84df56181 100644 --- a/IkiWiki/Plugin/git.pm +++ b/IkiWiki/Plugin/git.pm @@ -619,7 +619,7 @@ sub rcs_receive () { #{{{ # Avoid chdir when running git here, because the changes # are in the master git repo, not the srcdir repo. - # The pre-receive hook already puts us in the right place. + # The pre-recieve hook already puts us in the right place. $no_chdir=1; my @changes=git_commit_info($oldrev."..".$newrev); $no_chdir=0; @@ -643,15 +643,6 @@ sub rcs_receive () { #{{{ elsif ($detail->{'status'} =~ /^[AM]+\d*$/) { $action="add"; $mode=$detail->{'mode_to'}; - if (! pagetype($file)) { - eval q{use File::Temp}; - die $@ if $@; - my $fh; - ($fh, $path)=File::Temp::tempfile("XXXXXXXXXX", UNLINK => 1); - if (system("git show ".$detail->{sha1_to}." > '$path'") != 0) { - error("failed writing temp file"); - } - } } elsif ($detail->{'status'} =~ /^[DAM]+\d*/) { $action="remove"; @@ -670,6 +661,18 @@ sub rcs_receive () { #{{{ error gettext("you are not allowed to change file modes"); } } + + # extract attachment to temp file + if (($action eq 'add' || $action eq 'change') && + ! pagetype($file)) { + eval q{use File::Temp}; + die $@ if $@; + my $fh; + ($fh, $path)=File::Temp::tempfile("XXXXXXXXXX", UNLINK => 1); + if (system("git show ".$detail->{sha1_to}." > '$path'") != 0) { + error("failed writing temp file"); + } + } push @rets, { file => $file, diff --git a/IkiWiki/Receive.pm b/IkiWiki/Receive.pm index c69911a7c..9a672abc9 100644 --- a/IkiWiki/Receive.pm +++ b/IkiWiki/Receive.pm @@ -45,7 +45,7 @@ sub test () { #{{{ $file=IkiWiki::possibly_foolish_untaint($file); if (! defined $file || ! length $file || IkiWiki::file_pruned($file, $config{srcdir})) { - error(gettext("bad file name")); + error(gettext("bad file name %s"), $file); } my $type=pagetype($file); diff --git a/doc/plugins/write.mdwn b/doc/plugins/write.mdwn index 5ee4acb53..abcabbdc3 100644 --- a/doc/plugins/write.mdwn +++ b/doc/plugins/write.mdwn @@ -837,8 +837,8 @@ files that were changed, in the form: file => # name of file that was changed action => # either "add", "change", or "remove" path => # temp file containing the new file content, only - # needed for "add", and only if the file is an - # attachment, not a page + # needed for "add"/"change", and only if the file + # is an attachment, not a page } The list will then be checked to make sure that each change is one that -- cgit v1.2.3 From 739e2ca0b442531b3f0e1b83c3338d2da65ca77d Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Fri, 24 Oct 2008 15:02:54 -0400 Subject: can't lock wiki due to permissions (probably) luckily, don't really need to here --- IkiWiki/Receive.pm | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'IkiWiki/Receive.pm') diff --git a/IkiWiki/Receive.pm b/IkiWiki/Receive.pm index 9a672abc9..81b67d9b4 100644 --- a/IkiWiki/Receive.pm +++ b/IkiWiki/Receive.pm @@ -33,7 +33,8 @@ sub test () { #{{{ $session->param("name", getuser()); $ENV{REMOTE_ADDR}='unknown' unless exists $ENV{REMOTE_ADDR}; - IkiWiki::lockwiki(); + # Wiki is not locked because we lack permission to do so. + # So, relying on atomic index file updates to avoid trouble. IkiWiki::loadindex(); my %newfiles; @@ -59,8 +60,8 @@ sub test () { #{{{ $change->{action} eq 'add') { if (defined $page) { if (IkiWiki->can("check_canedit")) { - IkiWiki::check_canedit($page, $cgi, $session); - next; + IkiWiki::check_canedit($page, $cgi, $session); + next; } } else { -- cgit v1.2.3 From 146192d5b01329bd8e5dfbf4045efded467151e0 Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Fri, 24 Oct 2008 15:47:42 -0400 Subject: the pre-receive wrapper needs to be suid after all It needs to write to the user db. --- IkiWiki/Plugin/git.pm | 2 +- IkiWiki/Receive.pm | 26 +++++++++++++++++++------- 2 files changed, 20 insertions(+), 8 deletions(-) (limited to 'IkiWiki/Receive.pm') diff --git a/IkiWiki/Plugin/git.pm b/IkiWiki/Plugin/git.pm index 84df56181..5bef92856 100644 --- a/IkiWiki/Plugin/git.pm +++ b/IkiWiki/Plugin/git.pm @@ -46,7 +46,7 @@ sub checkconfig () { #{{{ push @{$config{wrappers}}, { test_receive => 1, wrapper => $config{git_test_receive_wrapper}, - wrappermode => "0755", + wrappermode => (defined $config{git_wrappermode} ? $config{git_wrappermode} : "06755"), }; } } #}}} diff --git a/IkiWiki/Receive.pm b/IkiWiki/Receive.pm index 81b67d9b4..451a3fe8e 100644 --- a/IkiWiki/Receive.pm +++ b/IkiWiki/Receive.pm @@ -7,7 +7,8 @@ use strict; use IkiWiki; sub getuser () { #{{{ - my $user=(getpwuid($<))[0]; + # CALLER_UID is set by the suid wrapper, to the original uid + my $user=(getpwuid(exists $ENV{CALLER_UID} ? $ENV{CALLER_UID} : $<))[0]; if (! defined $user) { error("cannot determine username for $<"); } @@ -23,20 +24,31 @@ sub trusted () { #{{{ sub test () { #{{{ exit 0 if trusted(); + IkiWiki::lockwiki(); + IkiWiki::loadindex(); + # Dummy up a cgi environment to use when calling check_canedit # and friends. eval q{use CGI}; error($@) if $@; my $cgi=CGI->new; + $ENV{REMOTE_ADDR}='unknown' unless exists $ENV{REMOTE_ADDR}; + + # And dummy up a session object. require IkiWiki::CGI; my $session=IkiWiki::cgi_getsession($cgi); $session->param("name", getuser()); - $ENV{REMOTE_ADDR}='unknown' unless exists $ENV{REMOTE_ADDR}; - - # Wiki is not locked because we lack permission to do so. - # So, relying on atomic index file updates to avoid trouble. - IkiWiki::loadindex(); - + # Make sure whatever user was authed is in the + # userinfo db. + require IkiWiki::UserInfo; + if (! IkiWiki::userinfo_get($session->param("name"), "regdate")) { + IkiWiki::userinfo_setall($session->param("name"), { + email => "", + password => "", + regdate => time, + }) || error("failed adding user"); + } + my %newfiles; foreach my $change (IkiWiki::rcs_receive()) { -- cgit v1.2.3 From 7ddea03684df47c861c264216b83e7653d6784fd Mon Sep 17 00:00:00 2001 From: Joey Hess Date: Sun, 26 Oct 2008 14:03:18 -0400 Subject: move untrusted committer test into the wrapper This saves around 1/4th second per trusted commit since ikiwiki doesn't need to start up. --- IkiWiki/Receive.pm | 26 +++++++++++++++++++++++++- IkiWiki/Wrapper.pm | 13 +++++++++---- ikiwiki.in | 5 ----- 3 files changed, 34 insertions(+), 10 deletions(-) (limited to 'IkiWiki/Receive.pm') diff --git a/IkiWiki/Receive.pm b/IkiWiki/Receive.pm index 451a3fe8e..72668d26a 100644 --- a/IkiWiki/Receive.pm +++ b/IkiWiki/Receive.pm @@ -7,7 +7,6 @@ use strict; use IkiWiki; sub getuser () { #{{{ - # CALLER_UID is set by the suid wrapper, to the original uid my $user=(getpwuid(exists $ENV{CALLER_UID} ? $ENV{CALLER_UID} : $<))[0]; if (! defined $user) { error("cannot determine username for $<"); @@ -21,6 +20,31 @@ sub trusted () { #{{{ ! grep { $_ eq $user } @{$config{untrusted_committers}}; } #}}} +sub gen_wrapper () { #{{{ + # Test for commits from untrusted committers in the wrapper, to + # avoid loading ikiwiki at all for trusted commits. + + my $ret=<<"EOF"; + { + int u=getuid(); +EOF + $ret.="\t\tif ( ". + join("&&", map { + my $uid=getpwnam($_); + if (! defined $uid) { + error(sprintf(gettext("cannot determine id of untrusted committer %s"), $_)); + } + "u != $uid"; + } @{$config{untrusted_committers}}). + ") exit(0);\n"; + $ret.=<<"EOF"; + asprintf(&s, "CALLER_UID=%i", u); + newenviron[i++]=s; + } +EOF + return $ret; +} #}}} + sub test () { #{{{ exit 0 if trusted(); diff --git a/IkiWiki/Wrapper.pm b/IkiWiki/Wrapper.pm index 0a2b8d4f8..fd8a0e5b0 100644 --- a/IkiWiki/Wrapper.pm +++ b/IkiWiki/Wrapper.pm @@ -36,7 +36,13 @@ sub gen_wrapper () { #{{{ addenv("$var", s); EOF } - + + my $test_receive=""; + if ($config{test_receive}) { + require IkiWiki::Receive; + $test_receive=IkiWiki::Receive::gen_wrapper(); + } + $Data::Dumper::Indent=0; # no newlines my $configstring=Data::Dumper->Dump([\%config], ['*config']); $configstring=~s/\\/\\\\/g; @@ -67,13 +73,12 @@ addenv(char *var, char *val) { } int main (int argc, char **argv) { - /* Sanitize environment. */ char *s; + +$test_receive $envsave newenviron[i++]="HOME=$ENV{HOME}"; newenviron[i++]="WRAPPED_OPTIONS=$configstring"; - asprintf(&s, "CALLER_UID=%i", getuid()); - newenviron[i++]=s; newenviron[i]=NULL; environ=newenviron; diff --git a/ikiwiki.in b/ikiwiki.in index d601d2739..f2407b8d0 100755 --- a/ikiwiki.in +++ b/ikiwiki.in @@ -123,11 +123,6 @@ sub getconfig () { #{{{ # optimisation for no-op post_commit exit 0; } - elsif ($config{test_receive}) { - # quick success if the user is trusted - require IkiWiki::Receive; - exit 0 if IkiWiki::Receive::trusted(); - } loadplugins(); checkconfig(); -- cgit v1.2.3