[v4] fastd: make secret key updatesafe

To use a whitelist easy, it is neccessary to make the fastd key updatesafe
This patch safe the key to uci fff and recover it, if a key is after the update available

---
Changes in v2:
- use variable in if
- remove trailing whitespace
- remove -q
---
Changes in v3:
- use only one variable $secret
---
Changes in v4:
- remove new line
- add dependencies to fff-config
---

Signed-off-by: Christian Dresel <fff@chrisi01.de>
Reviewed-by: lemmi <lemmi@nerd2nerd.org>
---
 src/packages/fff/fff-fastd/Makefile                            |  3 ++-
 src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd | 10 +++++++++-
 2 files changed, 11 insertions(+), 2 deletions(-)

Reviewed-by: Robert Langhammer <rlanghammer@web.de>

Am 10.01.20 um 12:57 schrieb Christian Dresel:
> To use a whitelist easy, it is neccessary to make the fastd key updatesafe
> This patch safe the key to uci fff and recover it, if a key is after the update available
>
> ---
> Changes in v2:
> - use variable in if
> - remove trailing whitespace
> - remove -q
> ---
> Changes in v3:
> - use only one variable $secret
> ---
> Changes in v4:
> - remove new line
> - add dependencies to fff-config
> ---
>
> Signed-off-by: Christian Dresel <fff@chrisi01.de>
> Reviewed-by: lemmi <lemmi@nerd2nerd.org>
> ---
>  src/packages/fff/fff-fastd/Makefile                            |  3 ++-
>  src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd | 10 +++++++++-
>  2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/src/packages/fff/fff-fastd/Makefile b/src/packages/fff/fff-fastd/Makefile
> index 513775d..0d9a9b5 100644
> --- a/src/packages/fff/fff-fastd/Makefile
> +++ b/src/packages/fff/fff-fastd/Makefile
> @@ -17,7 +17,8 @@ define Package/$(PKG_NAME)
>  			 +@FASTD_ENABLE_CIPHER_NULL \
>  			 +@FASTD_WITH_STATUS_SOCKET \
>  			 +fastd \
> -			 +fff-random
> +			 +fff-random \
> +			 +fff-config
>  endef
>
>  define Package/$(PKG_NAME)/description
> diff --git a/src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd b/src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd
> index d53eb43..08ceecb 100644
> --- a/src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd
> +++ b/src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd
> @@ -15,9 +15,17 @@ uci batch <<EOF
>    set fastd.fff.mtu='1426'
>    set fastd.fff.on_up="/etc/fastd/fff/up.sh"
>    set fastd.fff.secure_handshakes='0'
> -  set fastd.fff.secret="generate"
>  EOF
>
> +if ! secret=$(uci -q get fff.fastd.secret); then
> +	secret=$(/usr/bin/fastd --generate-key --machine-readable)
> +	uci set fff.fastd='fff'
> +	uci set fff.fastd.secret="$secret"
> +	uci commit fff
> +fi
> +uci set fastd.fff.secret="$secret"
> +uci commit fastd
> +
>  [ ! -d /etc/fastd/fff ] &&  mkdir -p /etc/fastd/fff
>  ln -s /tmp/fastd_fff_peers /etc/fastd/fff/peers
>  echo "#!/bin/sh" > /etc/fastd/fff/up.sh

Reviewed-by: Adrian Schmutzler <freifunk@adrianschmutzler.de>

