Bitte um kurzes Review

[Tom23]

Hi Leute!

Ich habe früher viele Webandwendungen mit ASP gemacht und möchte nun privat auf Perl umsteigen. Doch leider habe ich von Perl und CGI noch nicht so viel Ahnung. Nichts desto trotz habe ich einen netten 40-Zeiler zusammen bekommen, der mir viel Freude bereitet. Es handelt sich um ein Skript, welches mittels XSLT einen Atom-Feed in XHTML transformiert.

Könntet ihr euch den Code mal anschauen und mir Verbesserungsvorschläge geben? Besonders im Hinblick auf die Sicherheit, da ich das Skript bald einmal ins Internet stellen möchte.

MfG & vielen Dank

Tom23

PS: Den überlangen Kommentar habe ich am Schluss angehängt.

#!/usr/bin/perl -w

Load modules

use strict;
use CGI qw(standard);
use CGI::Carp qw(fatalsToBrowser);

Set configuration constants

use constant FEED => '../RealWorld/xhtml/news.xml';
use constant FEED2XHTML => '../RealWorld/feed_atom.xsl';
use constant XHTMLTEMPLATE => '../RealWorld/template.xsl';
use constant PLAINCACHE => 'feed.tmp';

Initialize variables

my $cgi = new CGI or die('Error while initializing CGI');
my $xslt_param_plain = ' --novalid --nonet';
my $xslt_param = $xslt_param_plain;
my $id = $cgi->param('id');
my $cat = $cgi->param('c');
my $mode = $cgi->param('m');

Filter parameters

$id = ($id and $id =~ /^\d$/) ? $id : undef;
$cat = ($cat and $cat =~ /^\w+?$/) ? $cat : undef;
$mode = ($mode and $mode =~ /^\w+?$/) ? $mode : undef;

Compose xsltproc parameters

$xslt_param .= " --param entry $id"     if ($id);
$xslt_param .= " --stringparam category  "$cat"" if ($cat);
$xslt_param .= " --stringparam param "m=$mode"" if ($mode);

Transform the feed

if ($mode and $mode eq 'raw') {
 print "Content-type: text/xml\n\n";
 system('cat '.FEED);
} else {
 print "Content-type: text/html\n\n";
 system("/usr/bin/xsltproc $xslt_param ".FEED2XHTML.' '.FEED.'>'.PLAINCACHE);
 if ($mode and $mode eq 'plain') {
  system('cat ' . PLAINCACHE);
 } else {
  system("/usr/bin/xsltproc $xslt_param_plain ".XHTMLTEMPLATE.' '.PLAINCACHE);
 }
}

END OF SCRIPT

###############################################################################

File:  feed.cgi

Version: 0.4 Beta

###############################################################################

Vision

From the MCV perspective, this is a simple control that calles several views

(XSL stylesheets) to display a specific data model (Atom feeds).

It should be simple but secure and as reusable as possible.

###############################################################################

Manual

This script can be used to deliver Atom feeds in a flexible way. It uses

xsltproc and a couple of XSL stylesheets to tranform a feed into plain or

completly layouted XHTML.

You can also choose a single entry or only a category of entries, but

this is handled by the XSL stylesheet. We only need to pass the correct

parameters to xsltproc. Have a look at the parameters and their descriptions.

ID : Summary vs. Single Entry

- ?id=1  Returns the entry which's atom:id ends with 'id=1'.

#    Without any id parameter you'll see a summary (no atom:content).

Mode : Enhanced, plain XHTML or raw Atom

-    Enhanced XHTML is the default mode.

- ?m=plain Returns a minimalistic XHTML version

- ?m=raw  Returns the atom feed without any transformation.

#    Not even id or category filtering is done.

Category : Whatever your feed knows

- ?c=News  This filters the feed for entries with the category term set

#    to "News" and returns a summary. This only has an effect without
#    an ID given.
###############################################################################

Setup

0. You'll need a working CGI environment and xsltproc

