[PATCH v3 4/4] fff-web: Label Freifunk routers individually in wifiscan.html

[Tim Niemeyer]

Hi Adrian

Am Samstag, den 20.01.2018, 23:17 +0100 schrieb mail at adrianschmutzler.de:
> Hallo Tim,
> 
> siehe unten.
> 
> > -----Original Message-----
> > > > From: Tim Niemeyer [mailto:tim at tn-x.org]
> > Sent: Samstag, 20. Januar 2018 12:43
> > > > To: Adrian Schmutzler <freifunk at adrianschmutzler.de>; franken-
> > dev at freifunk.net
> > Subject: Re: [PATCH v3 4/4] fff-web: Label Freifunk routers individually in
> > wifiscan.html
> > 
> > Am Sonntag, den 26.11.2017, 14:01 +0100 schrieb Adrian Schmutzler:
> > > This patch enables labelling of routers in the WebUI's WiFi scan page,
> > > so their hostname is displayed instead of freifunk.franken.net. The
> > > evaluation is performed based on a WifiAnalyzer style node file
> > > /tmp/wifinodelist.
> > > 
> > > In the WebUI, this file may be provided by file upload or by
> > > downloading from the Monitoring API (if the router has internet
> > > access).
> > > 
> > > > > > > > Signed-off-by: Adrian Schmutzler <freifunk at adrianschmutzler.de>
> > > > Tested-by: Adrian Schmutzler <freifunk at adrianschmutzler.de>
> > > 
> > > ---
> > > 
> > > Changes in v2:
> > > none
> > > 
> > > Changes in v3:
> > > - Put to the end of the patchset
> > > - Use wifianalall API (hood-independent) as this is now available
> > >   and eves the file for all hoods has only < 200 kB
> > 
> > Was meinst du mit "eves"?
> 
> Typo: "even"
Achso ich war mir nicht ganz sicher, ob das eine Feststellung (die ja
nur für den Moment gilt) war, oder ob das ein Ausdruck oder Hoffnung
sein sollte. Die Feststellung beruhigt mich, für den Moment, etwas. ;)

> 
> > 
> > Skaliert das mit allen Knoten überhaupt?
> 
> Die Frage verstehe ich nicht. Hat im Test wunderbar geklappt.

Du hast es vielleicht mit 5000 Knoten getestet. Aber was ist, wenn es
mal 15000 sind?

> 
> > 
> > > - Reduced upload file size to 1MB
> > > ---
> > >  .../fff-web/files/www/ssl/cgi-bin/wifiscan.html    | 79
> > > +++++++++++++++++++++-
> > >  1 file changed, 77 insertions(+), 2 deletions(-)
> > > 
> > > diff --git
> > > a/src/packages/fff/fff-web/files/www/ssl/cgi-bin/wifiscan.html
> > > b/src/packages/fff/fff-web/files/www/ssl/cgi-bin/wifiscan.html
> > > index f9186d2..320019b 100755
> > > --- a/src/packages/fff/fff-web/files/www/ssl/cgi-bin/wifiscan.html
> > > +++ b/src/packages/fff/fff-web/files/www/ssl/cgi-bin/wifiscan.html
> > > @@ -1,7 +1,40 @@
> > > -#!/usr/bin/haserl
> > > +#!/usr/bin/haserl --upload-dir=/tmp --upload-limit=1000
> > 
> > Ich sehe hier immer noch ein Sicherheitsrisiko!
> > 
> > Können bei dem upload-dir wichtige Status-Files etc überschrieben werden?
> > Sollte ein extra Verzeichnis gewählt werden?
> > 
> > Vielleicht mit sowas: mktemp -d, vielleicht geht auch -u (für den Fall, dass gar
> > nicht hochgeladen wurde).
> 
> Ich habe das so verstanden, dass haserl da eh einen random Namen generiert:
> 
> http://haserl.sourceforge.net/manpage.html
> 
> --upload-dir=dirspec  Defaults to "/tmp". All uploaded files are
> created with temporary filename in this directory HASERL_xxx_path
> contains the name of the temporary file. FORM_xxx_name contains the
> original name of the file, as specified by the client.
> 
> Ist nicht ganz eindeutig. Ggf. teste ich mal, was für Namen der so
> macht, kann die ja ausgeben lassen. Ich verstehe das so, dass der
> sowas wie mktemp ohnehin intrinsisch macht. Oder habe ich falsch
> verstanden, worum es dir geht?

Mir geht es darum, dass ein möglicher Angreifer ggfs Dateien in /tmp
ablegen kann, die aber gerade einen wichtigen System-Zustand speichern.

Das ist es jetzt konstruiert, aber z.B. könnte in /tmp/ eine Datei
namens 3254jkh2 stehen, in dieser steht: "system operate: normal". Ein
script wertet das regelmäßig aus, und wenn da was anderes drin steht
machts ein reboot. Das als Verzeichnis /tmp gewählt ist, könnte ein
Angreifer ggfs die Datei 3254jkh2 überschreiben und so das System zum
restarten zwingen.

Wenn du sicher sagen kannst, dass sich das haserl bereits darum
kümmert, dass da nichts ausversehen überschrieben wird und und das
temporary filename auch auf keinen Fall so sein kann wie eine Datei die
eh da grad liegt, dann ist ja alles in Ordnung. Auf der anderen Seite
frage ich mich, warum wir da überhaupt ein Risiko eingehen sollten,
wenn man es ganz einfach ohne lange Diskussion in einen eigenen Ordner
stopfen könnte.

> > > 
[..]
> > >  <%
> > > > > > > > -	readIWinfo "$iface" "td" "td"
> > > > > > > > +	if [ -s /tmp/wifinodelist ] ; then
> > > > +		firststep="$(readIWinfo "$iface" "ssid" "mac")"
> > > 
> > > +		rewriteIWinfo "$firststep"
> > 
> > Ich hab hier ein bisschen Angst, dass ggfs die Command-Line zu lang werden
> > könnte.
> 
> Was sind da die Limits? In meinem sehr vollen Netz war das nie ein
> Problem, aber das muss ja nichts heißen.
Das weiß ich grad nicht, aber es wird sicher welche geben.

> Hast du eine bessere Idee?
Ja, aber ich weiß nicht genau wie man das umsetzt.

Letztlich sollte der Code dann so aussehen:
if [ -s /tmp/wifinodelist ]; then
	readIWinfo "$iface" "ssid" "mac" | rewriteIWinfo
fi

So wäre das von der Commandline weg und würde sauber über eine Pipe
übergeben werden. Da in der rewriteIWinfo eh ein readline gemacht wird,
liegt das irgendwie sowie nahe.

Aber wie gesagt, wie und ob das mit einer Shell Funktion geht, müsste
man sich nochmal eben angucken.

Tim
-------------- nächster Teil --------------
Ein Dateianhang mit Binärdaten wurde abgetrennt...
Dateiname   : signature.asc
Dateityp    : application/pgp-signature
Dateigröße  : 484 bytes
Beschreibung: This is a digitally signed message part
URL         : <http://lists.freifunk.net/pipermail/franken-dev-freifunk.net/attachments/20180121/fb942c3d/attachment.sig>