> -----Original Message-----
> From: franken-dev [mailto:franken-dev-bounces@freifunk.net] On Behalf Of
> robert
> Sent: Freitag, 10. Januar 2020 13:18
> To: franken-dev@freifunk.net
> Subject: Re: [PATCH v4] fastd: make secret key updatesafe
> 
> Reviewed-by: Robert Langhammer <rlanghammer@web.de>
> 
> Am 10.01.20 um 12:57 schrieb Christian Dresel:
> > To use a whitelist easy, it is neccessary to make the fastd key updatesafe
> > This patch safe the key to uci fff and recover it, if a key is after the update
> available
> >
> > ---
> > Changes in v2:
> > - use variable in if
> > - remove trailing whitespace
> > - remove -q
> > ---
> > Changes in v3:
> > - use only one variable $secret
> > ---
> > Changes in v4:
> > - remove new line
> > - add dependencies to fff-config
> > ---
> >
> > Signed-off-by: Christian Dresel <fff@chrisi01.de>
> > Reviewed-by: lemmi <lemmi@nerd2nerd.org>
> > ---
> >  src/packages/fff/fff-fastd/Makefile                            |  3 ++-
> >  src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd | 10 +++++++++-
> >  2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/packages/fff/fff-fastd/Makefile b/src/packages/fff/fff-
> fastd/Makefile
> > index 513775d..0d9a9b5 100644
> > --- a/src/packages/fff/fff-fastd/Makefile
> > +++ b/src/packages/fff/fff-fastd/Makefile
> > @@ -17,7 +17,8 @@ define Package/$(PKG_NAME)
> >  			 +@FASTD_ENABLE_CIPHER_NULL \
> >  			 +@FASTD_WITH_STATUS_SOCKET \
> >  			 +fastd \
> > -			 +fff-random
> > +			 +fff-random \
> > +			 +fff-config
> >  endef
> >
> >  define Package/$(PKG_NAME)/description
> > diff --git a/src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd
> b/src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd
> > index d53eb43..08ceecb 100644
> > --- a/src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd
> > +++ b/src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd
> > @@ -15,9 +15,17 @@ uci batch <<EOF
> >    set fastd.fff.mtu='1426'
> >    set fastd.fff.on_up="/etc/fastd/fff/up.sh"
> >    set fastd.fff.secure_handshakes='0'
> > -  set fastd.fff.secret="generate"
> >  EOF
> >
> > +if ! secret=$(uci -q get fff.fastd.secret); then
> > +	secret=$(/usr/bin/fastd --generate-key --machine-readable)
> > +	uci set fff.fastd='fff'
> > +	uci set fff.fastd.secret="$secret"
> > +	uci commit fff
> > +fi
> > +uci set fastd.fff.secret="$secret"
> > +uci commit fastd
> > +
> >  [ ! -d /etc/fastd/fff ] &&  mkdir -p /etc/fastd/fff
> >  ln -s /tmp/fastd_fff_peers /etc/fastd/fff/peers
> >  echo "#!/bin/sh" > /etc/fastd/fff/up.sh

Ich hatte das Ding gerade schon fast gemerged, aber jetzt bin ich wieder unsicher.

Im Prinzip gehört dieser Key nicht nach /etc/config/fff, da dort eine User-Config abgelegt werden soll.
Neben dem rein pedantischen (was nicht so wichtig wäre) macht dies den Key nebenbei auch viel sichtbarer für alle Nutzer.

Was passiert, wenn dort jemand den gleichen Key für mehrere Geräte einträgt?
Was passiert, wenn dort jemand einfach "brumm" einträgt?

Beide Probleme bestehen natürlich genauso in der /etc/config/fastd, aber dort ist das Ganze wesentlich weniger sichtbar.
Alternativ zu diesem Patch könnte man auch einfach die /etc/config/fastd upgradesicher machen, aber das hat dann wieder andere Nachteile.

Ich will jetzt den Patch nicht kaputtreden, aber ich fände es schön, wenn wir nochmal kurz darüber nachdenken, was die obigen Aspekte bedeuten.

Ein paar Kleinigkeiten noch unten, braucht aber keine v5, habe ich jetzt eh schon alles selbst gemacht.
 
> > Am 10.01.20 um 12:57 schrieb Christian Dresel:
> > > To use a whitelist easy, it is neccessary to make the fastd key
> updatesafe
> > > This patch safe the key to uci fff and recover it, if a key is
> after the update
> > available
> > >
> > > ---
> > > Changes in v2:
> > > - use variable in if
> > > - remove trailing whitespace
> > > - remove -q
> > > ---
> > > Changes in v3:
> > > - use only one variable $secret
> > > ---
> > > Changes in v4:
> > > - remove new line
> > > - add dependencies to fff-config
> > > ---
> > >
> > > Signed-off-by: Christian Dresel <fff@chrisi01.de>
> > > Reviewed-by: lemmi <lemmi@nerd2nerd.org>

Das kommt nach dem Separator ("---") und wird daher mit abgeschnitten. Das nächste Mal darauf achten, dass die "Changes" und der Separator erst nach dem Signed-off kommen.

Des Weiteren habe ich keine Ahnung, wie wir es mit dem Klarnamenzwang halten. Für den Reviewed-by ist das jetzt völlig egal, aber man sollte sich die Frage vielleicht schon mal grundsätzlich stellen.

> > > ---
> > >  src/packages/fff/fff-fastd/Makefile                            |
> 3 ++-
> > >  src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd |
> 10 +++++++++-
> > >  2 files changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/src/packages/fff/fff-fastd/Makefile
> b/src/packages/fff/fff-
> > fastd/Makefile
> > > index 513775d..0d9a9b5 100644
> > > --- a/src/packages/fff/fff-fastd/Makefile
> > > +++ b/src/packages/fff/fff-fastd/Makefile

Hier habe ich noch den PKG_RELEASE bump ergänzt.

Grüße

Adrian

hi

