[RFC] fff-hoods/fff-hoodutils: Use different files for www and checksum

Submitted by Adrian Schmutzler on June 10, 2018, 12:39 p.m.

Details

Message ID 1528634348-2223-1-git-send-email-freifunk@adrianschmutzler.de
State Superseded
Headers show

Commit Message

Adrian Schmutzler June 10, 2018, 12:39 p.m.
Since using the same file for detecting changes and for the hidden
AP causes trouble, this introduces two files.

Note that $hoodfilecopy is on /tmp, so reconfiguration will now
always occur on restarts.

Signed-off-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>
---
 src/packages/fff/fff-hoods/files/usr/sbin/configurehood           | 6 ++++--
 src/packages/fff/fff-hoodutils/files/lib/functions/fff/keyxchange | 7 +++++--
 2 files changed, 9 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
index 780c2ba..455674d 100755
--- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
+++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
@@ -38,7 +38,7 @@  hasInternet() {
 
 # Hidden AP check
 
-if [ -s "$hoodfilecopy" ] && isGatewayAvailable ; then
+if [ -s "$hoodfilewww" ] && isGatewayAvailable ; then
 	needwifi="0"
 	for radio in $(uci show wireless | sed -n 's,.*\.\([a-z0-9]*\)=wifi-device,\1,p'); do
 		freq="2"
@@ -91,6 +91,7 @@  else
 		if ! isGatewayAvailable ; then
 			#now we haven't a gateway in Range, we search for a hidden AP to get a keyxchangev2data file!
 			#first we delete all wifi settings
+			rm -f "$hoodfilewww" # delete this, so interfaces are recreated if reconnect with unchanged hood file takes place
 			rm -f "$hoodfilecopy" # delete this, so interfaces are recreated if reconnect with unchanged hood file takes place
 			rm -f "$sectorcopy" # always delete: no broadcast for isolated device
 			rm -f "$sectortmp"
@@ -215,7 +216,8 @@  if [ -s "$hoodfile" ]; then
 
 		# copy the file to webroot so that other mesh routers can download it;
 		# copy only after all other steps so IF can be reentered if something goes wrong
-		cp "$hoodfile" "$hoodfilecopy"
+		cp "$hoodfile" "$hoodfilecopy" # provide file for checksum verification
+		isGatewayAvailable && [ ! -s "$hoodlocal" ] && cp "$hoodfile" "$hoodfilewww" # provide file for hiddenap (NO file is served if local hood file present)
 		[ -s "$sectortmp" ] && cp "$sectortmp" "$sectorcopy"
 
 		# This is a workaround to enable alfred on devices which do not see a configap during initial setup
diff --git a/src/packages/fff/fff-hoodutils/files/lib/functions/fff/keyxchange b/src/packages/fff/fff-hoodutils/files/lib/functions/fff/keyxchange
index 30963ae..94b09bf 100644
--- a/src/packages/fff/fff-hoodutils/files/lib/functions/fff/keyxchange
+++ b/src/packages/fff/fff-hoodutils/files/lib/functions/fff/keyxchange
@@ -5,11 +5,14 @@ 
 . /usr/share/libubox/jshn.sh
 
 hoodfile="/tmp/keyxchangev2data"
-hoodfilecopy="/www/hood/keyxchangev2data"
+hoodfilewww="/www/hood/keyxchangev2data"
+hoodfilecopy="/tmp/keyxchangev2copy"
 
 getJsonPath() {
 	jsonfile=""
-	if [ -s "$hoodfilecopy" ] ; then
+	if [ -s "$hoodfilewww" ] ; then
+		jsonfile="$hoodfilewww"
+	elif [ -s "$hoodfilecopy" ] ; then
 		jsonfile="$hoodfilecopy"
 	elif [ -s "$hoodfile" ] ; then
 		jsonfile="$hoodfile"

Comments

Fabian Blaese June 13, 2018, 7:43 a.m.
Moin,

hab mir grade ein paar Gedanken dazu gemacht (inline).

> On 10. Jun 2018, at 14:39, Adrian Schmutzler <freifunk@adrianschmutzler.de> wrote:
> 
> diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> index 780c2ba..455674d 100755
> --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> @@ -38,7 +38,7 @@ hasInternet() {
> 
> # Hidden AP check
> 
> -if [ -s "$hoodfilecopy" ] && isGatewayAvailable ; then
> +if [ -s "$hoodfilewww" ] && isGatewayAvailable ; then
> 	needwifi="0"
> 	for radio in $(uci show wireless | sed -n 's,.*\.\([a-z0-9]*\)=wifi-device,\1,p'); do
> 		freq="2"
> @@ -91,6 +91,7 @@ else
> 		if ! isGatewayAvailable ; then
> 			#now we haven't a gateway in Range, we search for a hidden AP to get a keyxchangev2data file!
> 			#first we delete all wifi settings
> +			rm -f "$hoodfilewww" # delete this, so interfaces are recreated if reconnect with unchanged hood file takes place
> 			rm -f "$hoodfilecopy" # delete this, so interfaces are recreated if reconnect with unchanged hood file takes place
> 			rm -f "$sectorcopy" # always delete: no broadcast for isolated device
> 			rm -f "$sectortmp"
> @@ -215,7 +216,8 @@ if [ -s "$hoodfile" ]; then
> 
> 		# copy the file to webroot so that other mesh routers can download it;
> 		# copy only after all other steps so IF can be reentered if something goes wrong
> -		cp "$hoodfile" "$hoodfilecopy"
> +		cp "$hoodfile" "$hoodfilecopy" # provide file for checksum verification
> +		isGatewayAvailable && [ ! -s "$hoodlocal" ] && cp "$hoodfile" "$hoodfilewww" # provide file for hiddenap (NO file is served if local hood file present)

Wir sollten hier wohl besser prüfen, ob das Hoodfile vom Gateway stammt. In allen anderen Fällen sollte es nicht angeboten werden, um Randfälle zu minimieren.
z.B. mithilfe einer Variable hoodfileIsFromGateway

> +++ b/src/packages/fff/fff-hoodutils/files/lib/functions/fff/keyxchange
> @@ -5,11 +5,14 @@
> . /usr/share/libubox/jshn.sh
> 
> hoodfile="/tmp/keyxchangev2data"
> -hoodfilecopy="/www/hood/keyxchangev2data"
> +hoodfilewww="/www/hood/keyxchangev2data"
> +hoodfilecopy="/tmp/keyxchangev2copy”

Vielleicht ist es eine schlaue Idee, einen Symlink nach /www/hood zu setzen, der auf /tmp/keyxchangev2www o.ä. zeigt.
Dann spart man sich ein paar Schreibzyklen auf dem Flash.
Nach einem Reboot würde dann allerdings neu konfiguriert werden.

Jetzt wäre vielleicht ein guter Zeitpunkt diesen Patch in ein Patchset aufzuteilen und den Variablen mal sinnvolle Namen zu geben.
Etwas klarer wäre z.B.:
- hoodfiletmp
- hoodfilewww
- hoodfilechecksum

Eigentlich ist mir auch dieser keyxchangev2data Dateiname ein Dorn im Auge.
Wenn das nicht nur mir so geht, wäre es cool, das nochmal VOR Release zu ändern. (z.B. in hoodfile im www und hoodfile_www usw. in /tmp, nicht im Rahmen dieses Patch(sets))

Eigentlich ist noch eine extra Datei total Kacke, mir fällt aktuell aber auch nichts besseres ein..

Gruß
Fabian
Adrian Schmutzler June 13, 2018, 12:01 p.m.
Hallo,

siehe auch inline.

> -----Original Message-----
> From: Fabian Bläse [mailto:fabian@blaese.de]
> Sent: Mittwoch, 13. Juni 2018 09:43
> To: Adrian Schmutzler <freifunk@adrianschmutzler.de>
> Cc: franken-dev <franken-dev@freifunk.net>
> Subject: Re: [RFC PATCH] fff-hoods/fff-hoodutils: Use different files for
> www and checksum
> 
> Moin,
> 
> hab mir grade ein paar Gedanken dazu gemacht (inline).
> 
> > On 10. Jun 2018, at 14:39, Adrian Schmutzler
> <freifunk@adrianschmutzler.de> wrote:
> >
> > diff --git a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > index 780c2ba..455674d 100755
> > --- a/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > +++ b/src/packages/fff/fff-hoods/files/usr/sbin/configurehood
> > @@ -38,7 +38,7 @@ hasInternet() {
> >
> > # Hidden AP check
> >
> > -if [ -s "$hoodfilecopy" ] && isGatewayAvailable ; then
> > +if [ -s "$hoodfilewww" ] && isGatewayAvailable ; then
> > 	needwifi="0"
> > 	for radio in $(uci show wireless | sed -n 's,.*\.\([a-z0-9]*\)=wifi-
> device,\1,p'); do
> > 		freq="2"
> > @@ -91,6 +91,7 @@ else
> > 		if ! isGatewayAvailable ; then
> > 			#now we haven't a gateway in Range, we search for a
> hidden AP to get a keyxchangev2data file!
> > 			#first we delete all wifi settings
> > +			rm -f "$hoodfilewww" # delete this, so interfaces are
> recreated if reconnect with unchanged hood file takes place
> > 			rm -f "$hoodfilecopy" # delete this, so interfaces are
> recreated if reconnect with unchanged hood file takes place
> > 			rm -f "$sectorcopy" # always delete: no broadcast for
> isolated device
> > 			rm -f "$sectortmp"
> > @@ -215,7 +216,8 @@ if [ -s "$hoodfile" ]; then
> >
> > 		# copy the file to webroot so that other mesh routers can
> download it;
> > 		# copy only after all other steps so IF can be reentered if
> something goes wrong
> > -		cp "$hoodfile" "$hoodfilecopy"
> > +		cp "$hoodfile" "$hoodfilecopy" # provide file for checksum
> verification
> > +		isGatewayAvailable && [ ! -s "$hoodlocal" ] && cp "$hoodfile"
> "$hoodfilewww" # provide file for hiddenap (NO file is served if local hood
> file present)
> 
> Wir sollten hier wohl besser prüfen, ob das Hoodfile vom Gateway stammt.
> In allen anderen Fällen sollte es nicht angeboten werden, um Randfälle zu
> minimieren.
> z.B. mithilfe einer Variable hoodfileIsFromGateway

Ich habe prinzipiell kein Problem, das mit Tims (?) Idee zwecks IsFromGateway Variable zu kombinieren.

Wird dadurch in meinen Augen etwas komplizierter, dafür werden wir die "willkürlichen" Bedingungen in diesem Patch los.

> 
> > +++ b/src/packages/fff/fff-hoodutils/files/lib/functions/fff/keyxchange
> > @@ -5,11 +5,14 @@
> > . /usr/share/libubox/jshn.sh
> >
> > hoodfile="/tmp/keyxchangev2data"
> > -hoodfilecopy="/www/hood/keyxchangev2data"
> > +hoodfilewww="/www/hood/keyxchangev2data"
> > +hoodfilecopy="/tmp/keyxchangev2copy”
> 
> Vielleicht ist es eine schlaue Idee, einen Symlink nach /www/hood zu setzen,
> der auf /tmp/keyxchangev2www o.ä. zeigt.
> Dann spart man sich ein paar Schreibzyklen auf dem Flash.
> Nach einem Reboot würde dann allerdings neu konfiguriert werden.

Wenn wir wie in diesem Patch hoodfilecopy nach /tmp legen, wird sowieso nach dem Neustart neu konfiguriert. Das fand ich auch schon immer erstrebenswert (wenn was kaputt geht, wird es durch Neustart repariert und nicht erst durch Reset).

Die Frage ist, was genau man macht: Existiert der Symlink IMMER (wird also mittels uci_default gesetzt o.ä.), und nur die Datei in /tmp wird erstellt und gelöscht, oder müssen wir den Symlink bei konfigurieren mit erstellen/löschen. Ersteren Fall fände ich erstrebenswert, wenn er funktioniert.

Ich verstehe das so, dass es sich dann wieder um einen eigenen File handelt, nur dass der dann eben in /tmp liegt, und nicht um einen Link auf einen der beiden anderen Files.

> 
> Jetzt wäre vielleicht ein guter Zeitpunkt diesen Patch in ein Patchset
> aufzuteilen und den Variablen mal sinnvolle Namen zu geben.
> Etwas klarer wäre z.B.:
> - hoodfiletmp
> - hoodfilewww
> - hoodfilechecksum

Ja, finde ich gut. Ich wollte den Patch zunächst einfach halten. Wenn das gewünscht ist, würde ich die Variablen sehr gerne ordentlich umbennen. Sollte man dann auch mit den sectorfile-Variablen machen, damit es konsistent ist. Würde ich als ersten Schritt in einem Patchset machen.

> 
> Eigentlich ist mir auch dieser keyxchangev2data Dateiname ein Dorn im Auge.
> Wenn das nicht nur mir so geht, wäre es cool, das nochmal VOR Release zu
> ändern. (z.B. in hoodfile im www und hoodfile_www usw. in /tmp, nicht im
> Rahmen dieses Patch(sets))

Bin ich sehr dafür. Das manuelle Hoodfile heißt ja auch /etc/hoodfile. Ich schlage vor:

hoodfilelocal=/etc/hoodfile
hoodfiletmp=/tmp/hoodfile
hoodfilewww=/tmp/hoodfilewww
hoodfileref=/tmp/hoodfileref ("ref"=reference, checksum ist mir zu lange, außerdem ist es ja keine checksumme, sondern ein ganzes file)
sectorfilelocal=/etc/sectorfile
sectorfiletmp=/tmp/sectorfile
sectorfilewww=/tmp/sectorfilewww

Dann brauchen wir jeweils für hoodfile und sectorfile einen link von www.

> 
> Eigentlich ist noch eine extra Datei total Kacke, mir fällt aktuell aber auch
> nichts besseres ein..

Können wir ja immer noch besser machen, wenn uns was einfällt.

Wenn keine Widersprüche kommen, würde ich später mal so ein Patchset bauen.

Grüße

Adrian

> 
> Gruß
> Fabian
Fabian Blaese June 13, 2018, 2:59 p.m.
Hallo Adrian,

> On 13. Jun 2018, at 14:01, Adrian Schmutzler <mail@adrianschmutzler.de> wrote:
> 
> Wenn wir wie in diesem Patch hoodfilecopy nach /tmp legen, wird sowieso nach dem Neustart neu konfiguriert. Das fand ich auch schon immer erstrebenswert (wenn was kaputt geht, wird es durch Neustart repariert und nicht erst durch Reset).
> 
> Die Frage ist, was genau man macht: Existiert der Symlink IMMER (wird also mittels uci_default gesetzt o.ä.), und nur die Datei in /tmp wird erstellt und gelöscht, oder müssen wir den Symlink bei konfigurieren mit erstellen/löschen. Ersteren Fall fände ich erstrebenswert, wenn er funktioniert.

Sollte immer existieren. Der uhttpd (bzw. das Linux) folgt dem Link dann und findet die passende Datei nicht. Dann müsste er genau den gleichen Fehler vom Linux bekommen, wie wenn die Datei von vornherein nicht da gewesen wäre.
Vorzugsweise sollte der Symlink schon im squashfs drin sein. Ich fürchte aber, dass man das nicht sinnvoll machen kann -> uci-defaults

>> Eigentlich ist mir auch dieser keyxchangev2data Dateiname ein Dorn im Auge.
>> Wenn das nicht nur mir so geht, wäre es cool, das nochmal VOR Release zu
>> ändern. (z.B. in hoodfile im www und hoodfile_www usw. in /tmp, nicht im
>> Rahmen dieses Patch(sets))
> 
> Bin ich sehr dafür. Das manuelle Hoodfile heißt ja auch /etc/hoodfile. Ich schlage vor:
> 
> hoodfilelocal=/etc/hoodfile
> hoodfiletmp=/tmp/hoodfile
> hoodfilewww=/tmp/hoodfilewww
> hoodfileref=/tmp/hoodfileref ("ref"=reference, checksum ist mir zu lange, außerdem ist es ja keine checksumme, sondern ein ganzes file)
> sectorfilelocal=/etc/sectorfile
> sectorfiletmp=/tmp/sectorfile
> sectorfilewww=/tmp/sectorfilewww

Gefällt mir an sich soweit.
Aber ich würde entweder bei hoodfiletmp das tmp weglassen oder entsprechend an die Datei ran hängen.

Gruß
Fabian