[V4][PATCH 0/8] Add support for asymmetric decryption

[Michael Glembotzki]

Hi Stefano,

your requested changes have been incorporated. Kindly inform me of any
additional feedback you may have. While the patch series has undergone thorough
testing, I would welcome another tester for further assurance.

Thank you and best regards

Michael


Michael Glembotzki (8):
parser: BUG: Image IVT with invalid size is accepted
util: Add functions for set/get temporary AES key
parser: Read temporary AES key from sw-description
Add functions for asymmetric file decryption with CMS
swupdate: Initialize the key pair for asymmetric decryption
util: Replace bool with enum for 'encrypted' Parameter
Add support for asymmetrical encrypted images
doc: Add documentation for asymmetric decryption

Kconfig | 12 +++
core/cpio_utils.c | 69 +++++++++++++---
core/installer.c | 7 ++
core/stream_interface.c | 31 ++++---
core/swupdate.c | 35 ++++++++
core/util.c | 82 ++++++++++++++++---
corelib/Makefile | 3 +
corelib/swupdate_cms_decrypt.c | 115 ++++++++++++++++++++++++++
doc/source/asym_encrypted_images.rst | 153 +++++++++++++++++++++++++++++++++++
doc/source/encrypted_images.rst | 2 +
doc/source/index.rst | 1 +
doc/source/roadmap.rst | 5 --
doc/source/sw-description.rst | 13 ++-
examples/configuration/swupdate.cfg | 3 +
include/sslapi.h | 9 +++
include/swupdate.h | 1 +
include/util.h | 21 ++++-
parser/parser.c | 44 +++++++++-
18 files changed, 567 insertions(+), 39 deletions(-)

[Michael Glembotzki]

An IVT with invalid size is currently accepted. Make an explicit size check
before setting the image IVT.

Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com>
---
parser/parser.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/parser/parser.c b/parser/parser.c
index e13992e..67ae1b3 100644
--- a/parser/parser.c
+++ b/parser/parser.c
@@ -393,6 +393,22 @@ static int run_embscript(parsertype p, void *elem, struct img_type *img,
return lua_parser_fn(L, embfcn, img);
}