On 10.01.20 14:42, Adrian Schmutzler wrote:
> Ich hatte das Ding gerade schon fast gemerged, aber jetzt bin ich wieder unsicher.
> 
> Im Prinzip gehört dieser Key nicht nach /etc/config/fff, da dort eine User-Config abgelegt werden soll.
> Neben dem rein pedantischen (was nicht so wichtig wäre) macht dies den Key nebenbei auch viel sichtbarer für alle Nutzer.
> 
> Was passiert, wenn dort jemand den gleichen Key für mehrere Geräte einträgt?
> Was passiert, wenn dort jemand einfach "brumm" einträgt?
> 
> Beide Probleme bestehen natürlich genauso in der /etc/config/fastd, aber dort ist das Ganze wesentlich weniger sichtbar.

wie du selbst sagst, wenn jemand bewusst Käse machen will kann er das
auch in der /etc/config/fastd.

Gleicher key ist vermutlich schlecht in der gleichen Hood. Scheint dann
wohl folgendes einzutreten:
https://forum.freifunk.net/t/fastd-duplicate-key-check-ausschalten/13937

Wenn jemand "brumm" einträgt... keine Ahnung? Vllt. startet fastd nicht
(davon gehe ich mal aus), vllt. kommt das Gateway nicht mit dem Key
klar. Außer das der Router vermutlich nicht geht, erwarte ich eigentlich
nicht das da was kaputt geht.

Ich persönlich sehe hier kein Problem, damit macht sich max. der User
den Router selbst unbrauchbar und dann ist er selbst Schuld.

> Alternativ zu diesem Patch könnte man auch einfach die /etc/config/fastd upgradesicher machen, aber das hat dann wieder andere Nachteile.

Ich glaube nicht, dass das gut ist. Hab ich auch überlegt aber wenn sich
am fastd irgendwas ändert müssen wir hier wieder mit Migrationssscripte
anfangen. Gefällt mir nicht.

Gruß

Christian

> 
> Ich will jetzt den Patch nicht kaputtreden, aber ich fände es schön, wenn wir nochmal kurz darüber nachdenken, was die obigen Aspekte bedeuten.
> 
> Ein paar Kleinigkeiten noch unten, braucht aber keine v5, habe ich jetzt eh schon alles selbst gemacht.
>  
>>> Am 10.01.20 um 12:57 schrieb Christian Dresel:
>>>> To use a whitelist easy, it is neccessary to make the fastd key
>> updatesafe
>>>> This patch safe the key to uci fff and recover it, if a key is
>> after the update
>>> available
>>>>
>>>> ---
>>>> Changes in v2:
>>>> - use variable in if
>>>> - remove trailing whitespace
>>>> - remove -q
>>>> ---
>>>> Changes in v3:
>>>> - use only one variable $secret
>>>> ---
>>>> Changes in v4:
>>>> - remove new line
>>>> - add dependencies to fff-config
>>>> ---
>>>>
>>>> Signed-off-by: Christian Dresel <fff@chrisi01.de>
>>>> Reviewed-by: lemmi <lemmi@nerd2nerd.org>
> 
> Das kommt nach dem Separator ("---") und wird daher mit abgeschnitten. Das nächste Mal darauf achten, dass die "Changes" und der Separator erst nach dem Signed-off kommen.
> 
> Des Weiteren habe ich keine Ahnung, wie wir es mit dem Klarnamenzwang halten. Für den Reviewed-by ist das jetzt völlig egal, aber man sollte sich die Frage vielleicht schon mal grundsätzlich stellen.
> 
>>>> ---
>>>>  src/packages/fff/fff-fastd/Makefile                            |
>> 3 ++-
>>>>  src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd |
>> 10 +++++++++-
>>>>  2 files changed, 11 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/src/packages/fff/fff-fastd/Makefile
>> b/src/packages/fff/fff-
>>> fastd/Makefile
>>>> index 513775d..0d9a9b5 100644
>>>> --- a/src/packages/fff/fff-fastd/Makefile
>>>> +++ b/src/packages/fff/fff-fastd/Makefile
> 
> Hier habe ich noch den PKG_RELEASE bump ergänzt.
> 
> Grüße
> 
> Adrian
>

Hallo Christian,

