diff mbox series

[v5,02/16] swupdate.c: Abort if no key for encrypted sw-description is provided

Message ID 20250904115704.58413-3-Michael.Glembotzki@iris-sensing.com
State Changes Requested
Delegated to: Stefano Babic
Headers show
Series Add support for asymmetric decryption | expand

Commit Message

Michael Glembotzki Sept. 4, 2025, 11:49 a.m. UTC
Signed-off-by: Michael Glembotzki <Michael.Glembotzki@iris-sensing.com>
---
 core/swupdate.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Stefano Babic Sept. 8, 2025, 9:50 a.m. UTC | #1
Hi Michael,

On 9/4/25 13:49, Michael Glembotzki wrote:
> Signed-off-by: Michael Glembotzki <Michael.Glembotzki@iris-sensing.com>

This looks apparently corrent, but it breaks a Use Case. In fact, the 
error should be raised if SWUpdate has no key when it starts to decrypt.

The USe Case is that no key is passed to SWUpdate via command line, 
because there is an agent doing this that transfer the key via IPC (see 
also swupdate_set_aes()). This is true also if sw-description is 
encrypted, because the key is just required when an update is started,.

Best regards,
Stefano


> ---
>   core/swupdate.c | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/core/swupdate.c b/core/swupdate.c
> index eca29846..4f013945 100644
> --- a/core/swupdate.c
> +++ b/core/swupdate.c
> @@ -1057,7 +1057,13 @@ int main(int argc, char **argv)
>   	 * If an AES key is passed, load it to allow
>   	 * to decrypt images
>   	 */
> -	if (strlen(swcfg.aeskeyfname)) {
> +	if (!strlen(swcfg.aeskeyfname)) {
> +#ifdef CONFIG_ENCRYPTED_SW_DESCRIPTION
> +		fprintf(stderr,
> +			"Error: Encrypted sw-description expected, but no key provided.\n");
> +		exit(EXIT_FAILURE);
> +#endif
> +	} else {
>   		if (load_decryption_key(swcfg.aeskeyfname)) {
>   			fprintf(stderr,
>   				"Error: Key file does not contain a valid AES key.\n");
Michael Glembotzki Sept. 8, 2025, 11:18 a.m. UTC | #2
Hi Stefano,

Stefano Babic schrieb am Montag, 8. September 2025 um 11:50:24 UTC+2:

Hi Michael, 

On 9/4/25 13:49, Michael Glembotzki wrote: 
> Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com> 

This looks apparently corrent, but it breaks a Use Case. In fact, the 
error should be raised if SWUpdate has no key when it starts to decrypt. 

The USe Case is that no key is passed to SWUpdate via command line, 
because there is an agent doing this that transfer the key via IPC (see 
also swupdate_set_aes()). This is true also if sw-description is 
encrypted, because the key is just required when an update is started,. 

Best regards, 
Stefano


Ah, okay, I see. Should we print it as a warning without exit(),
or would you rather skip the check?
 
Best regards,
Michael


> --- 
> core/swupdate.c | 8 +++++++- 
> 1 file changed, 7 insertions(+), 1 deletion(-) 
> 
> diff --git a/core/swupdate.c b/core/swupdate.c 
> index eca29846..4f013945 100644 
> --- a/core/swupdate.c 
> +++ b/core/swupdate.c 
> @@ -1057,7 +1057,13 @@ int main(int argc, char **argv) 
> * If an AES key is passed, load it to allow 
> * to decrypt images 
> */ 
> - if (strlen(swcfg.aeskeyfname)) { 
> + if (!strlen(swcfg.aeskeyfname)) { 
> +#ifdef CONFIG_ENCRYPTED_SW_DESCRIPTION 
> + fprintf(stderr, 
> + "Error: Encrypted sw-description expected, but no key provided.\n"); 
> + exit(EXIT_FAILURE); 
> +#endif 
> + } else { 
> if (load_decryption_key(swcfg.aeskeyfname)) { 
> fprintf(stderr, 
> "Error: Key file does not contain a valid AES key.\n");
Michael Glembotzki Sept. 14, 2025, 9:12 a.m. UTC | #3
Hi Stefano,

Michael Glembotzki schrieb am Montag, 8. September 2025 um 13:18:50 UTC+2:

Hi Stefano,

Stefano Babic schrieb am Montag, 8. September 2025 um 11:50:24 UTC+2:

Hi Michael, 

On 9/4/25 13:49, Michael Glembotzki wrote: 
> Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com> 

This looks apparently corrent, but it breaks a Use Case. In fact, the 
error should be raised if SWUpdate has no key when it starts to decrypt. 

The USe Case is that no key is passed to SWUpdate via command line, 
because there is an agent doing this that transfer the key via IPC (see 
also swupdate_set_aes()). This is true also if sw-description is 
encrypted, because the key is just required when an update is started,. 

Best regards, 
Stefano


Ah, okay, I see. Should we print it as a warning without exit(),
or would you rather skip the check?


A quick ping. Perhaps the follow-up question got lost.

And then a question about the further review. Would you prefer
to send a v6 with the comments so far or shall I wait for the
finished review?
 
Best regards,
Michael

Best regards,
Michael


> --- 
> core/swupdate.c | 8 +++++++- 
> 1 file changed, 7 insertions(+), 1 deletion(-) 
> 
> diff --git a/core/swupdate.c b/core/swupdate.c 
> index eca29846..4f013945 100644 
> --- a/core/swupdate.c 
> +++ b/core/swupdate.c 
> @@ -1057,7 +1057,13 @@ int main(int argc, char **argv) 
> * If an AES key is passed, load it to allow 
> * to decrypt images 
> */ 
> - if (strlen(swcfg.aeskeyfname)) { 
> + if (!strlen(swcfg.aeskeyfname)) { 
> +#ifdef CONFIG_ENCRYPTED_SW_DESCRIPTION 
> + fprintf(stderr, 
> + "Error: Encrypted sw-description expected, but no key provided.\n"); 
> + exit(EXIT_FAILURE); 
> +#endif 
> + } else { 
> if (load_decryption_key(swcfg.aeskeyfname)) { 
> fprintf(stderr, 
> "Error: Key file does not contain a valid AES key.\n");
Stefano Babic Sept. 14, 2025, 9:22 a.m. UTC | #4
Hi Michael,

On 14.09.25 11:12, Michael Glembotzki wrote:
> Hi Stefano,
> 
> Michael Glembotzki schrieb am Montag, 8. September 2025 um 13:18:50 UTC+2:
> 
>     Hi Stefano,
> 
>     Stefano Babic schrieb am Montag, 8. September 2025 um 11:50:24 UTC+2:
> 
>         Hi Michael,
> 
>         On 9/4/25 13:49, Michael Glembotzki wrote:
>          > Signed-off-by: Michael Glembotzki <Michael.G...@iris-
>         sensing.com>
> 
>         This looks apparently corrent, but it breaks a Use Case. In
>         fact, the
>         error should be raised if SWUpdate has no key when it starts to
>         decrypt.
> 
>         The USe Case is that no key is passed to SWUpdate via command line,
>         because there is an agent doing this that transfer the key via
>         IPC (see
>         also swupdate_set_aes()). This is true also if sw-description is
>         encrypted, because the key is just required when an update is
>         started,.
> 
>         Best regards,
>         Stefano
> 
> 
>     Ah, okay, I see. Should we print it as a warning without exit(),
>     or would you rather skip the check?

Skip the check.

> 
> 
> A quick ping. Perhaps the follow-up question got lost.
> 
> And then a question about the further review. Would you prefer
> to send a v6 with the comments so far or shall I wait for the
> finished review?

I am going to review the rest today, then just send a V6.

Regards,
Stefano

> Best regards,
> Michael
> 
>     Best regards,
>     Michael
> 
> 
>          > ---
>          > core/swupdate.c | 8 +++++++-
>          > 1 file changed, 7 insertions(+), 1 deletion(-)
>          >
>          > diff --git a/core/swupdate.c b/core/swupdate.c
>          > index eca29846..4f013945 100644
>          > --- a/core/swupdate.c
>          > +++ b/core/swupdate.c
>          > @@ -1057,7 +1057,13 @@ int main(int argc, char **argv)
>          > * If an AES key is passed, load it to allow
>          > * to decrypt images
>          > */
>          > - if (strlen(swcfg.aeskeyfname)) {
>          > + if (!strlen(swcfg.aeskeyfname)) {
>          > +#ifdef CONFIG_ENCRYPTED_SW_DESCRIPTION
>          > + fprintf(stderr,
>          > + "Error: Encrypted sw-description expected, but no key
>         provided.\n");
>          > + exit(EXIT_FAILURE);
>          > +#endif
>          > + } else {
>          > if (load_decryption_key(swcfg.aeskeyfname)) {
>          > fprintf(stderr,
>          > "Error: Key file does not contain a valid AES key.\n");
> 
> -- 
> You received this message because you are subscribed to the Google 
> Groups "swupdate" group.
> To unsubscribe from this group and stop receiving emails from it, send 
> an email to swupdate+unsubscribe@googlegroups.com 
> <mailto:swupdate+unsubscribe@googlegroups.com>.
> To view this discussion visit https://groups.google.com/d/msgid/ 
> swupdate/5cb0008c-5c85-4948-b967-36c13b85edcbn%40googlegroups.com 
> <https://groups.google.com/d/msgid/swupdate/5cb0008c-5c85-4948- 
> b967-36c13b85edcbn%40googlegroups.com?utm_medium=email&utm_source=footer>.
diff mbox series

Patch

diff --git a/core/swupdate.c b/core/swupdate.c
index eca29846..4f013945 100644
--- a/core/swupdate.c
+++ b/core/swupdate.c
@@ -1057,7 +1057,13 @@  int main(int argc, char **argv)
 	 * If an AES key is passed, load it to allow
 	 * to decrypt images
 	 */
-	if (strlen(swcfg.aeskeyfname)) {
+	if (!strlen(swcfg.aeskeyfname)) {
+#ifdef CONFIG_ENCRYPTED_SW_DESCRIPTION
+		fprintf(stderr,
+			"Error: Encrypted sw-description expected, but no key provided.\n");
+		exit(EXIT_FAILURE);
+#endif
+	} else {
 		if (load_decryption_key(swcfg.aeskeyfname)) {
 			fprintf(stderr,
 				"Error: Key file does not contain a valid AES key.\n");