[ptxdist] [RFC PATCH] rc-once: openssh: Do not overwrite existing keys

[ptxdist] [RFC PATCH] rc-once: openssh: Do not overwrite existing keys

[Alexander Dahl]

When storing your keys not in rootfs but on a separate data partition
(using symbolic links or overlay fs), keys are overwritten on each
firmware upgrade which lets rc-once run again (which happens when using
opkg upgrade/update or RAUC in an A/B scheme for example).

Changing keys are at best annoying, but may be interpreted as an attack
as well.

For dropbear the same behaviour was implemented with ac97e77eedf7
("[dropbear] rc.once: only generate keys if they aren't present yet").

Signed-off-by: Alexander Dahl <ada@thorsis.com>
---
 projectroot/etc/rc.once.d/openssh | 1 +
 1 file changed, 1 insertion(+)

diff --git a/projectroot/etc/rc.once.d/openssh b/projectroot/etc/rc.once.d/openssh
index 545586f07..595e28477 100644
--- a/projectroot/etc/rc.once.d/openssh
+++ b/projectroot/etc/rc.once.d/openssh
@@ -27,6 +27,7 @@ create_keys() {
 	hostkeys="$(get_hostkeys)" || return
 
 	for keyfile in $hostkeys; do
+		[ -e "$keyfile" ] && continue
 		create_key "$keyfile" || return
 	done
 }

base-commit: 51994d1b518323d2975491090a2452d34b1a39f9
-- 
2.30.2


_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de

Re: [ptxdist] [RFC PATCH] rc-once: openssh: Do not overwrite existing keys

[Michael Olbrich]

On Mon, Oct 11, 2021 at 02:54:01PM +0200, Alexander Dahl wrote:
> When storing your keys not in rootfs but on a separate data partition
> (using symbolic links or overlay fs), keys are overwritten on each
> firmware upgrade which lets rc-once run again (which happens when using
> opkg upgrade/update or RAUC in an A/B scheme for example).
> 
> Changing keys are at best annoying, but may be interpreted as an attack
> as well.

This has come up before (I'm not sure if it was on this list or some other
channel). I'm not quite certain how to handle this.

Someone may depend on the current behavior. I think it's rather unlikely so
I'll probably ignore that but we should keep it in mind.

I'm more concerned with broken keys caused by power failures or things like
that while the keys are created. So maybe a better check than just file
existence?

> For dropbear the same behaviour was implemented with ac97e77eedf7
> ("[dropbear] rc.once: only generate keys if they aren't present yet").

Marc applied that patch. I'm probably a bit more pedantic about stuff like
that :-).

> Signed-off-by: Alexander Dahl <ada@thorsis.com>
> ---
>  projectroot/etc/rc.once.d/openssh | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/projectroot/etc/rc.once.d/openssh b/projectroot/etc/rc.once.d/openssh
> index 545586f07..595e28477 100644
> --- a/projectroot/etc/rc.once.d/openssh
> +++ b/projectroot/etc/rc.once.d/openssh
> @@ -27,6 +27,7 @@ create_keys() {
>  	hostkeys="$(get_hostkeys)" || return
>  
>  	for keyfile in $hostkeys; do
> +		[ -e "$keyfile" ] && continue

Maybe:

		[ -s "$keyfile" ] && ssh-keygen -l -f "$keyfile.pub" > /dev/null && continue

A non-empty private key file and a probably valid public key should be
sufficient to prevent issues with power failures, I think.

Michael

>  		create_key "$keyfile" || return
>  	done
>  }
> 
> base-commit: 51994d1b518323d2975491090a2452d34b1a39f9
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> ptxdist mailing list
> ptxdist@pengutronix.de
> To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de

Re: [ptxdist] [RFC PATCH] rc-once: openssh: Do not overwrite existing keys

[Alexander Dahl]

Hello Michael,

