Bugzilla – Attachment 154427 Details for
Bug 25672
Administrators should be able to restrict client-side plugin upload to trusted sources
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 25672: Convert to restricting to listed repositories only
Bug-25672-Convert-to-restricting-to-listed-reposit.patch (text/plain), 14.25 KB, created by
Martin Renvoize (ashimema)
on 2023-08-15 11:45:05 UTC
(
hide
)
Description:
Bug 25672: Convert to restricting to listed repositories only
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2023-08-15 11:45:05 UTC
Size:
14.25 KB
patch
obsolete
>From 916cb085d420237fcdc6dd0f2cfb66c09206773f Mon Sep 17 00:00:00 2001 >From: Martin Renvoize <martin.renvoize@ptfs-europe.com> >Date: Tue, 25 Jul 2023 15:46:34 +0100 >Subject: [PATCH] Bug 25672: Convert to restricting to listed repositories only > >I think instead of a plain on/off switch we should use it in combination >with the plugin_repo's and set it to restrict to only those repos' (i.e. >disable uploads entirely if no repo's are listed, or just allow those >repo's when there are). > >This patch achieves that, but only if plugins are installed via the >plugin browser method. We disable all direct upload avenues, so install >is blocked for other cases. >--- > debian/templates/koha-conf-site.xml.in | 2 +- > etc/koha-conf.xml | 4 +- > .../en/modules/plugins/plugins-disabled.tt | 6 +- > .../prog/en/modules/plugins/plugins-home.tt | 8 +-- > .../prog/en/modules/plugins/plugins-upload.tt | 1 + > plugins/plugins-home.pl | 2 +- > plugins/plugins-uninstall.pl | 3 +- > plugins/plugins-upload.pl | 55 +++++++++++++------ > 8 files changed, 48 insertions(+), 33 deletions(-) > >diff --git a/debian/templates/koha-conf-site.xml.in b/debian/templates/koha-conf-site.xml.in >index 3895c5667c..7f4a645c0d 100644 >--- a/debian/templates/koha-conf-site.xml.in >+++ b/debian/templates/koha-conf-site.xml.in >@@ -266,7 +266,6 @@ __END_SRU_PUBLICSERVER__ > <authorityservershadow>1</authorityservershadow> > <pluginsdir>__PLUGINS_DIR__</pluginsdir> <!-- This entry can be repeated to use multiple directories --> > <enable_plugins>0</enable_plugins> >- <enable_plugin_browser_upload>1</enable_plugin_browser_upload> > <upload_path>__UPLOAD_PATH__</upload_path> > <tmp_path>__TMP_PATH__</tmp_path> > <intranetdir>/usr/share/koha/intranet/cgi-bin</intranetdir> >@@ -412,6 +411,7 @@ __END_SRU_PUBLICSERVER__ > developers use it to catch bugs related to strict SQL modes --> > <dev_install>0</dev_install> > <strict_sql_modes>0</strict_sql_modes> >+ <plugins_restricted>1</plugins_restricted> > <plugin_repos> > <!-- > <repo> >diff --git a/etc/koha-conf.xml b/etc/koha-conf.xml >index 5b350732d5..eafd43562f 100644 >--- a/etc/koha-conf.xml >+++ b/etc/koha-conf.xml >@@ -79,7 +79,6 @@ > <authorityservershadow>1</authorityservershadow> > <pluginsdir>__PLUGINS_DIR__</pluginsdir> <!-- This entry can be repeated to use multiple directories --> > <enable_plugins>0</enable_plugins> >- <enable_plugin_browser_upload>1</enable_plugin_browser_upload> > <upload_path></upload_path> > <tmp_path></tmp_path> > <intranetdir>__INTRANET_CGI_DIR__</intranetdir> >@@ -229,8 +228,8 @@ > developers use it to catch bugs related to strict SQL modes --> > <dev_install>0</dev_install> > <strict_sql_modes>0</strict_sql_modes> >+ <plugins_restricted>1</plugins_restricted> > <plugin_repos> >- <!-- > <repo> > <name>ByWater Solutions</name> > <org_name>bywatersolutions</org_name> >@@ -246,7 +245,6 @@ > <org_name>ptfs-europe</org_name> > <service>github</service> > </repo> >- --> > </plugin_repos> > > <koha_xslt_security> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-disabled.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-disabled.tt >index 4b25a95829..98c6d73551 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-disabled.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-disabled.tt >@@ -24,9 +24,9 @@ > <div class="container-fluid"> > <div class="row"> > <div class="col-sm-6 col-sm-offset-3 col-md-6 col-md-offset-3"> >- [% IF ( browser_upload_enabled.defined ) && ( browser_upload_enabled == 0 ) %] >- <h1>Plugin browser upload disabled!</h1> >- <p>To enable Koha plugin browser upload, the flag enable_plugin_browser_upload must be set in the Koha configuration file</p> >+ [% IF ( plugins_restricted.defined ) && ( plugins_restricted == 1 ) %] >+ <h1>Plugin upload is restricted to only those plugins listed by your server administrator!</h1> >+ <p>To enable unrestricted plugin uploads, ask your administrator to unset plugins_restricted in the Koha configuration file</p> > [% ELSE %] > <h1>Plugins disabled!</h1> > <p>To enable Koha plugins, the flag enable_plugins must be set in the Koha configuration file</p> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-home.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-home.tt >index a7c49eee4f..de9afe9fc1 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-home.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-home.tt >@@ -45,7 +45,7 @@ > > [% IF ( CAN_user_plugins_manage ) %] > <div class="btn-toolbar" id="toolbar"> >- [% IF ( enable_browser_upload ) %] >+ [% IF ( !plugins_restricted ) %] > <a href="/cgi-bin/koha/plugins/plugins-upload.pl" id="upload_plugin" class="btn btn-default"><i class="fa fa-upload"></i> Upload plugin</a> > [% END %] > >@@ -89,9 +89,7 @@ > <th>Description</th> > <th>Organization</th> > <th>Latest version</th> >- [% IF ( enable_browser_upload ) %] > <th> </th> >- [% END %] > </tr> > </thead> > >@@ -101,9 +99,7 @@ > <td>[% sr.result.description | html %]</td> > <td>[% sr.repo.name | html %]</td> > <td>[% sr.result.tag_name | html %]</td> >- [% IF ( enable_browser_upload ) %] > <td><a class="btn btn-default btn-sm btn-install-plugin" href="/cgi-bin/koha/plugins/plugins-upload.pl?op=Upload&uploadfile=[% sr.result.install_name | uri %]&uploadlocation=[% sr.result.install_url | uri %]"><i class="fa fa-download"></i> Install</a></td> >- [% END %] > </tr> > [% END %] > </table> >@@ -219,9 +215,7 @@ > [% END %] > [% END %] > [% IF ( CAN_user_plugins_manage ) %] >- [% IF ( enable_browser_upload ) %] > <li><a class="uninstall_plugin" data-plugin-name="[% plugin.metadata.name | html %]" href="/cgi-bin/koha/plugins/plugins-uninstall.pl?class=[% plugin.class | html %]"><i class="fa fa-trash-can fa-fw"></i> Uninstall</a></li> >- [% END %] > [% IF ( plugin.is_enabled ) %] > <li><a class="enable_plugin" data-plugin-name="[% plugin.metadata.name | html %]" href="/cgi-bin/koha/plugins/plugins-enable.pl?class=[% plugin.class | html %]&method=disable"><i class="fa fa-pause fa-fw"></i> Disable</a></li> > [% ELSE %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-upload.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-upload.tt >index 96e74dc7f6..ccaba71134 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-upload.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/plugins/plugins-upload.tt >@@ -34,6 +34,7 @@ > [% ELSIF ( ERROR.EMPTYUPLOAD ) %]<li><strong>The upload file appears to be empty.</strong></li> > [% ELSIF ( ERROR.UZIPFAIL ) %]<li><strong>[% ERROR.UZIPFAIL | html %] failed to unpack.<br />Please verify the integrity of the zip file and retry.</strong></li> > [% ELSIF ( ERROR.NOWRITEPLUGINS ) %]<li><strong>Cannot unpack file to the plugins directory.<br />Please verify that the Apache user can write to the plugins directory.</strong></li> >+ [% ELSIF ( ERROR.RESTRICTED ) %]<li><strong>Cannot install plugin from unknown source whilst plugins_restricted is enabled.<br /></strong></li> > [% ELSE %]<li><strong>[% ERROR.CORERR | html %] An unknown error has occurred.<br />Please review the error log for more details.</strong></li>[% END %] > [% END %] > </div> >diff --git a/plugins/plugins-home.pl b/plugins/plugins-home.pl >index 4b76299535..95dd79231a 100755 >--- a/plugins/plugins-home.pl >+++ b/plugins/plugins-home.pl >@@ -59,7 +59,7 @@ if ($plugins_enabled) { > ); > > $template->param( plugins => \@plugins, ); >- $template->param( enable_browser_upload => C4::Context->config('enable_plugin_browser_upload') ); >+ $template->param( plugins_restricted => C4::Context->config('plugins_restricted') ); > > $template->param( can_search => C4::Context->config('plugin_repos') ? 1 : 0 ); > my @results; >diff --git a/plugins/plugins-uninstall.pl b/plugins/plugins-uninstall.pl >index 50ee3490c3..c797af16c3 100755 >--- a/plugins/plugins-uninstall.pl >+++ b/plugins/plugins-uninstall.pl >@@ -38,10 +38,9 @@ my ( $template, $loggedinuser, $cookie ) = get_template_and_user( > } > ); > >-my $browser_upload_enabled = C4::Context->config('enable_plugin_browser_upload'); > my $class = $input->param('class'); > >-if ( $browser_upload_enabled && $class ) { >+if ( $class ) { > Koha::Plugins::Handler->delete( { class => $class } ); > } > >diff --git a/plugins/plugins-upload.pl b/plugins/plugins-upload.pl >index 1fe6b3b041..fcc20238e4 100755 >--- a/plugins/plugins-upload.pl >+++ b/plugins/plugins-upload.pl >@@ -24,27 +24,33 @@ use Mojo::UserAgent; > use File::Temp; > > use C4::Context; >-use C4::Auth qw( get_template_and_user ); >+use C4::Auth qw( get_template_and_user ); > use C4::Output qw( output_html_with_http_headers ); > use C4::Members; > use Koha::Logger; > use Koha::Plugins; > >-my $plugins_enabled = C4::Context->config("enable_plugins"); >-my $browser_upload_enabled = C4::Context->config('enable_plugin_browser_upload'); >+my $plugins_enabled = C4::Context->config("enable_plugins"); >+my $plugins_restricted = C4::Context->config("plugins_restricted"); > > my $input = CGI->new; > > my ( $template, $loggedinuser, $cookie ) = get_template_and_user( >- { template_name => ($plugins_enabled && $browser_upload_enabled) ? "plugins/plugins-upload.tt" : "plugins/plugins-disabled.tt", >+ { >+ template_name => $plugins_enabled ? "plugins/plugins-upload.tt" : "plugins/plugins-disabled.tt", > query => $input, > type => "intranet", >- flagsrequired => { plugins => 'manage' }, >+ flagsrequired => { plugins => 'manage' }, > } > ); > >-if ($plugins_enabled){ >- $template->param( browser_upload_enabled => $browser_upload_enabled ); >+if ($plugins_enabled) { >+ $template->param( plugins_restricted => $plugins_restricted ); >+} >+ >+# Exit early if uploads are not enabled >+if ( !$ ) { >+ output_html_with_http_headers $input, $cookie, $template->output; > } > > my $uploadfilename = $input->param('uploadfile'); >@@ -56,7 +62,7 @@ my ( $tempfile, $tfh ); > > my %errors; > >-if ($plugins_enabled && $browser_upload_enabled) { >+if ($plugins_enabled) { > if ( ( $op eq 'Upload' ) && ( $uploadfile || $uploadlocation ) ) { > my $plugins_dir = C4::Context->config("pluginsdir"); > $plugins_dir = ref($plugins_dir) eq 'ARRAY' ? $plugins_dir->[0] : $plugins_dir; >@@ -67,22 +73,39 @@ if ($plugins_enabled && $browser_upload_enabled) { > $filesuffix = $1 if $uploadfilename =~ m/(\..+)$/i; > ( $tfh, $tempfile ) = File::Temp::tempfile( SUFFIX => $filesuffix, UNLINK => 1 ); > >- $errors{'NOTKPZ'} = 1 if ( $uploadfilename !~ /\.kpz$/i ); >+ $errors{'NOTKPZ'} = 1 if ( $uploadfilename !~ /\.kpz$/i ); > $errors{'NOWRITETEMP'} = 1 unless ( -w $dirname ); > $errors{'NOWRITEPLUGINS'} = 1 unless ( -w $plugins_dir ); > >- if ( $uploadlocation ) { >- my $ua = Mojo::UserAgent->new(max_redirects => 5); >- my $tx = $ua->get($uploadlocation); >- $tx->result->content->asset->move_to($tempfile); >+ if ($uploadlocation) { >+ my $do_get = 1; >+ if ( $plugins_restricted ) { >+ my $repos = C4::Context->config('plugin_repos'); >+ >+ # Fix data structure if only one repo defined >+ if ( ref($repos->{repo}) eq 'HASH' ) { >+ $repos = { repo => [ $repos->{repo} ] }; >+ } >+ >+ $do_get = any { index($uploadlocation, $_->{org_name}) != -1 } @{ $repos->{repo} }; >+ } >+ >+ if ( $do_get ) { >+ my $ua = Mojo::UserAgent->new( max_redirects => 5 ); >+ my $tx = $ua->get($uploadlocation); >+ $tx->result->content->asset->move_to($tempfile); >+ } else { >+ $errors{'RESTRICTED'} = 1; >+ } > } else { >- $errors{'EMPTYUPLOAD'} = 1 unless ( length($uploadfile) > 0 ); >+ $errors{'RESTRICTED'} = 1 unless ( !$plugins_restricted ); >+ $errors{'EMPTYUPLOAD'} = 1 unless ( length($uploadfile) > 0 ); > } > > if (%errors) { > $template->param( ERRORS => [ \%errors ] ); > } else { >- if ( $uploadfile ) { >+ if ($uploadfile && !$plugins_restricted) { > while (<$uploadfile>) { > print $tfh $_; > } >@@ -104,7 +127,7 @@ if ($plugins_enabled && $browser_upload_enabled) { > warn "Problem uploading file or no file uploaded."; > } > >- if ( ($uploadfile || $uploadlocation) && !%errors && !$template->param('ERRORS') ) { >+ if ( ( $uploadfile || $uploadlocation ) && !%errors && !$template->param('ERRORS') ) { > print $input->redirect("/cgi-bin/koha/plugins/plugins-home.pl"); > } else { > output_html_with_http_headers $input, $cookie, $template->output; >-- >2.41.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=154427">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 25672
:
106651
|
106671
|
106673
|
106674
|
107172
|
107173
|
107174
|
153824
|
153825
|
153826
|
153827
|
153925
|
154423
|
154424
|
154425
|
154426
|
154427
|
154453
|
154455
|
154464
|
154465
|
154466
|
154467
|
154468
|
154474
|
154475
|
154476
|
154477
|
154478
|
156715
|
156716
|
156717
|
156718
|
156719
|
156720
|
156734
|
156735
|
156736
|
156737
|
156738
|
156739
|
156740
|
157730
|
157810
|
157811
|
157812
|
157813
|
157814
|
157815
|
157816
|
157817
|
157836
|
157837
|
157838
|
157839
|
157840
|
157841
|
157842
|
157843
|
157846
|
157847
|
157848
|
157849
|
157850
|
157851
|
157852
|
157853