1. Create the PLAINCACHE file and make it modifyable by the webserver

2. Place stylesheets and the feed on the server

4. Adjust the configuration constants in this script

5. Place this script in your CGI folder and make it executable

6. Check if everything works fine.

###############################################################################

Release History

0.4 Summaries by category and fixed ID filtering (in feed_atom.xsl)

0.3 Implements summary and sinle entry views (ID)

0.2 Choose between raw Atom or plain and enhanced XHTML

0.1 Transformation from Atom to plain XHTML works

###############################################################################

Future Releases

0.5 External code audit and Atom/XHTML validation

0.6 Choose from several atom feeds (maybe aggregateable)

0.7 Usable from outside of CGI (shell), check cache date before retransform

0.8 Transform other feed formats (RSS Version x.y)

0.9 Try to hack it (heavy research required)

1.0 Remove debugging stuff and make it open to the public

###############################################################################

[Struppi]

#!/usr/bin/perl -w

Load modules

use strict;
use CGI qw(standard);

Das standard kannste weglassen, du nutzt das Modul ja Objektorientiert.

system('cat '.FEED);

Wozu nutzt du hier ein Systemkommando?

Gibt es für die Transformierung keine Module bei CPAN?

Struppi.

[Alexander (HH)]

Moin Moin!

#!/usr/bin/perl -w

-T fehlt.

Set configuration constants

use constant FEED => '../RealWorld/xhtml/news.xml';
use constant FEED2XHTML => '../RealWorld/feed_atom.xsl';
use constant XHTMLTEMPLATE => '../RealWorld/template.xsl';

Wer oder was garantiert Dir, dass das Script im "richtigen" Verzeichnis aufgerufen wird? Absolute Pfade würden meiner Paranoia weniger weh tun.

use constant PLAINCACHE => 'feed.tmp';

Das aktuelle Verzeichnis ist für den WWW-User (sprich: das GESAMTE Internet) BESCHREIBBAR? SOFORT ABSTELLEN!

Die Temp-Datei ist ohnehin unnötig, siehe unten. Ansonsten benutze ein Modul, das sicheren Umgang mit Temp-Files garantiert.

Initialize variables

my $cgi = new CGI or die('Error while initializing CGI');
my $xslt_param_plain = ' --novalid --nonet';
my $xslt_param = $xslt_param_plain;
my $id = $cgi->param('id');
my $cat = $cgi->param('c');
my $mode = $cgi->param('m');

Filter parameters

$id = ($id and $id =~ /^\d$/) ? $id : undef;
$cat = ($cat and $cat =~ /^\w+?$/) ? $cat : undef;
$mode = ($mode and $mode =~ /^\w+?$/) ? $mode : undef;

Compose xsltproc parameters

$xslt_param .= " --param entry $id"     if ($id);
$xslt_param .= " --stringparam category  "$cat"" if ($cat);
$xslt_param .= " --stringparam param "m=$mode"" if ($mode);

Umständlich. Und wegen dem system("xsltproc") weiter unten und dem -T von oben nicht wirklich guter Stil.

my @xslt_with_arguments=('/usr/bin/xsltproc');
push @xslt_with_arguments,'--param','entry',$1 if $id=~/^(\d+)$/;
push @xslt_with_arguments,'--stringparam','category',$1 if $cat=~/^(\w+)$/;
push @xslt_with_arguments,'--stringparam','param',$1 if $mode=~/^(\w+)$/;

Auf diese Art sparst Du Dir jede Menge Ärger mit der Shell (siehe unten) und übergibst xsltproc nur geprüfte Parameter.

Transform the feed