Am Freitag, 15. Oktober 2021, 14:52:45 CEST schrieb Michael Olbrich:
> On Mon, Oct 11, 2021 at 02:54:01PM +0200, Alexander Dahl wrote:
> > When storing your keys not in rootfs but on a separate data partition
> > (using symbolic links or overlay fs), keys are overwritten on each
> > firmware upgrade which lets rc-once run again (which happens when using
> > opkg upgrade/update or RAUC in an A/B scheme for example).
> > 
> > Changing keys are at best annoying, but may be interpreted as an attack
> > as well.
> 
> This has come up before (I'm not sure if it was on this list or some other
> channel). I'm not quite certain how to handle this.
> 
> Someone may depend on the current behavior. I think it's rather unlikely so
> I'll probably ignore that but we should keep it in mind.

Yes, I thought about that. That's why I wanted to discuss it first.

> I'm more concerned with broken keys caused by power failures or things like
> that while the keys are created. So maybe a better check than just file
> existence?
> 
> > For dropbear the same behaviour was implemented with ac97e77eedf7
> > ("[dropbear] rc.once: only generate keys if they aren't present yet").
> 
> Marc applied that patch. I'm probably a bit more pedantic about stuff like
> that :-).
> 
> > Signed-off-by: Alexander Dahl <ada@thorsis.com>
> > ---
> > 
> >  projectroot/etc/rc.once.d/openssh | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/projectroot/etc/rc.once.d/openssh
> > b/projectroot/etc/rc.once.d/openssh index 545586f07..595e28477 100644
> > --- a/projectroot/etc/rc.once.d/openssh
> > +++ b/projectroot/etc/rc.once.d/openssh
> > @@ -27,6 +27,7 @@ create_keys() {
> > 
> >  	hostkeys="$(get_hostkeys)" || return
> >  	
> >  	for keyfile in $hostkeys; do
> > 
> > +		[ -e "$keyfile" ] && continue
> 
> Maybe:
> 
> 		[ -s "$keyfile" ] && ssh-keygen -l -f "$keyfile.pub" > /dev/null &&
> continue
> 
> A non-empty private key file and a probably valid public key should be
> sufficient to prevent issues with power failures, I think.

It's a good idea to have a better check than just for file existence!

We already had that empty file problem in the past. That would also be nice to 
have for the dropbear package. 

Not sure yet about that public key check, but I'll have a look into it.

Thanks for your feedback, I'll send a v2 with improved checks later.

Greets
Alex

> 
> Michael
> 
> >  		create_key "$keyfile" || return
> >  	
> >  	done
> >  
> >  }
> > 
> > base-commit: 51994d1b518323d2975491090a2452d34b1a39f9


-- 




_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de

Re: [ptxdist] [RFC PATCH] rc-once: openssh: Do not overwrite existing keys

[Denis Osterland-Heim]

Hi,

I think a check if the the file exists would be sufficient,
if the key is generated to a temporary file first.

--- a/projectroot/etc/rc.once.d/openssh
+++ b/projectroot/etc/rc.once.d/openssh
@@ -19,7 +19,11 @@ create_key() {

 echo "Create $keytype key; this may take some time ..."
 rm -f "$keyfile" &&
-ssh-keygen -q -f "$keyfile" -N '' -t "$keytype" $keygen_args || return
+ssh-keygen -q -f "$keyfile.tmp" -N '' -t "$keytype" $keygen_args || return
+sync
+mv "$keyfile.tmp" "$keyfile"
+mv "$keyfile.tmp.pub" "$keyfile.pub"
+sync
 echo "Created $keytype key."
 }

Regards Denis