+static void get_ivt_value(parsertype p, void *elem, char *ivt_ascii)
+{
+ size_t ivtlen;
+ const char *s = NULL;
+
+ s = get_field_string(p, elem, "ivt");
+ if (s) {
+ ivtlen = strnlen(s, SWUPDATE_GENERAL_STRING_SIZE);
+ if (ivtlen != (AES_BLK_SIZE * 2)) {
+ ERROR("Invalid ivt length");
+ return;
+ }
+ strncpy(ivt_ascii, s, ivtlen);
+ }
+}
+
static int parse_common_attributes(parsertype p, void *elem, struct img_type *image, struct swupdate_cfg *cfg)
{
char seek_str[MAX_SEEK_STRING_SIZE];
@@ -451,7 +467,7 @@ static int parse_common_attributes(parsertype p, void *elem, struct img_type *im
get_field(p, elem, "install-if-different", &image->id.install_if_different);
get_field(p, elem, "install-if-higher", &image->id.install_if_higher);
get_field(p, elem, "encrypted", &image->is_encrypted);
- GET_FIELD_STRING(p, elem, "ivt", image->ivt_ascii);
+ get_ivt_value(p, elem, image->ivt_ascii);

if (is_image_installed(&cfg->installed_sw_list, image)) {
image->skip = SKIP_SAME;
--
2.43.0

[Michael Glembotzki]

Enhance functionality to allow temporary storage of an additional AES key,
complementing existing functions for setting default AES key.

Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com>
---

core/util.c | 82 ++++++++++++++++++++++++++++++++++++++++++++------
include/util.h | 11 ++++++-
2 files changed, 82 insertions(+), 11 deletions(-)

diff --git a/core/util.c b/core/util.c
index 99ed628..396d7d7 100644
--- a/core/util.c
+++ b/core/util.c
@@ -53,6 +53,10 @@ struct decryption_key {

static struct decryption_key *aes_key = NULL;

+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+static struct decryption_key *tmp_aes_key = NULL;
+#endif
+
/*
* Configuration file for fw_env.config
*/
@@ -537,6 +541,20 @@ bool is_hex_str(const char *ascii) {
return true;
}

+bool is_valid_aes_keylen(size_t keylen_ascii)
+{
+ switch (keylen_ascii) {
+ case AES_128_KEY_LEN * 2:
+ case AES_192_KEY_LEN * 2:
+ case AES_256_KEY_LEN * 2:
+ // valid hex string size for AES 128/192/256
+ return true;
+ default:
+ ERROR("Invalid AES key length");
+ return false;
+ }
+}
+
int set_aes_key(const char *key, const char *ivt)
{
int ret;
@@ -565,17 +583,12 @@ int set_aes_key(const char *key, const char *ivt)
strncpy(aes_key->key, key, keylen);
#else
keylen = strlen(key);
- switch (keylen) {
- case AES_128_KEY_LEN * 2:
- case AES_192_KEY_LEN * 2:
- case AES_256_KEY_LEN * 2:
- // valid hex string size for AES 128/192/256
- aes_key->keylen = keylen / 2;
- break;
- default:
- ERROR("Invalid aes_key length");
+
+ if (!is_valid_aes_keylen(keylen))
return -EINVAL;
- }
+
+ aes_key->keylen = keylen / 2;
+
ret |= !is_hex_str(key);
ret |= ascii_to_bin(aes_key->key, aes_key->keylen, key);
#endif
@@ -588,6 +601,55 @@ int set_aes_key(const char *key, const char *ivt)
return 0;
}

+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+int set_tmp_aes_key(const char *key_ascii)
+{
+ size_t keylen;
+
+ if (!tmp_aes_key) {
+ tmp_aes_key = (struct decryption_key *)calloc(1, sizeof(*tmp_aes_key));
+ if (!tmp_aes_key)
+ return -ENOMEM;
+ }
+
+ keylen = strlen(key_ascii);
+
+ if (!is_valid_aes_keylen(keylen))
+ return -EINVAL;
+
+ tmp_aes_key->keylen = keylen / 2;
+
+ if (!is_hex_str(key_ascii) || ascii_to_bin(tmp_aes_key->key, tmp_aes_key->keylen, key_ascii)) {
+ ERROR("Invalid tmp aes_key");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+unsigned char *get_tmp_aes_key(void)
+{
+ if (!tmp_aes_key)
+ return NULL;
+ return tmp_aes_key->key;
+}
+
+char get_tmp_aes_keylen(void)
+{
+ if (!tmp_aes_key)
+ return -1;
+ return tmp_aes_key->keylen;
+}
+
+void clear_tmp_aes_key(void)
+{
+ if (!tmp_aes_key)
+ return;
+ memset(tmp_aes_key->key, 0, sizeof(tmp_aes_key->key));
+ tmp_aes_key->keylen = 0;
+}
+#endif
+
const char *get_fwenv_config(void) {
if (!fwenv_config)
#if defined(CONFIG_UBOOT)
diff --git a/include/util.h b/include/util.h
index 062840f..f4a67ef 100644
--- a/include/util.h
+++ b/include/util.h
@@ -164,6 +164,7 @@ int ascii_to_bin(unsigned char *dest, size_t dstlen, const char *src);
void hash_to_ascii(const unsigned char *hash, char *s);
int IsValidHash(const unsigned char *hash);
bool is_hex_str(const char *ascii);
+bool is_valid_aes_keylen(size_t keylen_ascii);

#ifndef typeof
#define typeof __typeof__
@@ -237,13 +238,21 @@ bool check_same_file(int fd1, int fd2);
const char *get_fwenv_config(void);
void set_fwenv_config(const char *fname);

-/* Decryption key functions */
+/* Decryption key functions for the (default) aes-key */
int load_decryption_key(char *fname);
unsigned char *get_aes_key(void);
char get_aes_keylen(void);
unsigned char *get_aes_ivt(void);
int set_aes_key(const char *key, const char *ivt);

+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+/* Decryption key functions for the temporary aes-key read from the sw-description */
+unsigned char *get_tmp_aes_key(void);
+char get_tmp_aes_keylen(void);
+int set_tmp_aes_key(const char *key_ascii);
+void clear_tmp_aes_key(void);
+#endif
+
/* Getting global information */
int get_install_info(sourcetype *source, char *buf, size_t len);
void get_install_swset(char *buf, size_t len);
--
2.43.0

[Michael Glembotzki]

With CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION, a temporary AES key can be
provided with the sw-description file.

Make an explicit size check of the field string before setting the
temporary AES key. Only set the image AES key if a valid key length is
given.

Clear the temporary AES key after the update is done.

Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com>
---

core/stream_interface.c | 4 ++++
parser/parser.c | 26 ++++++++++++++++++++++++++
2 files changed, 30 insertions(+)

diff --git a/core/stream_interface.c b/core/stream_interface.c
index 0b78329..1cd148f 100644
--- a/core/stream_interface.c
+++ b/core/stream_interface.c
@@ -703,6 +703,10 @@ void *network_initializer(void *data)
/* release temp files we may have created */
cleanup_files(software);

+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ clear_tmp_aes_key();
+#endif
+
#ifndef CONFIG_NOCLEANUP
swupdate_remove_directory(SCRIPTS_DIR_SUFFIX);
swupdate_remove_directory(DATADST_DIR_SUFFIX);
diff --git a/parser/parser.c b/parser/parser.c
index 67ae1b3..70cc548 100644
--- a/parser/parser.c
+++ b/parser/parser.c
@@ -240,6 +240,32 @@ static bool get_common_fields(parsertype p, void *cfg, struct swupdate_cfg *swcf
TRACE("Namespaced used to store SWUpdate's vars: %s", swcfg->namespace_for_vars);
}

+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ /*
+ * Set sw-description aes-key, if present
+ */
+ if ((setting = find_node(p, cfg, "aes-key", swcfg)) != NULL) {
+ char aeskey_ascii[AES_256_KEY_LEN * 2 + 1] = {0};
+ size_t keylen;
+ const char *s = get_field_string(p, setting, NULL);
+
+ if (s) {
+ keylen = strnlen(s, SWUPDATE_GENERAL_STRING_SIZE);
+
+ if (!is_valid_aes_keylen(keylen))
+ return false;
+
+ strncpy(aeskey_ascii, s, keylen);
+ }
+ if (!s || !strlen(aeskey_ascii) || set_tmp_aes_key(aeskey_ascii)) {
+ ERROR("Provided aes-key in the sw-description file is invalid!");
+ return false;
+ }
+ } else {
+ TRACE("No AES key in the sw-description file.");
+ }
+#endif
+
return true;
}

--
2.43.0

[Michael Glembotzki]

Decryption with OpenSSL CMS is limited to entire files, preventing the
ability to decrypt data in chunks, as is possible with symmetric
decryption.

Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com>
---

corelib/Makefile | 3 +
corelib/swupdate_cms_decrypt.c | 115 +++++++++++++++++++++++++++++++++
include/sslapi.h | 9 +++
3 files changed, 127 insertions(+)
create mode 100644 corelib/swupdate_cms_decrypt.c

diff --git a/corelib/Makefile b/corelib/Makefile
index c9ca4aa..06690d8 100644
--- a/corelib/Makefile
+++ b/corelib/Makefile
@@ -18,6 +18,9 @@ endif
lib-$(CONFIG_SIGALG_RAWRSA) += swupdate_rsa_verify.o
lib-$(CONFIG_SIGALG_RSAPSS) += swupdate_rsa_verify.o
endif
+ifeq ($(CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION),y)
+lib-$(CONFIG_ENCRYPTED_IMAGES) += swupdate_cms_decrypt.o
+endif
ifeq ($(CONFIG_SSL_IMPL_OPENSSL),y)
lib-$(CONFIG_SIGALG_CMS) += swupdate_cms_verify.o
endif
diff --git a/corelib/swupdate_cms_decrypt.c b/corelib/swupdate_cms_decrypt.c
new file mode 100644
index 0000000..45aa596
--- /dev/null
+++ b/corelib/swupdate_cms_decrypt.c
@@ -0,0 +1,115 @@
+/*
+ * (C) Copyright 2024
+ * Michael Glembotzki, iris-GmbH infrared & intelligent sensors, michael.g...@iris-sensing.com
+ *
+ * SPDX-License-Identifier: GPL-2.0-only
+ *
+ * Code mostly taken from openssl examples
+ */
+#include <sys/stat.h>
+#include "swupdate.h"
+#include "sslapi.h"
+#include "util.h"
+
+int swupdate_dgst_add_asym_keypair(struct swupdate_cfg *sw, const char *keypair_file)
+{
+ X509 *asym_decryption_cert = NULL;
+ EVP_PKEY *asym_decryption_key = NULL;
+ BIO *tbio = NULL;
+ struct swupdate_digest *dgst = sw->dgst;
+ int ret = 0;
+
+ if (!dgst) {
+ dgst = calloc(1, sizeof(*dgst));
+ if (!dgst) {
+ ret = 1;
+ goto err;
+ }
+ }
+
+ tbio = BIO_new_file(keypair_file, "r");
+
+ if (!tbio) {
+ ERROR("%s cannot be opened", keypair_file);
+ ret = 1;
+ goto err;
+ }
+
+ asym_decryption_cert = PEM_read_bio_X509(tbio, NULL, 0, NULL);
+ if (!asym_decryption_cert)
+ WARN("Decryption cert not found");
+
+ BIO_reset(tbio);
+
+ asym_decryption_key = PEM_read_bio_PrivateKey(tbio, NULL, 0, NULL);
+ BIO_free(tbio);
+ if (!asym_decryption_key) {
+ ERROR("Decryption key not found");
+ ret = 1;
+ goto err;
+ }
+
+ dgst->asym_decryption_cert = asym_decryption_cert;
+ dgst->asym_decryption_key = asym_decryption_key;
+
+ return ret;
+
+err:
+ if (dgst)
+ free(dgst);
+
+ return ret;
+}
+
+int swupdate_decrypt_file(struct swupdate_digest *dgst, const char *infile, const char *outfile)
+{
+ BIO *in = NULL, *out = NULL;
+ CMS_ContentInfo *cms = NULL;
+ int ret = 0;
+
+ if (!dgst || !infile || !outfile)
+ return 1;
+
+ /* Open CMS message to decrypt */
+ in = BIO_new_file(infile, "rb");
+ if (!in) {
+ ERROR("%s cannot be opened", infile);
+ ret = 1;
+ goto err;
+ }
+
+ /* Parse message */
+ cms = d2i_CMS_bio(in, NULL);
+ if (!cms) {
+ ERROR("%s cannot be parsed as DER-encoded CMS blob", infile);
+ ret = 1;
+ goto err;
+ }
+
+ out = BIO_new_file(outfile, "wb");
+ if (!out) {
+ ERROR("%s cannot be opened", outfile);
+ ret = 1;
+ goto err;
+ }
+
+ if (chmod(outfile, 0600)) {
+ ERROR("Setting file permissions");
+ ret = 1;
+ goto err;
+ }
+
+ /* Decrypt CMS message */
+ if (!CMS_decrypt(cms, dgst->asym_decryption_key, dgst->asym_decryption_cert, NULL, out, 0)) {
+ ERR_print_errors_fp(stderr);
+ ERROR("Decrypting %s failed", infile);
+ ret = 1;
+ goto err;
+ }
+
+err:
+ BIO_free(in);
+ BIO_free(out);
+ CMS_ContentInfo_free(cms);
+ return ret;
+}
diff --git a/include/sslapi.h b/include/sslapi.h
index 83efd9f..d27a23c 100644
--- a/include/sslapi.h
+++ b/include/sslapi.h
@@ -113,6 +113,10 @@ struct swupdate_digest {
int verbose;
char *gpgme_protocol;
#endif
+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ EVP_PKEY *asym_decryption_key;
+ X509 *asym_decryption_cert;
+#endif
};

#if OPENSSL_VERSION_NUMBER < 0x10100000L
@@ -222,6 +226,11 @@ UNUSED static inline struct swupdate_digest *swupdate_DECRYPT_init(
#define swupdate_DECRYPT_cleanup(p)
#endif

+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+int swupdate_dgst_add_asym_keypair(struct swupdate_cfg *sw, const char *keypair_file);
+int swupdate_decrypt_file(struct swupdate_digest *dgst, const char *infile, const char *outfile);
+#endif
+
#ifndef SSL_PURPOSE_DEFAULT
#define SSL_PURPOSE_EMAIL_PROT -1
#define SSL_PURPOSE_CODE_SIGN -1
--
2.43.0

[Michael Glembotzki]

Add asymmetric decryption key pair fname to swupdate_cfg. Read and
initialize the asym decryption key pair from argument -a or configuration
file.

Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com>
---

core/swupdate.c | 35 +++++++++++++++++++++++++++++
examples/configuration/swupdate.cfg | 3 +++
include/swupdate.h | 1 +
3 files changed, 39 insertions(+)

diff --git a/core/swupdate.c b/core/swupdate.c
index 6f9938e..9c3f289 100644
--- a/core/swupdate.c
+++ b/core/swupdate.c
@@ -103,6 +103,9 @@ static struct option long_options[] = {
#endif
#ifdef CONFIG_ENCRYPTED_IMAGES
{"key-aes", required_argument, NULL, 'K'},
+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ {"asym-decryption-keypair", required_argument, NULL, 'a'},
+#endif
#endif
{"loglevel", required_argument, NULL, 'l'},
{"max-version", required_argument, NULL, '3'},
@@ -165,6 +168,10 @@ static void usage(char *programname)
#ifdef CONFIG_ENCRYPTED_IMAGES
" -K, --key-aes <key file> : the file contains the symmetric key to be used\n"
" to decrypt images\n"
+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ " -a, --asym-decryption-keypair\n"
+ " <key pair file> : path to the asym decryption key pair (PEM)\n"
+#endif
#endif
" -n, --dry-run : run SWUpdate without installing the software\n"
" -N, --no-downgrading <version> : not install a release older as <version>\n"
@@ -312,6 +319,10 @@ static int read_globals_settings(void *elem, void *data)
"ca-path", sw->publickeyfname);
GET_FIELD_STRING(LIBCFG_PARSER, elem,
"aes-key-file", sw->aeskeyfname);
+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ GET_FIELD_STRING(LIBCFG_PARSER, elem,
+ "asym-decryption-keypair", sw->asym_decryption_keypair_fname);
+#endif
GET_FIELD_STRING(LIBCFG_PARSER, elem,
"mtd-blacklist", sw->mtdblacklist);
GET_FIELD_STRING(LIBCFG_PARSER, elem,
@@ -499,6 +510,9 @@ int main(int argc, char **argv)
#endif
#ifdef CONFIG_ENCRYPTED_IMAGES
strcat(main_options, "K:");
+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ strcat(main_options, "a:");
+#endif
#endif

memset(fname, 0, sizeof(fname));
@@ -662,6 +676,13 @@ int main(int argc, char **argv)
optarg,
sizeof(swcfg.aeskeyfname));
break;
+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ case 'a':
+ strlcpy(swcfg.asym_decryption_keypair_fname,
+ optarg,
+ sizeof(swcfg.asym_decryption_keypair_fname));
+ break;
+#endif
#endif
case 'N':
swcfg.no_downgrading = true;
@@ -854,6 +875,20 @@ int main(int argc, char **argv)
}
}

+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ if (strlen(swcfg.asym_decryption_keypair_fname)) {
+ if (swupdate_dgst_add_asym_keypair(&swcfg, swcfg.asym_decryption_keypair_fname)) {
+ fprintf(stderr,
+ "Error: Asym decryption key pair cannot be initialized.\n");
+ exit(EXIT_FAILURE);
+ }
+ } else {
+ fprintf(stderr,
+ "Error: SWUpdate is built for asym encrypted images, provide a decryption key pair.\n");
+ exit(EXIT_FAILURE);
+ }
+#endif
+
lua_handlers_init();

if(!get_hw_revision(&swcfg.hw))
diff --git a/examples/configuration/swupdate.cfg b/examples/configuration/swupdate.cfg
index 8b8a6b1..844cdc5 100644
--- a/examples/configuration/swupdate.cfg
+++ b/examples/configuration/swupdate.cfg
@@ -25,6 +25,9 @@
# aes-key-file : string
# file containing the symmetric key for
# image decryption
+# asym-decryption-keypair : string
+# file containing the key pair (private key and cert) in PEM for
+# asymmetric image decryption
# preupdatecmd : string
# command to be executed right before the update
# is installed
diff --git a/include/swupdate.h b/include/swupdate.h
index c1f86b3..c54647e 100644
--- a/include/swupdate.h
+++ b/include/swupdate.h
@@ -57,6 +57,7 @@ struct swupdate_cfg {
char output[SWUPDATE_GENERAL_STRING_SIZE];
char publickeyfname[SWUPDATE_GENERAL_STRING_SIZE];
char aeskeyfname[SWUPDATE_GENERAL_STRING_SIZE];
+ char asym_decryption_keypair_fname[SWUPDATE_GENERAL_STRING_SIZE];
char postupdatecmd[SWUPDATE_GENERAL_STRING_SIZE];
char preupdatecmd[SWUPDATE_GENERAL_STRING_SIZE];
char minimum_version[SWUPDATE_GENERAL_STRING_SIZE];
--
2.43.0

[Michael Glembotzki]

Previously, artifacts were limited to symmetric encryption, requiring a
boolean. To enable __swupdate_copy for asymmetrically encrypted artifacts,
the boolean has been replaced with an enum.

Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com>
---

core/cpio_utils.c | 14 +++++++-------
core/stream_interface.c | 27 ++++++++++++++++++---------
include/util.h | 10 ++++++++--
3 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index 5b99904..03d43c9 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -431,7 +431,7 @@ static int zstd_step(void* state, void* buffer, size_t size)

static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nbytes, unsigned long *offs, unsigned long long seek,
int skip_file, int __attribute__ ((__unused__)) compressed,
- uint32_t *checksum, unsigned char *hash, bool encrypted, const char *imgivt, writeimage callback)
+ uint32_t *checksum, unsigned char *hash, encrypted_t encrypted, const char *imgivt, writeimage callback)
{
unsigned int percent, prevpercent = 0;
int ret = 0;
@@ -512,7 +512,7 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
return -EFAULT;
}

- if (encrypted) {
+ if (encrypted == SYMMETRIC) {
aes_key = get_aes_key();
if (imgivt) {
if (!strlen(imgivt) || !is_hex_str(imgivt) || ascii_to_bin(ivtbuf, sizeof(ivtbuf), imgivt)) {
@@ -587,7 +587,7 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby

#if defined(CONFIG_GUNZIP) || defined(CONFIG_ZSTD)
if (compressed) {
- if (encrypted) {
+ if (encrypted == SYMMETRIC) {
decrypt_state.upstream_step = &input_step;
decrypt_state.upstream_state = &input_state;
decompress_state.upstream_step = &decrypt_step;
@@ -600,7 +600,7 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
state = &decompress_state;
} else {
#endif
- if (encrypted) {
+ if (encrypted == SYMMETRIC) {
decrypt_state.upstream_step = &input_step;
decrypt_state.upstream_state = &input_state;
step = &decrypt_step;
@@ -705,7 +705,7 @@ copyfile_exit:

int copyfile(int fdin, void *out, size_t nbytes, unsigned long *offs, unsigned long long seek,
int skip_file, int __attribute__ ((__unused__)) compressed,
- uint32_t *checksum, unsigned char *hash, bool encrypted, const char *imgivt, writeimage callback)
+ uint32_t *checksum, unsigned char *hash, encrypted_t encrypted, const char *imgivt, writeimage callback)
{
return __swupdate_copy(fdin,
NULL,
@@ -723,7 +723,7 @@ int copyfile(int fdin, void *out, size_t nbytes, unsigned long *offs, unsigned l
}

int copybuffer(unsigned char *inbuf, void *out, size_t nbytes, int __attribute__ ((__unused__)) compressed,
- unsigned char *hash, bool encrypted, const char *imgivt, writeimage callback)
+ unsigned char *hash, encrypted_t encrypted, const char *imgivt, writeimage callback)
{
return __swupdate_copy(-1,
inbuf,
@@ -837,7 +837,7 @@ int cpio_scan(int fd, struct swupdate_cfg *cfg, off_t start)
* we do not have to provide fdout
*/
if (copyfile(fd, NULL, fdh.size, &offset, 0, 1, 0, &checksum, img ? img->sha256 : NULL,
- false, NULL, NULL) != 0) {
+ NO_ENCRYPTION, NULL, NULL) != 0) {
ERROR("invalid archive");
return -1;
}
diff --git a/core/stream_interface.c b/core/stream_interface.c
index 1cd148f..557cc5d 100644
--- a/core/stream_interface.c
+++ b/core/stream_interface.c
@@ -73,7 +73,7 @@ pthread_cond_t stream_cond = PTHREAD_COND_INITIALIZER;

static struct installer inst;

-static int extract_file_to_tmp(int fd, const char *fname, unsigned long *poffs, bool encrypted)
+static int extract_file_to_tmp(int fd, const char *fname, unsigned long *poffs, encrypted_t encrypted)
{
char output_file[MAX_IMAGE_FNAME];
struct filehdr fdh;
@@ -146,10 +146,14 @@ static int extract_files(int fd, struct swupdate_cfg *software)
char output_file[MAX_IMAGE_FNAME];
const char* TMPDIR = get_tmpdir();
bool installed_directly = false;
- bool encrypted_sw_desc = false;
+ encrypted_t encrypted_sw_desc = NO_ENCRYPTION;

#ifdef CONFIG_ENCRYPTED_SW_DESCRIPTION
- encrypted_sw_desc = true;
+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ encrypted_sw_desc = ASYMMETRIC;
+#else
+ encrypted_sw_desc = SYMMETRIC;
+#endif
#endif

/* preset the info about the install parts */
@@ -174,7 +178,7 @@ static int extract_files(int fd, struct swupdate_cfg *software)
case STREAM_WAIT_SIGNATURE:
#ifdef CONFIG_SIGNED_IMAGES
snprintf(output_file, sizeof(output_file), "%s.sig", SW_DESCRIPTION_FILENAME);
- if (extract_file_to_tmp(fd, output_file, &offset, false) < 0 )
+ if (extract_file_to_tmp(fd, output_file, &offset, NO_ENCRYPTION) < 0)
return -1;
#endif
snprintf(output_file, sizeof(output_file), "%s%s", TMPDIR, SW_DESCRIPTION_FILENAME);
@@ -243,7 +247,7 @@ static int extract_files(int fd, struct swupdate_cfg *software)
close(fdout);
return -1;
}
- if (copyfile(fd, &fdout, fdh.size, &offset, 0, 0, 0, &checksum, img->sha256, false, NULL, NULL) < 0) {
+ if (copyfile(fd, &fdout, fdh.size, &offset, 0, 0, 0, &checksum, img->sha256, NO_ENCRYPTION, NULL, NULL) < 0) {
close(fdout);
return -1;
}
@@ -255,7 +259,7 @@ static int extract_files(int fd, struct swupdate_cfg *software)
break;

case SKIP_FILE:
- if (copyfile(fd, &fdout, fdh.size, &offset, 0, skip, 0, &checksum, NULL, false, NULL, NULL) < 0) {
+ if (copyfile(fd, &fdout, fdh.size, &offset, 0, skip, 0, &checksum, NULL, NO_ENCRYPTION, NULL, NULL) < 0) {
return -1;
}
if (!swupdate_verify_chksum(checksum, &fdh)) {
@@ -382,11 +386,16 @@ static int save_stream(int fdin, struct swupdate_cfg *software)
unsigned long offset;
char output_file[MAX_IMAGE_FNAME];
const char* TMPDIR = get_tmpdir();
- bool encrypted_sw_desc = false;
+ encrypted_t encrypted_sw_desc = NO_ENCRYPTION;

#ifdef CONFIG_ENCRYPTED_SW_DESCRIPTION
- encrypted_sw_desc = true;
+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ encrypted_sw_desc = ASYMMETRIC;
+#else
+ encrypted_sw_desc = SYMMETRIC;
#endif
+#endif
+
if (fdin < 0)
return -EINVAL;

@@ -454,7 +463,7 @@ static int save_stream(int fdin, struct swupdate_cfg *software)
}
#ifdef CONFIG_SIGNED_IMAGES
snprintf(output_file, sizeof(output_file), "%s.sig", SW_DESCRIPTION_FILENAME);
- if (extract_file_to_tmp(tmpfd, output_file, &offset, false) < 0 ) {
+ if (extract_file_to_tmp(tmpfd, output_file, &offset, NO_ENCRYPTION) < 0) {
ERROR("Signature cannot be extracted:%s", output_file);
ret = -EINVAL;
goto no_copy_output;
diff --git a/include/util.h b/include/util.h
index f4a67ef..f995520 100644
--- a/include/util.h
+++ b/include/util.h
@@ -79,6 +79,12 @@ typedef enum {
LASTLOGLEVEL=DEBUGLEVEL
} LOGLEVEL;

+typedef enum {
+ NO_ENCRYPTION,
+ SYMMETRIC,
+ ASYMMETRIC
+} encrypted_t;
+
/*
* Following are used for notification from another process
*/
@@ -205,10 +211,10 @@ strlcpy(char *dst, const char * src, size_t size);
int copyfile(int fdin, void *out, size_t nbytes, unsigned long *offs,
unsigned long long seek,
int skip_file, int compressed, uint32_t *checksum,
- unsigned char *hash, bool encrypted, const char *imgivt, writeimage callback);
+ unsigned char *hash, encrypted_t encrypted, const char *imgivt, writeimage callback);
int copyimage(void *out, struct img_type *img, writeimage callback);
int copybuffer(unsigned char *inbuf, void *out, size_t nbytes, int compressed,
- unsigned char *hash, bool encrypted, const char *imgivt, writeimage callback);
+ unsigned char *hash, encrypted_t encrypted, const char *imgivt, writeimage callback);
int openfileoutput(const char *filename);
int mkpath(char *dir, mode_t mode);
int swupdate_file_setnonblock(int fd, bool block);
--
2.43.0

[Michael Glembotzki]

Asymmetric decryption is now supported exclusively for the sw-description
file. Applying asymmetric decryption to other artifacts is deemed
impractical. Hence, when 'encrypted == ASYMMETRIC,' an asymmetrically
encrypted sw-description file is anticipated and written to fdout. The
__swupdate_copy function decrypts the sw-description file from a temporary
copy named 'sw-description.enc,' which is subsequently removed post-update.

Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com>
---

Kconfig | 12 +++++++++++
core/cpio_utils.c | 55 +++++++++++++++++++++++++++++++++++++++++++++--
core/installer.c | 7 ++++++
3 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/Kconfig b/Kconfig
index 5a3dc9a..a6f0671 100644
--- a/Kconfig
+++ b/Kconfig
@@ -507,6 +507,18 @@ config ENCRYPTED_SW_DESCRIPTION
if this is set. It is a compile time option, and mix of plain and
encrypted sw-descriptions is not possible.

+config ASYM_ENCRYPTED_SW_DESCRIPTION
+ bool "Asymmetrical encrypted sw-description"
+ depends on ENCRYPTED_SW_DESCRIPTION && !PKCS11
+ depends on SSL_IMPL_OPENSSL
+ default n
+ help
+ This option enables support for asymmetrical encrypted sw-description,
+ making it possible to decrypt images device specific. The artifacts
+ themselves are still encrypted symmetrically. An AES key can optionally
+ be provided in the sw-description, or the default AES key will be used.
+ Cryptographic Message Syntax (CMS) is used for decryption.
+
config ENCRYPTED_IMAGES_HARDEN_LOGGING
bool "Harden logging for encrypted images"
default n
diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index 03d43c9..2310156 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -26,6 +26,7 @@
#include "util.h"
#include "sslapi.h"
#include "progress.h"
+#include "parsers.h"

#define MODULE_NAME "cpio"

@@ -444,6 +445,7 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
unsigned char *aes_key = NULL;
unsigned char *ivt = NULL;
unsigned char ivtbuf[AES_BLK_SIZE];
+ char keylen;

struct InputState input_state = {
.fdin = fdin,
@@ -513,7 +515,7 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
}

if (encrypted == SYMMETRIC) {
- aes_key = get_aes_key();
+ /* Use default ivt, if no image ivt is provided */

if (imgivt) {
if (!strlen(imgivt) || !is_hex_str(imgivt) || ascii_to_bin(ivtbuf, sizeof(ivtbuf), imgivt)) {

ERROR("Invalid image ivt");
@@ -522,7 +524,19 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
ivt = ivtbuf;
} else
ivt = get_aes_ivt();
- decrypt_state.dcrypt = swupdate_DECRYPT_init(aes_key, get_aes_keylen(), ivt);
+
+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ aes_key = get_tmp_aes_key();
+ keylen = get_tmp_aes_keylen();
+#endif
+
+ /* Use default aes-key, if no aes-key is provided within the sw-description */
+ if (!aes_key) {
+ aes_key = get_aes_key();
+ keylen = get_aes_keylen();
+ }
+
+ decrypt_state.dcrypt = swupdate_DECRYPT_init(aes_key, keylen, ivt);
if (!decrypt_state.dcrypt) {
ERROR("decrypt initialization failure, aborting");
ret = -EFAULT;
@@ -680,6 +694,43 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
*checksum = input_state.checksum;
}

+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ if (encrypted == ASYMMETRIC) {
+ char sw_desc_file[MAX_IMAGE_FNAME];
+ char sw_desc_file_enc[MAX_IMAGE_FNAME];
+ const char *TMPDIR = get_tmpdir();
+ /*
+ * Assume the asym encrypted sw-description file is written to fdout
+ */
+ int fdout = out ? *(int *)out : -1;
+
+ if (fdout < 0) {
+ ERROR("out argument: invalid fd or pointer");
+ ret = -EFAULT;
+ goto copyfile_exit;
+ }
+ close(fdout);
+
+ snprintf(sw_desc_file, sizeof(sw_desc_file), "%s%s", TMPDIR, SW_DESCRIPTION_FILENAME);
+ snprintf(sw_desc_file_enc, sizeof(sw_desc_file_enc), "%s.enc", sw_desc_file);
+
+ if (rename(sw_desc_file, sw_desc_file_enc)) {
+ ERROR("Renaming %s to %s failed", sw_desc_file, sw_desc_file_enc);
+ ret = -EFAULT;
+ goto copyfile_exit;
+ }
+
+ /*
+ * Decrypt the asym encrypted sw-description file
+ */
+ if (swupdate_decrypt_file(get_swupdate_cfg()->dgst, sw_desc_file_enc, sw_desc_file)) {
+ ERROR("Decrypting %s failed", sw_desc_file);
+ ret = -EFAULT;
+ goto copyfile_exit;
+ }
+ }
+#endif
+
ret = 0;

copyfile_exit:
diff --git a/core/installer.c b/core/installer.c
index 20b5b51..7707672 100644
--- a/core/installer.c
+++ b/core/installer.c
@@ -497,6 +497,13 @@ void cleanup_files(struct swupdate_cfg *software) {
free(fn);
}
#endif
+
+#ifdef CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION
+ if (asprintf(&fn, "%s%s.enc", TMPDIR, SW_DESCRIPTION_FILENAME) != ENOMEM_ASPRINTF) {
+ remove_sw_file(fn);
+ free(fn);
+ }
+#endif
}

int preupdatecmd(struct swupdate_cfg *swcfg)
--
2.43.0

[Michael Glembotzki]

Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com>
---

doc/source/asym_encrypted_images.rst | 153 +++++++++++++++++++++++++++

doc/source/encrypted_images.rst | 2 +
doc/source/index.rst | 1 +
doc/source/roadmap.rst | 5 -

doc/source/sw-description.rst | 13 ++-
5 files changed, 167 insertions(+), 7 deletions(-)
create mode 100644 doc/source/asym_encrypted_images.rst

diff --git a/doc/source/asym_encrypted_images.rst b/doc/source/asym_encrypted_images.rst
new file mode 100644
index 0000000..aa7bc5c
--- /dev/null
+++ b/doc/source/asym_encrypted_images.rst
@@ -0,0 +1,153 @@
+.. SPDX-FileCopyrightText: 2023 Michael Glembotzki <michael.g...@iris-sensing.com>
+.. SPDX-License-Identifier: GPL-2.0-only
+
+Asymmetrically Encrypted Update Images
+======================================
+
+Asymmetrically encrypted update images are realized by an asymmetrical
+encrypted sw-description, making it possible to decrypt images device specific.
+The artifacts themselves are still encrypted symmetrically. An AES key can
+optionally be provided in the sw-description, or the default AES key will be
+used. Cryptographic Message Syntax (CMS) is used for decryption.
+
+
+Use Cases
+---------
+
+- Asymmetrically encrypted update images, with individual device key pairs, are
+ inherently more secure than a purely symmetrical solution, because one
+ compromised private device key does not affect the security of the others.
+- If ``CONFIG_SIGNED_IMAGES`` is enabled too and a device's private key is
+ compromised, the key pair can be excluded from the list of eligible devices
+ for receiving new update images.
+- The AES key can be securely **exchanged** with each new update image, as it is
+ part of the sw-description, even in the absence of direct access to the
+ device.
+
+
+Create a Self-Signed Device Key Pair
+------------------------------------
+
+As an example, an elliptic curve key pair (PEM) is generated for a single
+device. These steps must be repeated for all other devices. An RSA key pair
+could be used in the same way.
+
+::
+
+ # Create a private key and a self-signed certificate
+ openssl ecparam -name secp521r1 -genkey -noout -out device-key-001.pem
+ openssl req -new -x509 -key device-key-001.pem -out device-cert-001.pem -subj "/O=SWUpdate /CN=target"
+
+ # Combine the private key and the certificate into a single file
+ cat device-key-001.pem device-cert-001.pem > device-001.pem
+
+
+Symmetric Encryption of Artifacts
+---------------------------------
+
+Generate an AES key and IV, as familiar from
+:ref:`symmetric image encryption <sym-encrypted-images>`. The encryption
+process for the artifacts remains unchanged.
+
+
+Encryption of sw-description for Multiple Devices
+-------------------------------------------------
+
+All device certificates togther are used for encryption.
+
+::
+
+ # Encrypt sw-description for multiple devices
+ openssl cms -encrypt -aes-256-cbc -in <INFILE> -out <OUTFILE> -outform DER -recip <CERT_1> <CERT_2> <CERT_X>
+
+Replace ``<INFILE>`` with the plain `sw-description` (e.g.
+`sw-description.in`) and the encrypted ``<OUTFILE>`` with `sw-description`.
+``<CERT_1>``, ``<CERT_2>``, [...] ``<CERT_X>`` constitute the comprehensive
+list of devices intended for encryption.
+
+
+Decryption of sw-description for a Single Device
+------------------------------------------------
+
+The combined key pair (private key and certificate) is used for decryption.
+SWUpdate handles the decryption process autonomously. Manually executing this
+step is not necessary and is provided here solely for development purposes.
+
+::
+
+ # Decrypt sw-description for a single device
+ openssl cms -decrypt -in <INFILE> -out ``<OUTFILE>`` -inform DER -inkey <PRIVATE_KEY_1> -recip <CERT_1>
+
+Replace the encrypted ``<INFILE>`` with `sw-description` and the
+``<OUTFILE>`` with plain `sw-description` (e.g. `sw-description.in`).
+``<PRIVATE_KEY_1>`` and ``<CERT_1>`` are used for the decryption.
+
+
+Example Asymmetrically Encrypted Image
+--------------------------------------
+
+The image artifacts should be symmetrically encrypted and signed in advance.
+Now, create a plain `sw-description.in` file. The ``encrypted`` attribute is
+necessary for encrypted artifacts. While it is strongly recommended to provide
+the attributes ``aes-key`` (global) and ``ivt`` (artifact-specific), they are
+not mandatory. If no ``aes-key`` or ``ivt`` is provided, the provided default
+``aes-key``/``ivt`` will be used.
+
+::
+
+ software =
+ {
+ version = "0.0.1";
+ aes-key = "ed73b9d3bf9c655d5a0b04836d8be48660a4a4bb6f4aa07c6778e00e342881ac";
+ images: ({
+ filename = "rootfs.ext4.enc";
+ device = "/dev/mmcblk0p3";
+ sha256 = "131159df3a4efaa890ff80173664a125c496c458dd432a8a6acae18872e35822";
+ encrypted = true;
+ ivt = "ea34a55a0c3476ed78f238ac87a7970c";
+ });
+ }
+
+
+Asymmetrically encrypt the `sw-description` for multiple devices:
+::
+
+ openssl cms -encrypt -aes-256-cbc -in sw-description.in -out sw-description -outform DER -recip device-cert-001.pem device-cert-002.pem device-cert-003.pem
+
+
+Create the new update image (SWU):
+
+::
+
+ #!/bin/sh
+
+ FILES="sw-description sw-description.sig rootfs.ext4.enc"
+
+ for i in $FILES; do
+ echo $i;done | cpio -ov -H crc > firmware.swu
+
+
+Running SWUpdate with Asymmetrically Encrypted Images
+-----------------------------------------------------
+
+Asymmetric encryption support can be enabled by configuring the compile-time
+option ``CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION``. To pass the combined key pair
+(PEM) generated earlier to SWUpdate, use the ``-a`` argument. Alternatively,
+use the ``asym-decryption-keypair`` parameter in the ``swupdate.cfg``.
+
+
+Security Considerations
+-----------------------
+- Ideally, generate the private key on the device during factory provisioning,
+ ensuring it never leaves the device. Only the public certificate leaves the
+ device for encrypting future update packages.
+- This feature should be used in conjunction with signature verification
+ (``CONFIG_SIGNED_IMAGES``) to ensure data integrity. In principle, anyone
+ with the corresponding device certificate can create update packages.
+- As a side effect, the size of the update package may significantly increase
+ in a large-scale deployment. To enhance scalability, consider using group
+ keys. Smaller groups should be preferred over larger ones.
+- Exchange the AES key in the sw-description with each update package.
+- Avoid encrypting new update packages for compromised devices, if there is no
+ direct access to the device or if unauthorized users have access to new update
+ packages.
diff --git a/doc/source/encrypted_images.rst b/doc/source/encrypted_images.rst
index 2b7c1ee..bc23681 100644
--- a/doc/source/encrypted_images.rst
+++ b/doc/source/encrypted_images.rst
@@ -1,6 +1,8 @@
.. SPDX-FileCopyrightText: 2013-2021 Stefano Babic <sba...@denx.de>
.. SPDX-License-Identifier: GPL-2.0-only

+.. _sym-encrypted-images:
+
Symmetrically Encrypted Update Images
=====================================

diff --git a/doc/source/index.rst b/doc/source/index.rst
index c3a8e88..3ed531a 100644
--- a/doc/source/index.rst
+++ b/doc/source/index.rst
@@ -41,6 +41,7 @@ SWUpdate Documentation
sw-description.rst
signed_images.rst
encrypted_images.rst
+ asym_encrypted_images.rst
handlers.rst
mongoose.rst
suricatta.rst
diff --git a/doc/source/roadmap.rst b/doc/source/roadmap.rst
index dc7d547..4e6caf4 100644
--- a/doc/source/roadmap.rst
+++ b/doc/source/roadmap.rst
@@ -138,11 +138,6 @@ BTRFS supports subvolume and delta backup for volumes - supporting subvolumes is
to move the delta approach to filesystems, while SWUpdate should apply the deltas
generated by BTRFS utilities.

-Security
-========
-
-- add support for asymmetryc decryption
-
Support for evaluation boards
=============================

diff --git a/doc/source/sw-description.rst b/doc/source/sw-description.rst
index 480ff4d..6e7e9bb 100644
--- a/doc/source/sw-description.rst
+++ b/doc/source/sw-description.rst
@@ -1441,8 +1441,17 @@ There are 4 main sections inside sw-description:
| | | scripts | and must be decrypted before |
| | | | installing. |
+-------------+----------+------------+---------------------------------------+
- | ivt | string | images | IVT in case of encrypted artefact |
- | | | files | It has no value if "encrypted" is not |
+ | aes-key | string | | Optional AES key for encrypted |
+ | | | | artefacts. It has no effect if not |
+ | | | | compiled with |
+ | | | | `CONFIG_ASYM_ENCRYPTED_SW_DESCRIPTION`|
+ | | | | or if attribute "encrypted" is not |
+ | | | | set. If no AES key is provided the |
+ | | | | default AES key is used. It is an |
+ | | | | ASCII hex string of 16/24/32 chars. |
+ +-------------+----------+------------+---------------------------------------+
+ | ivt | string | images | Optional IVT for encrypted artefacts. |
+ | | | files | It has no effect if "encrypted" is not|
| | | scripts | set. Each artefact can have an own |
| | | | IVT to avoid attacker can guess the |
| | | | the key. |
--
2.43.0

[Dominique MARTINET]

Michael Glembotzki wrote on Mon, Jan 15, 2024 at 08:26:45PM +0100:
> Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com>

Thanks for this patch series!

I'll check the code more thoroughly as time permits but these points
below aside I didn't see any obviously big problem.

> +Example Asymmetrically Encrypted Image
> +--------------------------------------
> +
> +The image artifacts should be symmetrically encrypted and signed in advance.

Sorry if this has been discussed earlier, but if I read this (and the
code) correctly, we're decrypting the data first and then checking the
signature -- I think it'd make more sense to do it the other way around
even if it means shuffling a bit of code around (cpio's first
extract_file_to_tmp for swdesc would be no encryption and then after
signature has been checked we can decrypt it)

Best practices would be to check signature as soon as possible; we're
already trusting unchecked input with cpio header processing, cms
decrpytion code is considerably harder to validate for
robustness/security and might have vulnerabilities or leak secrets
through error timing on invalid input.

As bonus, this would allow making this feature more easily optional in
configuration (this is $jobs' problem but we're not building images
directly for our customers, so we'll have to support plain text
sw-description forever, but I'd love to upgrade and provide this as an
optional feature) -- since we've checked the signature first, we can
trust a header or whatever (alternate file name?) to decide whether it's
plain text or encrypted and proceed accordingly.

> +Create the new update image (SWU):
> +
> +::
> +
> + #!/bin/sh
> +
> + FILES="sw-description sw-description.sig rootfs.ext4.enc"
> +
> + for i in $FILES; do
> + echo $i;done | cpio -ov -H crc > firmware.swu

(code formatting here is a bit weird)

> +Security Considerations
> +-----------------------
> +- Ideally, generate the private key on the device during factory provisioning,
> + ensuring it never leaves the device. Only the public certificate leaves the
> + device for encrypting future update packages.

We have a 'secure element' on our boards so I'd love to try it in
combinaison with that...

> +- As a side effect, the size of the update package may significantly increase
> + in a large-scale deployment. To enhance scalability, consider using group
> + keys. Smaller groups should be preferred over larger ones.

One could also generate a batch of swu, each for a smaller group of
devices -- this degrades security less than group keys which have to get
out of the device at some point and could leak.

Another consideration here is more of a bug than a problem, but I think
the current core/stream_interface.c save_stream() will break with a few
handful of devices -- it's only reading 16k to the temporary file from
which we extract the sw-description and its signature, but with this and
a very small sw-description I hit 16k with around 35 devices (I expect
to hit it with even less on real world sw-descriptions)
We should probably increase that size quite a bit, or make it pull in
more data dynamically...



Cheers,
--
Dominique

[Stefano Babic]

Hi,

On 16.01.24 01:57, Dominique MARTINET wrote:
> Michael Glembotzki wrote on Mon, Jan 15, 2024 at 08:26:45PM +0100:
>> Signed-off-by: Michael Glembotzki <Michael.G...@iris-sensing.com>
>
> Thanks for this patch series!
>
> I'll check the code more thoroughly as time permits but these points
> below aside I didn't see any obviously big problem.
>

I have not yet checked the code, I just jump in the discussion:

>
>> +Example Asymmetrically Encrypted Image
>> +--------------------------------------
>> +
>> +The image artifacts should be symmetrically encrypted and signed in advance.
>
> Sorry if this has been discussed earlier, but if I read this (and the
> code) correctly, we're decrypting the data first and then checking the
> signature -- I think it'd make more sense to do it the other way around
> even if it means shuffling a bit of code around (cpio's first
> extract_file_to_tmp for swdesc would be no encryption and then after
> signature has been checked we can decrypt it)
>
> Best practices would be to check signature as soon as possible; we're
> already trusting unchecked input with cpio header processing, cms
> decrpytion code is considerably harder to validate for
> robustness/security and might have vulnerabilities or leak secrets
> through error timing on invalid input.

Nevertheless, symmetric encryption is since long time supported, and
order is that sw-description is first decrypted and then verified. Even
if you exposed a right concept, changing the order introduces a hard
backward compatibility.

On the other hand, I do not think that this introduce an issue: the
major point with decryption is that devices should have a safe box to
store the keys. Some customers of mine have just use encryption without
verification, because, they say, there is no way to push a malformed
software until key is safe. Of course, it is better to have both, but if
sw-description is encrypted and it contains sha256 for artifacts the
artifacts are verified as well.

>
> As bonus, this would allow making this feature more easily optional in
> configuration (this is $jobs' problem but we're not building images
> directly for our customers, so we'll have to support plain text
> sw-description forever, but I'd love to upgrade and provide this as an
> optional feature)

Is it an optional feature, isn't it ? SWUpdate must be explicitly
configured to enable encryption (via symmetric or asymmetric keys) of
sw-description.

Yes, possible, it could be a use case.

> -- this degrades security less than group keys which have to get
> out of the device at some point and could leak.
>
> Another consideration here is more of a bug than a problem, but I think
> the current core/stream_interface.c save_stream() will break with a few
> handful of devices -- it's only reading 16k to the temporary file from
> which we extract the sw-description and its signature, but with this and
> a very small sw-description I hit 16k with around 35 devices (I expect
> to hit it with even less on real world sw-descriptions)
> We should probably increase that size quite a bit, or make it pull in
> more data dynamically...

You hit an issue and this should be then fixed - save_stream already
detect the size for sw-description, it should then copy until the end
and get again the size of the signature from the cpio header.

Best regards,
Stefano

[Dominique MARTINET]

Stefano Babic wrote on Tue, Jan 16, 2024 at 09:31:52PM +0100:
> > > +Example Asymmetrically Encrypted Image
> > > +--------------------------------------
> > > +
> > > +The image artifacts should be symmetrically encrypted and signed in advance.
> >
> > Sorry if this has been discussed earlier, but if I read this (and the
> > code) correctly, we're decrypting the data first and then checking the
> > signature -- I think it'd make more sense to do it the other way around
> > even if it means shuffling a bit of code around (cpio's first
> > extract_file_to_tmp for swdesc would be no encryption and then after
> > signature has been checked we can decrypt it)
> >
> > Best practices would be to check signature as soon as possible; we're
> > already trusting unchecked input with cpio header processing, cms
> > decrpytion code is considerably harder to validate for
> > robustness/security and might have vulnerabilities or leak secrets
> > through error timing on invalid input.
>
> Nevertheless, symmetric encryption is since long time supported, and order
> is that sw-description is first decrypted and then verified. Even if you
> exposed a right concept, changing the order introduces a hard backward
> compatibility.

Yes, symmetric encryption cannot be changed, so if we do this it'll be
somewhat confusing in that we'll have symmetric do decryption then
signature check and assymetric do it the other way around.

I still think it's worth it.

> On the other hand, I do not think that this introduce an issue: the major
> point with decryption is that devices should have a safe box to store the
> keys. Some customers of mine have just use encryption without verification,
> because, they say, there is no way to push a malformed software until key is
> safe. Of course, it is better to have both, but if sw-description is
> encrypted and it contains sha256 for artifacts the artifacts are verified as
> well.

I am sorry for them...
The current sw-description symmetric encryption is using AES with a
fixed key/iv (yes, it's possible to change iv after each update -- how
many users actually do it?) in cbc mode;

- the key is 100% shared and symmetric so if an attacker gets their hand
on any device they can create updates that'll be installable on all
devices freely!
- even if iv wasn't fixed, AES cbc mode without hmac has no tampering
protections so attacks such as this one are possible:
https://crypto.stackexchange.com/a/66086
It might take a few attempts to get the location right but in our case I
think it wouldn't be too difficult to change just a hash somewhere to
install only part of a update (that'll fail the update but if part of it
isn't A/B-safe it might have some impact), or with a few more attempts
change the version of a component so it's skipped with a successful
install.
(I didn't spend a lot of time studying this, but I've read enough bad
things about AES CBC to not trust it with authentifying data)

Anyway, we're not here to discuss about bad practices in current code -
let them do as they want.


For these patches at hand, assuming signature check is enabled, what I'm
worried about is that the code for CMS decryption is much larger than
simple AES CBC code -- there's ASN.1 parsing and a lot of fun x509
stuff which has had vulnerabilities in them, so I'd be much more at ease
knowing the data has been verified to be trusted first.
(Admittedly, the .sig file parsing should be pretty similar, so if there
are such bugs there they might already be reachable... Hopefully that
part has had more scrutinity though as it's untrusted data by design)

I'm curious so I'll try to setup AFL++ on something that calls
extract_file_to_tmp() directly with various options when I can find
time... Might take a week or two but I'll come back on this.

> > As bonus, this would allow making this feature more easily optional in
> > configuration (this is $jobs' problem but we're not building images
> > directly for our customers, so we'll have to support plain text
> > sw-description forever, but I'd love to upgrade and provide this as an
> > optional feature)
>
> Is it an optional feature, isn't it ? SWUpdate must be explicitly configured
> to enable encryption (via symmetric or asymmetric keys) of sw-description.

It's optional at compile time - this is my eternal problem that I'm not
the final integrator but just providing "easy to use" blocks, so to
allow this new usecase while preserving backwards compatibility I'd need
to ship two swupdate binaries with either mode enable (plaintext or
asym).
In this particular case it'd be easy enough to add a setting in
swupdate.cfg to toggle that instead, but that's still less flexible than
what I'm suggesting here:
if we check signature first, there should be no problem trusting the
file itself to decide if it wants to be encrypted or not, and decide
based on what was given.
This is already what we do for artifact encryption (for anything else
than sw-description), the sw-description decides if files are encrypted
or not so it's possible to have a decyption key configured but still
install an update where nothing is encrypted.

> > -- this degrades security less than group keys which have to get
> > out of the device at some point and could leak.
> >
> > Another consideration here is more of a bug than a problem, but I think
> > the current core/stream_interface.c save_stream() will break with a few
> > handful of devices -- it's only reading 16k to the temporary file from
> > which we extract the sw-description and its signature, but with this and
> > a very small sw-description I hit 16k with around 35 devices (I expect
> > to hit it with even less on real world sw-descriptions)
> > We should probably increase that size quite a bit, or make it pull in
> > more data dynamically...
>
> You hit an issue and this should be then fixed - save_stream already detect
> the size for sw-description, it should then copy until the end and get again
> the size of the signature from the cpio header.

Yes, up to some (configurable?) limit though or that opens up a new
attack (making TMPDIR full) because data hasn't been verified yet at
this point.

--
Dominique

[Michael Glembotzki]

Hi,

Am Mi., 17. Jan. 2024 um 00:19 Uhr schrieb Dominique MARTINET
<dominique...@atmark-techno.com>:

>
> Stefano Babic wrote on Tue, Jan 16, 2024 at 09:31:52PM +0100:
> > > > +Example Asymmetrically Encrypted Image
> > > > +--------------------------------------
> > > > +
> > > > +The image artifacts should be symmetrically encrypted and signed in advance.
> > >
> > > Sorry if this has been discussed earlier, but if I read this (and the
> > > code) correctly, we're decrypting the data first and then checking the
> > > signature -- I think it'd make more sense to do it the other way around
> > > even if it means shuffling a bit of code around (cpio's first
> > > extract_file_to_tmp for swdesc would be no encryption and then after
> > > signature has been checked we can decrypt it)
> > >
> > > Best practices would be to check signature as soon as possible; we're
> > > already trusting unchecked input with cpio header processing, cms
> > > decrpytion code is considerably harder to validate for
> > > robustness/security and might have vulnerabilities or leak secrets
> > > through error timing on invalid input.
> >
> > Nevertheless, symmetric encryption is since long time supported, and order
> > is that sw-description is first decrypted and then verified. Even if you
> > exposed a right concept, changing the order introduces a hard backward
> > compatibility.
>
> Yes, symmetric encryption cannot be changed, so if we do this it'll be
> somewhat confusing in that we'll have symmetric do decryption then
> signature check and assymetric do it the other way around.

Switching the order only for asymmetric decryption seems off to me. We could
fix this by using a Compile Time Config. New projects go for the secure way
(verify first, then decrypt), while existing ones stick to the old
method (decrypt
first, then verify) to avoid issues. This way, we don't need to resolve it
immediately within this patch series.

> For these patches at hand, assuming signature check is enabled, what I'm
> worried about is that the code for CMS decryption is much larger than
> simple AES CBC code -- there's ASN.1 parsing and a lot of fun x509
> stuff which has had vulnerabilities in them, so I'd be much more at ease
> knowing the data has been verified to be trusted first.
> (Admittedly, the .sig file parsing should be pretty similar, so if there
> are such bugs there they might already be reachable... Hopefully that
> part has had more scrutinity though as it's untrusted data by design)

What about CMS verify? If you consider the possibility of similar
vulnerabilities in CMS decrypt, there could also be potential issues in
CMS verify. So, changing the order might not yield any immediate benefits.

> I'm curious so I'll try to setup AFL++ on something that calls
> extract_file_to_tmp() directly with various options when I can find
> time... Might take a week or two but I'll come back on this.

I would be interested in the results.

> > > +- As a side effect, the size of the update package may significantly increase
> > > + in a large-scale deployment. To enhance scalability, consider using group
> > > + keys. Smaller groups should be preferred over larger ones.
> >
> > One could also generate a batch of swu, each for a smaller group of
> > devices
> Yes, possible, it could be a use case

By the term group keys I actually meant group sizes of perhaps e.g. 20..50
devices per group. In the end, you have to estimate the maximum expected
size of the update packages depending on your expected total number of
devices (or batches) and key type/size.

> > > Another consideration here is more of a bug than a problem, but I think
> > > the current core/stream_interface.c save_stream() will break with a few
> > > handful of devices -- it's only reading 16k to the temporary file from
> > > which we extract the sw-description and its signature, but with this and
> > > a very small sw-description I hit 16k with around 35 devices (I expect
> > > to hit it with even less on real world sw-descriptions)
> > > We should probably increase that size quite a bit, or make it pull in
> > > more data dynamically...
> >
> > You hit an issue and this should be then fixed - save_stream already detect
> > the size for sw-description, it should then copy until the end and get again
> > the size of the signature from the cpio header.
>

Good finding. A multiple of 16K is read (depending on the size of
sw-description). This is a problem if the sw-desc is just a few bytes below the
16K limit. Then the signature no longer fits. Will you fix it Dominique?

Best regards,
Michael

[Stefano Babic]

Hi Dominique,

I agree that reverting the order makes sense, but it should be done in a
way that does not break devices in field.

>
>> On the other hand, I do not think that this introduce an issue: the major
>> point with decryption is that devices should have a safe box to store the
>> keys. Some customers of mine have just use encryption without verification,
>> because, they say, there is no way to push a malformed software until key is
>> safe. Of course, it is better to have both, but if sw-description is
>> encrypted and it contains sha256 for artifacts the artifacts are verified as
>> well.
>
> I am sorry for them...
> The current sw-description symmetric encryption is using AES with a
> fixed key/iv (yes, it's possible to change iv after each update -- how
> many users actually do it?) in cbc mode;

I will happy to provide statistics if users will share their experience,
but I got no answers each time I asked to share own experience (like
just to report who is using the project...)

I can just tell that some customers of mine are using it.

>
> - the key is 100% shared and symmetric so if an attacker gets their hand
> on any device they can create updates that'll be installable on all
> devices freely!

But this is the very old story with keys. Of course it is, but it always
happened when the secrets are not secret anymore.

It is best practice in SWupdate to upgrade the symmetric key with any
update, but as drawback a downgrading doesn't work anymore.

> - even if iv wasn't fixed, AES cbc mode without hmac has no tampering
> protections so attacks such as this one are possible:
> https://crypto.stackexchange.com/a/66086
> It might take a few attempts to get the location right but in our case I
> think it wouldn't be too difficult to change just a hash somewhere to
> install only part of a update (that'll fail the update but if part of it
> isn't A/B-safe it might have some impact),

If update fails, the installed artifact is not activated. If it is, the
integrator has done a bad work.

> or with a few more attempts
> change the version of a component so it's skipped with a successful
> install.

Version of artifacts in the SWU are put inside sw-description and they
are verified. I don't think it is easy as you describe.

> (I didn't spend a lot of time studying this, but I've read enough bad
> things about AES CBC to not trust it with authentifying data)

Anyway, independently of this, I would like to add more decryption
algorithms to SWUpdate, and not just rely on AES-CBC. I would like to
add, if some company will finance the development, AES-CTR to have
encrypted delta updates, for example. But I see that the "encrypted"
attribute could switch from boll value to a string with the selected
crypto alg.

>
> Anyway, we're not here to discuss about bad practices in current code -
> let them do as they want.
>
>
> For these patches at hand, assuming signature check is enabled, what I'm
> worried about is that the code for CMS decryption is much larger than
> simple AES CBC code

Sorry, this is not a reason. There is a lot of complicated features, and
of course this requires code. If there will be a leak / bug, it should
be fixed. What shouldn't be done, is to reimplement the algorithms
inside SWUpdate instead of using well proven libraries, and this is not
done.

>-- there's ASN.1 parsing and a lot of fun x509
> stuff which has had vulnerabilities in them, so I'd be much more at ease
> knowing the data has been verified to be trusted first.
> (Admittedly, the .sig file parsing should be pretty similar, so if there
> are such bugs there they might already be reachable

Right, this is not an argument. Verification takes place, and it is also
not easy.

>... Hopefully that
> part has had more scrutinity though as it's untrusted data by design)

I think this is just your feeling.

>
> I'm curious so I'll try to setup AFL++ on something that calls
> extract_file_to_tmp() directly with various options when I can find
> time... Might take a week or two but I'll come back on this.
>
>
>>> As bonus, this would allow making this feature more easily optional in
>>> configuration (this is $jobs' problem but we're not building images
>>> directly for our customers, so we'll have to support plain text
>>> sw-description forever, but I'd love to upgrade and provide this as an
>>> optional feature)
>>
>> Is it an optional feature, isn't it ? SWUpdate must be explicitly configured
>> to enable encryption (via symmetric or asymmetric keys) of sw-description.
>
> It's optional at compile time - this is my eternal problem that I'm not
> the final integrator but just providing "easy to use" blocks, so to
> allow this new usecase while preserving backwards compatibility I'd need
> to ship two swupdate binaries with either mode enable (plaintext or
> asym).

Yes, understood - not easy to be solved. SWUpdate requires that the
integrator understands what is going on, and adjusts SWupdate for the
specific project. It is not thought to deliver N binaries and to let the
integrator just to pick one up.

> In this particular case it'd be easy enough to add a setting in
> swupdate.cfg to toggle that instead, but that's still less flexible than
> what I'm suggesting here:

The reason that these security features are defined at compile time is
to avoid that in some way an attacker can circumvent the chosen way. If
it is not compiled, it cannot run. So I already rejected patches that
allows to skip verification process, or to change verification /
decryption at runtime.

> if we check signature first, there should be no problem trusting the
> file itself to decide if it wants to be encrypted or not, and decide
> based on what was given.
> This is already what we do for artifact encryption (for anything else
> than sw-description), the sw-description decides if files are encrypted
> or not so it's possible to have a decyption key configured but still
> install an update where nothing is encrypted.

Verification first should be the path to do, and I cannot say why I did
the opposite in the past for sw-description. Rather, nobody raised the
point when I pushed it. I will agree to change it if we maintain a
compiler switch (at least for some versions) that guarantees
compatibility with the past, else there will be a lot of complaints.

>
>>> -- this degrades security less than group keys which have to get
>>> out of the device at some point and could leak.
>>>
>>> Another consideration here is more of a bug than a problem, but I think
>>> the current core/stream_interface.c save_stream() will break with a few
>>> handful of devices -- it's only reading 16k to the temporary file from
>>> which we extract the sw-description and its signature, but with this and
>>> a very small sw-description I hit 16k with around 35 devices (I expect
>>> to hit it with even less on real world sw-descriptions)
>>> We should probably increase that size quite a bit, or make it pull in
>>> more data dynamically...
>>
>> You hit an issue and this should be then fixed - save_stream already detect
>> the size for sw-description, it should then copy until the end and get again
>> the size of the signature from the cpio header.
>
> Yes, up to some (configurable?) limit though or that opens up a new
> attack (making TMPDIR full) because data hasn't been verified yet at
> this point.
>

Best regards,
Stefano

[Stefano Babic]

Hi Michael,

I agree with this approach to guarantee backward compatibility.
Dominique is right to ask to revert the order, but well, there are a lot
of devices in field.

>
>> For these patches at hand, assuming signature check is enabled, what I'm
>> worried about is that the code for CMS decryption is much larger than
>> simple AES CBC code -- there's ASN.1 parsing and a lot of fun x509
>> stuff which has had vulnerabilities in them, so I'd be much more at ease
>> knowing the data has been verified to be trusted first.
>> (Admittedly, the .sig file parsing should be pretty similar, so if there
>> are such bugs there they might already be reachable... Hopefully that
>> part has had more scrutinity though as it's untrusted data by design)
> What about CMS verify? If you consider the possibility of similar
> vulnerabilities in CMS decrypt, there could also be potential issues in
> CMS verify. So, changing the order might not yield any immediate benefits.

It is also my point.

Agree that this must be fixed, too, else we can quickly get into trouble
when sw-description's size increases.


Best regards,
Stefano

[Dominique MARTINET]

Michael Glembotzki wrote on Wed, Jan 17, 2024 at 01:01:59PM +0100:
> > Yes, symmetric encryption cannot be changed, so if we do this it'll be
> > somewhat confusing in that we'll have symmetric do decryption then
> > signature check and assymetric do it the other way around.
> Switching the order only for asymmetric decryption seems off to me. We could
> fix this by using a Compile Time Config. New projects go for the secure way
> (verify first, then decrypt), while existing ones stick to the old
> method (decrypt first, then verify) to avoid issues. This way, we
> don't need to resolve it immediately within this patch series.

Hmm, I feel it's a bit suboptimal to allow the (decrypt async, verify)
order in the first place if we're going to do that, but it's probably
better than delaying this feature.

Okay, let's go with that (if someone works on it)

> > For these patches at hand, assuming signature check is enabled, what I'm
> > worried about is that the code for CMS decryption is much larger than
> > simple AES CBC code -- there's ASN.1 parsing and a lot of fun x509
> > stuff which has had vulnerabilities in them, so I'd be much more at ease
> > knowing the data has been verified to be trusted first.
> > (Admittedly, the .sig file parsing should be pretty similar, so if there
> > are such bugs there they might already be reachable... Hopefully that
> > part has had more scrutinity though as it's untrusted data by design)
>
> What about CMS verify? If you consider the possibility of similar
> vulnerabilities in CMS decrypt, there could also be potential issues in
> CMS verify. So, changing the order might not yield any immediate benefits.

Yes, that's what I pointed at :)
I'd assume verify is more battle-tested than decrypt because the point
is checking untrusted data, but I agree the code will partly be shared
between the two.

> > I'm curious so I'll try to setup AFL++ on something that calls
> > extract_file_to_tmp() directly with various options when I can find
> > time... Might take a week or two but I'll come back on this.
>
> I would be interested in the results.

I've let it run over the weekend (1 day with just cpio/signature, a few
hours with symmetric decryption and 2.5 days with asymmetric decryption)
and was happy to not see any obvious failure.

If anyone cares to check, I've used the attached swupdate-fuzz.c as a
drop-in replacement to swupdate.c + patch and built with afl
(`CC=afl-clang-fast make`), running afl-fuzz as documented (with a few
variant builds and slave processes) after checking my sample input
passed all the way through verifying the signature.

I'm told blind fuzzing isn't actually that well suited for this kind of
cryptography checks (the input will almost always just fail to validate
something obvious) so this doesn't guarantee much, but it's better than
nothing.

> > > > +- As a side effect, the size of the update package may significantly increase
> > > > + in a large-scale deployment. To enhance scalability, consider using group
> > > > + keys. Smaller groups should be preferred over larger ones.
> > >
> > > One could also generate a batch of swu, each for a smaller group of
> > > devices
> > Yes, possible, it could be a use case
> By the term group keys I actually meant group sizes of perhaps e.g. 20..50
> devices per group. In the end, you have to estimate the maximum expected
> size of the update packages depending on your expected total number of
> devices (or batches) and key type/size.

I'm not exactly sure what this clarifies. From my understanding there
are two main ways this could be split to limit the number of
certificates in a given "recip" list pass:
- share a key between multiple devices, so your update file will still
be installable on all your fleet
- build multiple update files, so each device still have their own key
but a given update fille will only be installable on part of the fleet
and your update mechanism will need to send the right file to the right
place

Given you were suggesting to not have the key leave the device at
install time (generated on the device), the later is probably more
appropriate to recommend, even if it's more work to setup.
(Well, I have no problem with both or either being listed, each vendor
should make their own informed decision ultimately)

> > > > Another consideration here is more of a bug than a problem, but I think
> > > > the current core/stream_interface.c save_stream() will break with a few
> > > > handful of devices -- it's only reading 16k to the temporary file from
> > > > which we extract the sw-description and its signature, but with this and
> > > > a very small sw-description I hit 16k with around 35 devices (I expect
> > > > to hit it with even less on real world sw-descriptions)
> > > > We should probably increase that size quite a bit, or make it pull in
> > > > more data dynamically...
> > >
> > > You hit an issue and this should be then fixed - save_stream already detect
> > > the size for sw-description, it should then copy until the end and get again
> > > the size of the signature from the cpio header.
> >
> Good finding. A multiple of 16K is read (depending on the size of
> sw-description). This is a problem if the sw-desc is just a few bytes below the
> 16K limit. Then the signature no longer fits. Will you fix it Dominique?

Sorry I was just mis-reading the code there, I missed the part where it
takes into account the sw-description size
(but now you're pointing this out I also think we could get the align
call to make the sig not fit if we try, I've never had any
sw-description close enough to 16k to hit this)


This brings in the issue I gave in an earlier reply which is we're
filling TMPDIR with as much data as an attacker wants:
---
$ dd if=/dev/zero of=sw-description bs=1M count=1000
$ echo sw-description | cpio -H newc -o > test.cpio
$ swupdate -i test.cpio
[...]
[ERROR] : SWUPDATE failed [0] ERROR : cannot write 16384 bytes: No space left on device
[ERROR] : SWUPDATE failed [1] Image invalid or corrupted. Not installing ...
---

Thanksfully swupdate cleans up sw-description immediately after that,
but if update can be triggered openly it's still possible to cause
trouble, so I'll send a patch to limit the size in config with some
arbitrary default value we can discuss there.

I'll have a look at the 16K boundary while I'm at it, it's probably not
hard to reproduce by just inserting an appropriate number of
whitespaces...

(But either way I probably won't have time this week and will have a
busy start of Feb -- so that'll probably be mid-Feb. If you want this
earlier feel free to not wait for me)


Thanks,
--
Dominique

[Michael Glembotzki]

Hi,

it's been a while. What is the next step?

Is there still interest in this feature?

I would appreciate a code review.

Best regards
Michael

[Stefano Babic]

Hi Michael,

On 01.03.24 14:11, Michael Glembotzki wrote:
>
> Hi,
> it's been a while. What is the next step?

I was thinking about...

> Is there still interest in this feature?

Sure - I think it is a nice feature.

There are some issues shown with this series and I checked deeply the
current status for encryption / decryption, see this thread, too:

https://groups.google.com/g/swupdate/c/z4oYcaJv5Rs

This convinces me that a rework of current crypto integration will be
necessary.

Nevertheless, this set has also discovered that sw-description.sig is
currently limited to 16KB. This is a topic I set in my TODO list, and I
will solve it.

> I would appreciate a code review.

Sure - I will review soon your patches.

Best regards,
Stefano

> --
> 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+u...@googlegroups.com
> <mailto:swupdate+u...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/swupdate/b0c55eb3-7996-4025-a1cb-c6e9b95f4732n%40googlegroups.com <https://groups.google.com/d/msgid/swupdate/b0c55eb3-7996-4025-a1cb-c6e9b95f4732n%40googlegroups.com?utm_medium=email&utm_source=footer>.