[PATCH] FritzBox-4040-UBOOT: Allow for easier devices recovery

[Enrico Mioso]

Hello all!!
thanks for taking a look at this patch!


On Tue, 23 Nov 2021, David Bauer wrote:

> Date: Tue, 23 Nov 2021 12:20:08
> From: David Bauer <mail at david-bauer.net>
> To: Enrico Mioso <mrkiko.rs at gmail.com>
> Cc: Christian Lamparter <chunkeey at gmail.com>,
>     OpenWrt Development List <openwrt-devel at lists.openwrt.org>
> Subject: Re: [PATCH] FritzBox-4040-UBOOT: Allow for easier devices recovery
> 
> Hello Enrico,
>
> On 11/22/21 11:55, Enrico Mioso wrote:
>> When flashing a broken kernel, or an image where failsafe mode is no more 
>> accessible, recoverying these devices can become needlessly painful.
>> Allow for easier recovery by unconditionally trying to get an initramfs 
>> image over TFTP once before booting, thereby giving the user a chance to 
>> sysupgrade to a working image.
>
> As I've already explained, I don't like increasing the time necessary for the 
> device to boot.

I think there are some balances to be made here. Booting Windows doesn't take less time, if access to a Windows installation is at all possible. :)
> Also, introducig such a method on a 4040 does not make sense, as its NOR 
> flash can be rewritten
> from EVA.

I am open to change this patch, but I think the feature is really needed here.
>
> That being said, unconditionally requesting a bootable image over the network 
> is a security
> risk in itself. NAND based ipq40xx boards from AVM also only allow 
> connections to their
> bootloader on cold-boots for exactly this reason.

Good point. Still, implementing a push-button process would be a little bit complicated for me. Any help would be greatly apreciated in this regard.

>
> For example, if an attacker is able to create a kernel-panic, your patch 
> would enable him
> to modify the router in case he is on the same network. A Pushbutton TFTP 
> procedure mitigates
> this problem, as it depends on the attacker having physical access to the 
> device.
>
> Recovery is - for all boards - possible using the AVM recovery tool or 
> manually patching the
> U-Boot and sideloading via EVA. So a network request for a boot image raises 
> more problems than
> it tries to solve.

Yes, sideloading is definitely an option, for sure. Still, I think we are being too user unfriendly here for no good reason.
Maybe we can find a middleground here?
I don't think this patch is the ideal solution, but I think there should be an easier way to recover a device, especially when it depends on "our" code.
>
> Best
> David
>
>> 
>> Signed-off-by: Enrico Mioso <mrkiko.rs at gmail.com>
>> CC: Christian Lamparter <chunkeey at gmail.com>
>> CC: David Bauer <mail at david-bauer.net>
>> ---
>> 
>> Reasons for this patch:
>> 1 - There are situations where it can be nice to recover a device without 
>> the AVM Recovery tool. In some cases the tool won't even be an option (as 
>> far as I know, it exists only for Windows, or am I wrong?).
>> 2 - Since the effort of creating a second-stage bootloader for these 
>> devices has been carried out (thanks a lot for this!), I think it makes 
>> sense to allow for things to be more friendly to developers and users.
>> 
>> Side effects:
>> When nandboot fails, there will be TWO tftp requests with no delay between 
>> them, then the sleep will kick in.
>> 
>> Possible "improvements":
>> Implementing a push-button method may be preferred. Still, I have no easy 
>> way to attach an UART to the device right now.
>> Moreover, being able to do this "more" remotely would be a vaulable feature 
>> to me.
>> 
>> Enrico
>>
>>   include/configs/fritz1200.h | 2 +-
>>   include/configs/fritz3000.h | 2 +-
>>   include/configs/fritz4040.h | 2 +-
>>   include/configs/fritz7530.h | 2 +-
>>   4 files changed, 4 insertions(+), 4 deletions(-)
>> 
>> diff --git a/include/configs/fritz1200.h b/include/configs/fritz1200.h
>> index 90d5186..16152a3 100644
>> --- a/include/configs/fritz1200.h
>> +++ b/include/configs/fritz1200.h
>> @@ -23,7 +23,7 @@
>>   	"mtdparts=" MTDPARTS_DEFAULT "\0"			\
>>   	"nandboot=ubi part ubi && ubi read 0x85000000 kernel && bootm\0" 
>> \
>>   	"tftpboot=tftpboot && bootm; sleep 5; run tftpboot\0"	\
>> -	"fritzboot=run nandboot || run tftpboot;\0"		\
>> +	"fritzboot=tftpboot && bootm; run nandboot || run tftpboot;\0" 
>> \
>>     #undef V_PROMPT
>>   #define V_PROMPT		"(" CONFIG_MODEL ") # "
>> diff --git a/include/configs/fritz3000.h b/include/configs/fritz3000.h
>> index e383ffb..3440550 100644
>> --- a/include/configs/fritz3000.h
>> +++ b/include/configs/fritz3000.h
>> @@ -23,7 +23,7 @@
>>   	"mtdparts=" MTDPARTS_DEFAULT "\0"			\
>>   	"nandboot=ubi part ubi && ubi read 0x85000000 kernel && bootm\0" 
>> \
>>   	"tftpboot=tftpboot && bootm; sleep 5; run tftpboot\0"	\
>> -	"fritzboot=run nandboot || run tftpboot;\0"		\
>> +	"fritzboot=tftpboot && bootm; run nandboot || run tftpboot;\0" 
>> \
>>     #undef V_PROMPT
>>   #define V_PROMPT		"(" CONFIG_MODEL ") # "
>> diff --git a/include/configs/fritz4040.h b/include/configs/fritz4040.h
>> index 060afb0..582edfd 100644
>> --- a/include/configs/fritz4040.h
>> +++ b/include/configs/fritz4040.h
>> @@ -23,7 +23,7 @@
>>   	"mtdparts=" MTDPARTS_DEFAULT "\0"			\
>>   	"nandboot=nboot firmware && bootm\0"			\
>>   	"tftpboot=tftpsrv && bootm; sleep 5; run tftpboot\0"	\
>> -	"fritzboot=run nandboot || run tftpboot;\0"		\
>> +	"fritzboot=tftpboot && bootm; run nandboot || run tftpboot;\0" 
>> \
>>     #undef V_PROMPT
>>   #define V_PROMPT		"(" CONFIG_MODEL ") # "
>> diff --git a/include/configs/fritz7530.h b/include/configs/fritz7530.h
>> index b07ecfc..caecd5d 100644
>> --- a/include/configs/fritz7530.h
>> +++ b/include/configs/fritz7530.h
>> @@ -23,7 +23,7 @@
>>   	"mtdparts=" MTDPARTS_DEFAULT "\0"			\
>>   	"nandboot=ubi part ubi && ubi read 0x85000000 kernel && bootm\0" 
>> \
>>   	"tftpboot=tftpboot && bootm; sleep 5; run tftpboot\0"	\
>> -	"fritzboot=run nandboot || run tftpboot;\0"		\
>> +	"fritzboot=tftpboot && bootm; run nandboot || run tftpboot;\0" 
>> \
>>     #undef V_PROMPT
>>   #define V_PROMPT		"(" CONFIG_MODEL ") # "
>> 
>