> -----Original Message-----
> From: Christian Dresel [mailto:fff@chrisi01.de]
> Sent: Freitag, 10. Januar 2020 14:52
> To: Adrian Schmutzler <mail@adrianschmutzler.de>; franken-dev@freifunk.net;
> 'robert' <rlanghammer@web.de>
> Subject: Re: [PATCH v4] fastd: make secret key updatesafe
> 
> hi
> 
> On 10.01.20 14:42, Adrian Schmutzler wrote:
> > Ich hatte das Ding gerade schon fast gemerged, aber jetzt bin ich
> wieder unsicher.
> >
> > Im Prinzip gehört dieser Key nicht nach /etc/config/fff, da dort eine
> User-Config abgelegt werden soll.
> > Neben dem rein pedantischen (was nicht so wichtig wäre) macht dies
> den Key nebenbei auch viel sichtbarer für alle Nutzer.
> 
> >
> > Was passiert, wenn dort jemand den gleichen Key für mehrere Geräte
> einträgt?
> > Was passiert, wenn dort jemand einfach "brumm" einträgt?
> >
> > Beide Probleme bestehen natürlich genauso in der /etc/config/fastd,
> aber dort ist das Ganze wesentlich weniger sichtbar.
> 
> wie du selbst sagst, wenn jemand bewusst Käse machen will kann er das
> auch in der /etc/config/fastd.
> 
> Gleicher key ist vermutlich schlecht in der gleichen Hood. Scheint dann
> wohl folgendes einzutreten:
> https://forum.freifunk.net/t/fastd-duplicate-key-check-ausschalten/1393
> 7
> 
> Wenn jemand "brumm" einträgt... keine Ahnung? Vllt. startet fastd nicht
> (davon gehe ich mal aus), vllt. kommt das Gateway nicht mit dem Key
> klar. Außer das der Router vermutlich nicht geht, erwarte ich
> eigentlich
> nicht das da was kaputt geht.
> 
> Ich persönlich sehe hier kein Problem, damit macht sich max. der User
> den Router selbst unbrauchbar und dann ist er selbst Schuld.
> 

Das ist die Antwort, die ich hören wollte. Wenn in beiden Fällen einfach keine fastd Verbindung zustandekommt (zumindest wahrscheinlicherweise), dann ist deine Lösung die beste, die mir im Moment einfällt.

Ich warte mal noch den Tag ab und merge das Ding dann heute abend, sofern nicht noch andere Meinungen kommen.

Grüße

Adrian

> > Alternativ zu diesem Patch könnte man auch einfach die
> /etc/config/fastd upgradesicher machen, aber das hat dann wieder andere
> Nachteile.
> 
> Ich glaube nicht, dass das gut ist. Hab ich auch überlegt aber wenn
> sich
> am fastd irgendwas ändert müssen wir hier wieder mit Migrationssscripte
> anfangen. Gefällt mir nicht.
> 
> Gruß
> 
> Christian
> 
> >
> > Ich will jetzt den Patch nicht kaputtreden, aber ich fände es schön,
> wenn wir nochmal kurz darüber nachdenken, was die obigen Aspekte
> bedeuten.
> 
> >
> > Ein paar Kleinigkeiten noch unten, braucht aber keine v5, habe ich
> jetzt eh schon alles selbst gemacht.
> >
> >>> Am 10.01.20 um 12:57 schrieb Christian Dresel:
> >>>> To use a whitelist easy, it is neccessary to make the fastd key
> >> updatesafe
> >>>> This patch safe the key to uci fff and recover it, if a key is
> >> after the update
> >>> available
> >>>>
> >>>> ---
> >>>> Changes in v2:
> >>>> - use variable in if
> >>>> - remove trailing whitespace
> >>>> - remove -q
> >>>> ---
> >>>> Changes in v3:
> >>>> - use only one variable $secret
> >>>> ---
> >>>> Changes in v4:
> >>>> - remove new line
> >>>> - add dependencies to fff-config
> >>>> ---
> >>>>
> >>>> Signed-off-by: Christian Dresel <fff@chrisi01.de>
> >>>> Reviewed-by: lemmi <lemmi@nerd2nerd.org>
> >
> > Das kommt nach dem Separator ("---") und wird daher mit
> abgeschnitten. Das nächste Mal darauf achten, dass die "Changes" und der
> Separator erst nach dem Signed-off kommen.
> 
> >
> > Des Weiteren habe ich keine Ahnung, wie wir es mit dem Klarnamenzwang
> halten. Für den Reviewed-by ist das jetzt völlig egal, aber man sollte
> sich die Frage vielleicht schon mal grundsätzlich stellen.
> 
> >
> >>>> ---
> >>>>  src/packages/fff/fff-fastd/Makefile                            |
> >> 3 ++-
> >>>>  src/packages/fff/fff-fastd/files/etc/uci-defaults/55_fff-fastd |
> >> 10 +++++++++-
> >>>>  2 files changed, 11 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/src/packages/fff/fff-fastd/Makefile
> >> b/src/packages/fff/fff-
> >>> fastd/Makefile
> >>>> index 513775d..0d9a9b5 100644
> >>>> --- a/src/packages/fff/fff-fastd/Makefile
> >>>> +++ b/src/packages/fff/fff-fastd/Makefile
> >
> > Hier habe ich noch den PKG_RELEASE bump ergänzt.
> >
> > Grüße
> >
> > Adrian
> >
> 
>

Applied.