if ($mode and $mode eq 'raw') {
print "Content-type: text/xml\n\n";
system('cat '.FEED);

Wer garantiert Dir, dass nicht irgendwo in $ENV{'PATH'} ein cat-Programm existiert, das etwas völlig Unerwartetes macht (böse Sprüche ausgeben, Festplatte plätten, Oma umbringen)?

Minimale Perl-Lösung:

open FILE,'<',FEED or die; print while <FILE>; close FILE;

} else {
print "Content-type: text/html\n\n";
system("/usr/bin/xsltproc $xslt_param ".FEED2XHTML.' '.FEED.'>'.PLAINCACHE);
if ($mode and $mode eq 'plain') {
  system('cat ' . PLAINCACHE);
} else {
  system("/usr/bin/xsltproc $xslt_param_plain ".XHTMLTEMPLATE.' '.PLAINCACHE);
}
}

END OF SCRIPT

Ganz übel!

Parameter solltest Du in einem Array übergeben, statt sie erst zu einem String zusammenzukleben, die die Shell wieder auseinander pflücken muß (und dabei gelegentlich katastrophalen Mist baut).

Temp-File ist unnötig, Du kannst das Programm direkt nach STDOUT schreiben lassen. Fester Name für das Temp-File bedeutet Datenverlust bei gleichzeitigem Zugriff mehrerer Benutzer.

perldoc perlipc: Safe Pipe opens sollte Dir helfen. Im Child machst Du jeweils ein exec @xlst_with_arguments ohne IO Redirection, im Parent ein print while <KID_TO_READ>.

Noch eleganter ist allerdings, da das Script hier ohnehin zu Ende ist, es direkt nach der Ausgabe des Headers mit exec @xlst_with_arguments durch xsltproc mit den passenden Parametern zu ersetzen.

Alexander

[Alexander (HH)]

Moin Moin!

} else {
print "Content-type: text/html\n\n";
system("/usr/bin/xsltproc $xslt_param ".FEED2XHTML.' '.FEED.'>'.PLAINCACHE);
if ($mode and $mode eq 'plain') {
  system('cat ' . PLAINCACHE);
} else {
  system("/usr/bin/xsltproc $xslt_param_plain ".XHTMLTEMPLATE.' '.PLAINCACHE);
}
}

END OF SCRIPT

Noch eleganter ist allerdings, da das Script hier ohnehin zu Ende ist, es direkt nach der Ausgabe des Headers mit exec @xlst_with_arguments durch xsltproc mit den passenden Parametern zu ersetzen.

Oops, hab überseten, das u.U. xsltproc zweimal aufgerufen wird. Da hilft nur Safe Pipe Open, oder xlstproc dazu überreden, ggf. die zwei Transformationen in einem Aufruf zu erledigen.

Alexander

[Tom23]

Hallo Alexander

#!/usr/bin/perl -w
-T fehlt.

Vielen Dank für die lehrreiche Antwort!  Dies hier habe ich angepasst und mit Mühe und Not wieder zum Laufen gebracht. Guter Hinweis auf perlsec! Aber ich hab da noch ein paar Fragen...

use constant XHTMLTEMPLATE => '../RealWorld/template.xsl';
Wer oder was garantiert Dir, dass das Script im "richtigen" Verzeichnis aufgerufen wird? Absolute Pfade würden meiner Paranoia weniger weh tun.

Es gibt nur ein CGI-Verzeichnis auf dem Webserver. Ich bevorzuge eigentlich relative Pfade weil ich sie für flexibler halte (sofern Relationen stimmen): Ich kann das komplette '../'-Verzeichnis frei bewegen. Warum macht dir das Angst? Ich habe jetzt trotzdem absoltue Pfade verwendet:

Set configuration constants

use constant BASEPATH => '/var/www/';
use constant FEED => BASEPATH . 'RealWorld/xhtml/news.xml';

use constant PLAINCACHE => 'feed.tmp';
Das aktuelle Verzeichnis ist für den WWW-User (sprich: das GESAMTE Internet) BESCHREIBBAR? SOFORT ABSTELLEN!

Sowas in der Art habe ich erwartet ;-) Aber keine Angst, das Skript hängt nicht am Netz.
Es ist nur diese eine Datei durch www-data veränderbar. Das Skript und die Temp-Datei liegen im CGI-Verzeichnis, wobei der Apache "nur" .cgi-Dateien ausführen darf. feed.tmp ist aber nicht executable und wird vor jeder Verwendung komplett neu erzeugt.