Am Freitag, den 15.10.2021, 15:22 +0200 schrieb Alexander Dahl:
> Hello Michael,
>
> Am Freitag, 15. Oktober 2021, 14:52:45 CEST schrieb Michael Olbrich:
> > On Mon, Oct 11, 2021 at 02:54:01PM +0200, Alexander Dahl wrote:
> > > When storing your keys not in rootfs but on a separate data partition
> > > (using symbolic links or overlay fs), keys are overwritten on each
> > > firmware upgrade which lets rc-once run again (which happens when using
> > > opkg upgrade/update or RAUC in an A/B scheme for example).
> > >
> > > Changing keys are at best annoying, but may be interpreted as an attack
> > > as well.
> >
> > This has come up before (I'm not sure if it was on this list or some other
> > channel). I'm not quite certain how to handle this.
> >
> > Someone may depend on the current behavior. I think it's rather unlikely so
> > I'll probably ignore that but we should keep it in mind.
>
> Yes, I thought about that. That's why I wanted to discuss it first.
>
> > I'm more concerned with broken keys caused by power failures or things like
> > that while the keys are created. So maybe a better check than just file
> > existence?
> >
> > > For dropbear the same behaviour was implemented with ac97e77eedf7
> > > ("[dropbear] rc.once: only generate keys if they aren't present yet").
> >
> > Marc applied that patch. I'm probably a bit more pedantic about stuff like
> > that :-).
> >
> > > Signed-off-by: Alexander Dahl <ada@thorsis.com>
> > > ---
> > >
> > >  projectroot/etc/rc.once.d/openssh | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/projectroot/etc/rc.once.d/openssh
> > > b/projectroot/etc/rc.once.d/openssh index 545586f07..595e28477 100644
> > > --- a/projectroot/etc/rc.once.d/openssh
> > > +++ b/projectroot/etc/rc.once.d/openssh
> > > @@ -27,6 +27,7 @@ create_keys() {
> > >
> > >  hostkeys="$(get_hostkeys)" || return
> > >
> > >  for keyfile in $hostkeys; do
> > >
> > > +[ -e "$keyfile" ] && continue
> >
> > Maybe:
> >
> > [ -s "$keyfile" ] && ssh-keygen -l -f "$keyfile.pub" > /dev/null &&
> > continue
> >
> > A non-empty private key file and a probably valid public key should be
> > sufficient to prevent issues with power failures, I think.
>
> It's a good idea to have a better check than just for file existence!
>
> We already had that empty file problem in the past. That would also be nice to
> have for the dropbear package.
>
> Not sure yet about that public key check, but I'll have a look into it.
>
> Thanks for your feedback, I'll send a v2 with improved checks later.
>
> Greets
> Alex
>
> >
> > Michael
> >
> > >  create_key "$keyfile" || return
> > >
> > >  done
> > >
> > >  }
> > >
> > > base-commit: 51994d1b518323d2975491090a2452d34b1a39f9
>
>
Diehl Connectivity Solutions GmbH
Geschäftsführung: Horst Leonberger
Sitz der Gesellschaft: Nürnberg - Registergericht: Amtsgericht
Nürnberg: HRB 32315

________________________________

Der Inhalt der vorstehenden E-Mail ist nicht rechtlich bindend. Diese E-Mail enthaelt vertrauliche und/oder rechtlich geschuetzte Informationen.
Informieren Sie uns bitte, wenn Sie diese E-Mail faelschlicherweise erhalten haben. Bitte loeschen Sie in diesem Fall die Nachricht.
Jede unerlaubte Form der Reproduktion, Bekanntgabe, Aenderung, Verteilung und/oder Publikation dieser E-Mail ist strengstens untersagt.

- Informationen zum Datenschutz, insbesondere zu Ihren Rechten, erhalten Sie unter:

https://www.diehl.com/group/de/transparenz-und-informationspflichten/

The contents of the above mentioned e-mail is not legally binding. This e-mail contains confidential and/or legally protected information. Please inform us if you have received this e-mail by
mistake and delete it in such a case. Each unauthorized reproduction, disclosure, alteration, distribution and/or publication of this e-mail is strictly prohibited.

- For general information on data protection and your respective rights please visit:

https://www.diehl.com/group/en/transparency-and-information-obligations/


_______________________________________________
ptxdist mailing list
ptxdist@pengutronix.de
To unsubscribe, send a mail with subject "unsubscribe" to ptxdist-request@pengutronix.de