»  Ansonsten benutze ein Modul, das sicheren Umgang mit Temp-Files garantiert.
Zum Beispiel File::Temp? Das muss ich morgen mal ausprobieren.

Auf diese Art sparst Du Dir jede Menge Ärger mit der Shell (siehe unten) und übergibst xsltproc nur geprüfte Parameter.

Filtern habe ich probiert, nur mit dem Untainting hat es nicht auf Anhieb geklappt. Da ich zumindest $mode auch als Variable brauche, habe ich die zwei Schritte beibehalten:

Untaint parameters

$id   = ($id   =~ /^(\d+)$/) ? $1 : undef;
$cat  = ($cat  =~ /^(\w+)$/) ? $1 : undef;
$mode = ($mode =~ /^(\w+)$/) ? $1 : undef;

Compose xsltproc parameters

push @param, '--param','entry',$id if $id;
push @param, '--stringparam', 'category', $cat if $cat;
push @param, '--stringparam', 'param', "m=$mode" if $mode;

system('cat '.FEED);
Wer garantiert Dir, dass nicht irgendwo in $ENV{'PATH'} ein cat-Programm existiert, das etwas völlig Unerwartetes macht (böse Sprüche ausgeben, Festplatte plätten, Oma umbringen)?

Ähm... $ENV{'PATH'} = '/usr/bin/'; # Jetzt neu in meinem Skript (für xsltproc)

open FILE,'<',FEED or die; print while <FILE>; close FILE;

Das habe ich übernommen.

system("/usr/bin/xsltproc $xslt_param_plain ".XHTMLTEMPLATE.'
Parameter solltest Du in einem Array übergeben, statt sie erst zu einem String zusammenzukleben, die die Shell wieder auseinander pflücken muß (und dabei gelegentlich katastrophalen Mist baut).

Habe ich nun auch gemacht. In @xslt sthet der Befehl und die sauberen, beiden Aufrufen gemeinsamen Werte. Den Rest hänge ich so an:

system(@xslt, XHTMLTEMPLATE, PLAINCACHE);

Temp-File ist unnötig, Du kannst das Programm direkt nach STDOUT schreiben lassen. Fester Name für das Temp-File bedeutet Datenverlust bei gleichzeitigem Zugriff mehrerer Benutzer.

Leider brauche ich das Temp-File. Ich schreibe nun über die --output-Option von xsltproc in die Datei. Ob dies vor simultanen Zugriffen schützt wage muss ich noch abklären. Eine geschützte Datei würde sich gut als Cache eignen, aber eventuell werde ich für jeden Aufruf eine eigene Temp-Datei erzeugen müssen.

perldoc perlipc: Safe Pipe opens sollte Dir helfen. Im Child machst Du jeweils ein exec @xlst_with_arguments ohne IO Redirection, im Parent ein print while <KID_TO_READ>.

Phu, das war schwere Kost! Doch leider scheinen mir diese Pipes für meinen Zweck nicht ganz richtig geeignet. (Ich brauche Dateien, nicht deren Inhalt).

Ich habe schon ein paar mal versucht das Modul XML:LibXSLT zu installieren, doch leider haut das bei mir nicht hin. Konnte nicht heraus finden wo das Problem liegt. Doch um von den system-Calls weg zu kommen gibt es wohl nichts besseres.

Denkst du das haut jetzt hin? Gruss & Dank

Tom

[Alexander (HH)]

Moin Moin!

use constant XHTMLTEMPLATE => '../RealWorld/template.xsl';
Wer oder was garantiert Dir, dass das Script im "richtigen" Verzeichnis aufgerufen wird? Absolute Pfade würden meiner Paranoia weniger weh tun.

Es gibt nur ein CGI-Verzeichnis auf dem Webserver. Ich bevorzuge eigentlich relative Pfade weil ich sie für flexibler halte (sofern Relationen stimmen): Ich kann das komplette '../'-Verzeichnis frei bewegen. Warum macht dir das Angst? Ich habe jetzt trotzdem absoltue Pfade verwendet:

Wer garantiert Dir, das zu jeder Zeit das CGI-Verzeichnis das aktuelle Verzeichnis ist? Irgendein nachgeladenes Modul oder eine externe Library könnte chdir() aufrufen -- vielleicht sogar in der Absicht, dass wieder rückgängig zu machen, was aber aus merkwürdigen Gründen (Permissions, ...) nicht klappt.

use constant PLAINCACHE => 'feed.tmp';
Das aktuelle Verzeichnis ist für den WWW-User (sprich: das GESAMTE Internet) BESCHREIBBAR? SOFORT ABSTELLEN!
Sowas in der Art habe ich erwartet ;-) Aber keine Angst, das Skript hängt nicht am Netz.

Je nach Quelle kommen 75% bis 90% der Angriffe von innen.

Es ist nur diese eine Datei durch www-data veränderbar. Das Skript und die Temp-Datei liegen im CGI-Verzeichnis, wobei der Apache "nur" .cgi-Dateien ausführen darf. feed.tmp ist aber nicht executable und wird vor jeder Verwendung komplett neu erzeugt.

Und wenn zwei Requests sich überschneiden (den Scheduler des Betriebssystems kannst Du nicht aussperren)? Dann schrottet der eine Request die Datei, die der andere Request gerade mühsam aufgebaut hat.

»  Ansonsten benutze ein Modul, das sicheren Umgang mit Temp-Files garantiert.
Zum Beispiel File::Temp? Das muss ich morgen mal ausprobieren.

Ja, aber bitte nicht die Legacy-Funktionen.

Leider brauche ich das Temp-File. Ich schreibe nun über die --output-Option von xsltproc in die Datei. Ob dies vor simultanen Zugriffen schützt wage muss ich noch abklären.

Vermutlich nicht. O_CREATE und O_EXCL müßten beim Aufruf von open() in xsltproc gesetzt sein und es darf kein NFS im Spiel sein, damit das einigermaßen klappt. Die open()-Manpage rät dazu, separate Lock-Files zu benutzen.

Eine geschützte Datei würde sich gut als Cache eignen, aber eventuell werde ich für jeden Aufruf eine eigene Temp-Datei erzeugen müssen.

Richtig. Und der Name sollte nicht vorhersagbar sein. File::Temp sollte helfen.

perldoc perlipc: Safe Pipe opens sollte Dir helfen. Im Child machst Du jeweils ein exec @xlst_with_arguments ohne IO Redirection, im Parent ein print while <KID_TO_READ>.

Phu, das war schwere Kost! Doch leider scheinen mir diese Pipes für meinen Zweck nicht ganz richtig geeignet. (Ich brauche Dateien, nicht deren Inhalt).

Letztlich sendest Du den Dateiinhalt. Wenn Du xsltproc in einem Subprozess dazu bringst, nach STDOUT zu schrieben, kannst Du die Ausgabe im Parent per KID_TO_READ einsammeln -- ganz ohne Temp-Datei.

Ich habe schon ein paar mal versucht das Modul XML:LibXSLT zu installieren, doch leider haut das bei mir nicht hin. Konnte nicht heraus finden wo das Problem liegt.

Betriebssystem? Version? Distribution? Version? Perl-Version? Fehlermeldung von cpan -i XML::LibXSLT?

Doch um von den system-Calls weg zu kommen gibt es wohl nichts besseres.

Denkst du das haut jetzt hin? Gruss & Dank

Du hast immer noch Race Conditions rund um das Temp-File. Und wenn ich die Paranoia richtig hochjage, hast Du keine Garantie, dass xsltproc nicht bei passendem Input Amok läuft und Deine Oma umbringt.

Alexander