diff mbox series

[V3,08/10] cpio_utils: Add argument imgaeskey to __swupdate_copy interface

Message ID 20231215142251.52393-9-Michael.Glembotzki@iris-sensing.com
State Changes Requested
Headers show
Series Add support for asymmetric decryption | expand

Commit Message

Michael Glembotzki Dec. 15, 2023, 2:19 p.m. UTC
Signed-off-by: Michael Glembotzki <Michael.Glembotzki@iris-sensing.com>
---
 core/cpio_utils.c           | 13 +++++++++----
 core/installer.c            |  1 +
 core/stream_interface.c     |  6 +++---
 corelib/lua_interface.c     |  2 ++
 handlers/copy_handler.c     |  1 +
 handlers/delta_handler.c    |  1 +
 handlers/rdiff_handler.c    |  1 +
 handlers/readback_handler.c |  1 +
 include/util.h              |  6 ++++--
 9 files changed, 23 insertions(+), 9 deletions(-)

Comments

Stefano Babic Dec. 19, 2023, 8:28 p.m. UTC | #1
Hi Michael,

On 15.12.23 15:19, Michael Glembotzki wrote:
> Signed-off-by: Michael Glembotzki <Michael.Glembotzki@iris-sensing.com>
> ---
>   core/cpio_utils.c           | 13 +++++++++----
>   core/installer.c            |  1 +
>   core/stream_interface.c     |  6 +++---
>   corelib/lua_interface.c     |  2 ++
>   handlers/copy_handler.c     |  1 +
>   handlers/delta_handler.c    |  1 +
>   handlers/rdiff_handler.c    |  1 +
>   handlers/readback_handler.c |  1 +
>   include/util.h              |  6 ++++--
>   9 files changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/core/cpio_utils.c b/core/cpio_utils.c
> index 5b99904..6e2c239 100644
> --- a/core/cpio_utils.c
> +++ b/core/cpio_utils.c
> @@ -431,7 +431,8 @@ 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, bool encrypted,
> +	const char __attribute__ ((__unused__)) *imgaeskey, const char *imgivt, writeimage callback)

Change of fingerprint of this core function does not convince me. I am 
also not convinced about "recipient" tag in several part of the series.

SWUpdate works sequentially, that means two decryption of an artifact at 
the same time is excluded by design.

The decryption of sw-description is done inside a single function 
(extract_file_to_tmp), and this is local to the stream_interface module. 
Type of decryption (symmetric vs asymettric) should be managed inside 
this function without spreading to all interfaces.

The sw-dgst structure (global) can have a flag to distinguish between 
symmetric and asymmetric decryption. The cpio functions should just know 
if artifact (or sw-description) to be handled is encrypted without being 
aware of detailt like type of the decryption and keys.

Best regards,
Stefano

>   {
>   	unsigned int percent, prevpercent = 0;
>   	int ret = 0;
> @@ -705,7 +706,8 @@ 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, bool encrypted,
> +	const char *imgaeskey, const char *imgivt, writeimage callback)
>   {
>   	return __swupdate_copy(fdin,
>   				NULL,
> @@ -718,12 +720,13 @@ int copyfile(int fdin, void *out, size_t nbytes, unsigned long *offs, unsigned l
>   				checksum,
>   				hash,
>   				encrypted,
> +				imgaeskey,
>   				imgivt,
>   				callback);
>   }
>   
>   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, bool encrypted, const char *imgaeskey, const char *imgivt, writeimage callback)
>   {
>   	return __swupdate_copy(-1,
>   				inbuf,
> @@ -736,6 +739,7 @@ int copybuffer(unsigned char *inbuf, void *out, size_t nbytes, int __attribute__
>   				NULL,
>   				hash,
>   				encrypted,
> +				imgaeskey,
>   				imgivt,
>   				callback);
>   }
> @@ -752,6 +756,7 @@ int copyimage(void *out, struct img_type *img, writeimage callback)
>   			&img->checksum,
>   			img->sha256,
>   			img->is_encrypted,
> +			img->aeskey_ascii,
>   			img->ivt_ascii,
>   			callback);
>   }
> @@ -837,7 +842,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) {
> +				false, NULL, NULL, NULL) != 0) {
>   			ERROR("invalid archive");
>   			return -1;
>   		}
> diff --git a/core/installer.c b/core/installer.c
> index 20b5b51..db86075 100644
> --- a/core/installer.c
> +++ b/core/installer.c
> @@ -145,6 +145,7 @@ static int extract_scripts(struct imglist *head)
>   				&checksum,
>   				script->sha256,
>   				script->is_encrypted,
> +				script->aeskey_ascii,
>   				script->ivt_ascii,
>   				NULL);
>   		close(fdin);
> diff --git a/core/stream_interface.c b/core/stream_interface.c
> index 0b78329..bfafa30 100644
> --- a/core/stream_interface.c
> +++ b/core/stream_interface.c
> @@ -104,7 +104,7 @@ static int extract_file_to_tmp(int fd, const char *fname, unsigned long *poffs,
>   		return -1;
>   
>   	if (copyfile(fd, &fdout, fdh.size, poffs, 0, 0, 0, &checksum, NULL,
> -		     encrypted, NULL, NULL) < 0) {
> +		     encrypted, NULL, NULL, NULL) < 0) {
>   		close(fdout);
>   		return -1;
>   	}
> @@ -243,7 +243,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, false, NULL, NULL, NULL) < 0) {
>   					close(fdout);
>   					return -1;
>   				}
> @@ -255,7 +255,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, false, NULL, NULL, NULL) < 0) {
>   					return -1;
>   				}
>   				if (!swupdate_verify_chksum(checksum, &fdh)) {
> diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
> index af7b554..1533e9d 100644
> --- a/corelib/lua_interface.c
> +++ b/corelib/lua_interface.c
> @@ -401,6 +401,7 @@ static int l_copy2file(lua_State *L)
>   				 &checksum,
>   				 img.sha256,
>   				 img.is_encrypted,
> +				 img.aeskey_ascii,
>   				 img.ivt_ascii,
>   				 NULL);
>   	update_table(L, &img);
> @@ -473,6 +474,7 @@ static int l_istream_read(lua_State* L)
>   				 &checksum,
>   				 img.sha256,
>   				 img.is_encrypted,
> +				 img.aeskey_ascii,
>   				 img.ivt_ascii,
>   				 istream_read_callback);
>   
> diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c
> index e463bb5..d09ca52 100644
> --- a/handlers/copy_handler.c
> +++ b/handlers/copy_handler.c
> @@ -131,6 +131,7 @@ static int copy_single_file(const char *path, ssize_t size, struct img_type *img
>   			&checksum,
>   			0, /* no sha256 */
>   			false, /* no encrypted */
> +			NULL, /* no AES Key */
>   			NULL, /* no IVT */
>   			NULL);
>   
> diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c
> index d1ff783..a5ee2a6 100644
> --- a/handlers/delta_handler.c
> +++ b/handlers/delta_handler.c
> @@ -169,6 +169,7 @@ static int network_process_data(multipart_parser* p, const char *at, size_t leng
>   						 hash,
>   						 0,
>   						 NULL,
> +						 NULL,
>   						 NULL);
>   			} else
>   				ret = 0; /* skipping, nothing to be copied */
> diff --git a/handlers/rdiff_handler.c b/handlers/rdiff_handler.c
> index e01a127..3f09ec2 100644
> --- a/handlers/rdiff_handler.c
> +++ b/handlers/rdiff_handler.c
> @@ -347,6 +347,7 @@ static int apply_rdiff_patch(struct img_type *img,
>   			&img->checksum,
>   			img->sha256,
>   			img->is_encrypted,
> +			img->aeskey_ascii,
>   			img->ivt_ascii,
>   			apply_rdiff_chunk_cb);
>   	if (ret != 0) {
> diff --git a/handlers/readback_handler.c b/handlers/readback_handler.c
> index 4b910bd..6d2eefa 100644
> --- a/handlers/readback_handler.c
> +++ b/handlers/readback_handler.c
> @@ -113,6 +113,7 @@ static int readback_postinst(struct img_type *img)
>   			NULL,  /* no checksum */
>   			hash,
>   			false,     /* no encrypted */
> +			NULL,  /* no AES Key */
>   			NULL,     /* no IVT */
>   			NULL); /* no callback */
>   	if (status == 0) {
> diff --git a/include/util.h b/include/util.h
> index 062840f..0c5564c 100644
> --- a/include/util.h
> +++ b/include/util.h
> @@ -204,10 +204,12 @@ 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, bool encrypted, const char *imgaeskey, 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, bool encrypted, const char *imgaeskey, 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);
Michael Glembotzki Dec. 20, 2023, 9:57 a.m. UTC | #2
Hi Stefano,

Am Di., 19. Dez. 2023 um 21:28 Uhr schrieb Stefano Babic
<stefano.babic@swupdate.org>:
>
> Hi Michael,
>
> On 15.12.23 15:19, Michael Glembotzki wrote:
> > Signed-off-by: Michael Glembotzki <Michael.Glembotzki@iris-sensing.com>
> > ---
> >   core/cpio_utils.c           | 13 +++++++++----
> >   core/installer.c            |  1 +
> >   core/stream_interface.c     |  6 +++---
> >   corelib/lua_interface.c     |  2 ++
> >   handlers/copy_handler.c     |  1 +
> >   handlers/delta_handler.c    |  1 +
> >   handlers/rdiff_handler.c    |  1 +
> >   handlers/readback_handler.c |  1 +
> >   include/util.h              |  6 ++++--
> >   9 files changed, 23 insertions(+), 9 deletions(-)
> >
> > diff --git a/core/cpio_utils.c b/core/cpio_utils.c
> > index 5b99904..6e2c239 100644
> > --- a/core/cpio_utils.c
> > +++ b/core/cpio_utils.c
> > @@ -431,7 +431,8 @@ 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, bool encrypted,
> > +     const char __attribute__ ((__unused__)) *imgaeskey, const char *imgivt, writeimage callback)
>
> Change of fingerprint of this core function does not convince me. I am

Somehow the image->aeskey_ascii (from parser.c) must be passed in
__swupdate_copy(cpio_utils.c), just like the ivt. However, I could combine
aes-key and ivt in the struct decryption_key, then you only have one
argument, but
the fingerprint of the core function also changes as a result. What do you
think?

struct decryption_key {
    unsigned char key[AES_256_KEY_LEN];
    char keylen;
    unsigned char ivt[AES_BLK_SIZE];
};

> also not convinced about "recipient" tag in several part of the series.

The term "recipient" is taken from openssl cms and comes from email encryption,
but I'm open to a better suggestion if you have one. Would probably want to
introduce a new parameter rather than rename -K --key-aes and aeskeyfname to
something more generic for compatibility reasons though.

>
> SWUpdate works sequentially, that means two decryption of an artifact at
> the same time is excluded by design.
>
> The decryption of sw-description is done inside a single function
> (extract_file_to_tmp), and this is local to the stream_interface module.
> Type of decryption (symmetric vs asymettric) should be managed inside
> this function without spreading to all interfaces.
>
> The sw-dgst structure (global) can have a flag to distinguish between
> symmetric and asymmetric decryption. The cpio functions should just know
> if artifact (or sw-description) to be handled is encrypted without being
> aware of detailt like type of the decryption and keys.

I can move the asymmetric decryption of the sw-description from the
stream_interface.c to __swupdate_copy (cpio_utils.c) (is that what you mean?),
but to do this you would have to:
- pass the file name or
- change the bool encrypted to enum encrypted { NO_ENC, SYM, ASYM)
to recognize the sw-description. Asymmetrical decryption doesn't make sense for
the other artifacts anyway.
And the asym keypair has to be read from global swupdate_cfg.dgst via
get_swupdate_cfg(), if I understand you correctly.

Another aspect regarding the partial decryption like we do with the
swupdate_DECRYPT_* functions, it is not supported by openssl cms. This means
that you cannot decrypt the sw-description chunk by chunk. At the end, the
sw-description has to be read completely once in order to be able to decrypt it.

>
> Best regards,
> Stefano
>
> >   {
> >       unsigned int percent, prevpercent = 0;
> >       int ret = 0;
> > @@ -705,7 +706,8 @@ 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, bool encrypted,
> > +     const char *imgaeskey, const char *imgivt, writeimage callback)
> >   {
> >       return __swupdate_copy(fdin,
> >                               NULL,
> > @@ -718,12 +720,13 @@ int copyfile(int fdin, void *out, size_t nbytes, unsigned long *offs, unsigned l
> >                               checksum,
> >                               hash,
> >                               encrypted,
> > +                             imgaeskey,
> >                               imgivt,
> >                               callback);
> >   }
> >
> >   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, bool encrypted, const char *imgaeskey, const char *imgivt, writeimage callback)
> >   {
> >       return __swupdate_copy(-1,
> >                               inbuf,
> > @@ -736,6 +739,7 @@ int copybuffer(unsigned char *inbuf, void *out, size_t nbytes, int __attribute__
> >                               NULL,
> >                               hash,
> >                               encrypted,
> > +                             imgaeskey,
> >                               imgivt,
> >                               callback);
> >   }
> > @@ -752,6 +756,7 @@ int copyimage(void *out, struct img_type *img, writeimage callback)
> >                       &img->checksum,
> >                       img->sha256,
> >                       img->is_encrypted,
> > +                     img->aeskey_ascii,
> >                       img->ivt_ascii,
> >                       callback);
> >   }
> > @@ -837,7 +842,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) {
> > +                             false, NULL, NULL, NULL) != 0) {
> >                       ERROR("invalid archive");
> >                       return -1;
> >               }
> > diff --git a/core/installer.c b/core/installer.c
> > index 20b5b51..db86075 100644
> > --- a/core/installer.c
> > +++ b/core/installer.c
> > @@ -145,6 +145,7 @@ static int extract_scripts(struct imglist *head)
> >                               &checksum,
> >                               script->sha256,
> >                               script->is_encrypted,
> > +                             script->aeskey_ascii,
> >                               script->ivt_ascii,
> >                               NULL);
> >               close(fdin);
> > diff --git a/core/stream_interface.c b/core/stream_interface.c
> > index 0b78329..bfafa30 100644
> > --- a/core/stream_interface.c
> > +++ b/core/stream_interface.c
> > @@ -104,7 +104,7 @@ static int extract_file_to_tmp(int fd, const char *fname, unsigned long *poffs,
> >               return -1;
> >
> >       if (copyfile(fd, &fdout, fdh.size, poffs, 0, 0, 0, &checksum, NULL,
> > -                  encrypted, NULL, NULL) < 0) {
> > +                  encrypted, NULL, NULL, NULL) < 0) {
> >               close(fdout);
> >               return -1;
> >       }
> > @@ -243,7 +243,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, false, NULL, NULL, NULL) < 0) {
> >                                       close(fdout);
> >                                       return -1;
> >                               }
> > @@ -255,7 +255,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, false, NULL, NULL, NULL) < 0) {
> >                                       return -1;
> >                               }
> >                               if (!swupdate_verify_chksum(checksum, &fdh)) {
> > diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
> > index af7b554..1533e9d 100644
> > --- a/corelib/lua_interface.c
> > +++ b/corelib/lua_interface.c
> > @@ -401,6 +401,7 @@ static int l_copy2file(lua_State *L)
> >                                &checksum,
> >                                img.sha256,
> >                                img.is_encrypted,
> > +                              img.aeskey_ascii,
> >                                img.ivt_ascii,
> >                                NULL);
> >       update_table(L, &img);
> > @@ -473,6 +474,7 @@ static int l_istream_read(lua_State* L)
> >                                &checksum,
> >                                img.sha256,
> >                                img.is_encrypted,
> > +                              img.aeskey_ascii,
> >                                img.ivt_ascii,
> >                                istream_read_callback);
> >
> > diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c
> > index e463bb5..d09ca52 100644
> > --- a/handlers/copy_handler.c
> > +++ b/handlers/copy_handler.c
> > @@ -131,6 +131,7 @@ static int copy_single_file(const char *path, ssize_t size, struct img_type *img
> >                       &checksum,
> >                       0, /* no sha256 */
> >                       false, /* no encrypted */
> > +                     NULL, /* no AES Key */
> >                       NULL, /* no IVT */
> >                       NULL);
> >
> > diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c
> > index d1ff783..a5ee2a6 100644
> > --- a/handlers/delta_handler.c
> > +++ b/handlers/delta_handler.c
> > @@ -169,6 +169,7 @@ static int network_process_data(multipart_parser* p, const char *at, size_t leng
> >                                                hash,
> >                                                0,
> >                                                NULL,
> > +                                              NULL,
> >                                                NULL);
> >                       } else
> >                               ret = 0; /* skipping, nothing to be copied */
> > diff --git a/handlers/rdiff_handler.c b/handlers/rdiff_handler.c
> > index e01a127..3f09ec2 100644
> > --- a/handlers/rdiff_handler.c
> > +++ b/handlers/rdiff_handler.c
> > @@ -347,6 +347,7 @@ static int apply_rdiff_patch(struct img_type *img,
> >                       &img->checksum,
> >                       img->sha256,
> >                       img->is_encrypted,
> > +                     img->aeskey_ascii,
> >                       img->ivt_ascii,
> >                       apply_rdiff_chunk_cb);
> >       if (ret != 0) {
> > diff --git a/handlers/readback_handler.c b/handlers/readback_handler.c
> > index 4b910bd..6d2eefa 100644
> > --- a/handlers/readback_handler.c
> > +++ b/handlers/readback_handler.c
> > @@ -113,6 +113,7 @@ static int readback_postinst(struct img_type *img)
> >                       NULL,  /* no checksum */
> >                       hash,
> >                       false,     /* no encrypted */
> > +                     NULL,  /* no AES Key */
> >                       NULL,     /* no IVT */
> >                       NULL); /* no callback */
> >       if (status == 0) {
> > diff --git a/include/util.h b/include/util.h
> > index 062840f..0c5564c 100644
> > --- a/include/util.h
> > +++ b/include/util.h
> > @@ -204,10 +204,12 @@ 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, bool encrypted, const char *imgaeskey, 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, bool encrypted, const char *imgaeskey, 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);

Best regards,
Michael
Stefano Babic Jan. 10, 2024, 12:33 a.m. UTC | #3
Hi Michael,

sorry for delay:

On 20.12.23 04:57, Michael Glembotzki wrote:
> Hi Stefano,
> 
> Am Di., 19. Dez. 2023 um 21:28 Uhr schrieb Stefano Babic
> <stefano.babic@swupdate.org>:
>>
>> Hi Michael,
>>
>> On 15.12.23 15:19, Michael Glembotzki wrote:
>>> Signed-off-by: Michael Glembotzki <Michael.Glembotzki@iris-sensing.com>
>>> ---
>>>    core/cpio_utils.c           | 13 +++++++++----
>>>    core/installer.c            |  1 +
>>>    core/stream_interface.c     |  6 +++---
>>>    corelib/lua_interface.c     |  2 ++
>>>    handlers/copy_handler.c     |  1 +
>>>    handlers/delta_handler.c    |  1 +
>>>    handlers/rdiff_handler.c    |  1 +
>>>    handlers/readback_handler.c |  1 +
>>>    include/util.h              |  6 ++++--
>>>    9 files changed, 23 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/core/cpio_utils.c b/core/cpio_utils.c
>>> index 5b99904..6e2c239 100644
>>> --- a/core/cpio_utils.c
>>> +++ b/core/cpio_utils.c
>>> @@ -431,7 +431,8 @@ 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, bool encrypted,
>>> +     const char __attribute__ ((__unused__)) *imgaeskey, const char *imgivt, writeimage callback)
>>
>> Change of fingerprint of this core function does not convince me. I am
> 
> Somehow the image->aeskey_ascii (from parser.c) must be passed in
> __swupdate_copy(cpio_utils.c), just like the ivt.

Well, the aeskey is currently global and the copy* function just access 
it via global function, that is the key is not passed. Reason is that 
the IVT can  be set individually for each image, the key not. And 
SWupdate from design does not install artifacts in parallel to stop at 
the first error. So my idea is that the key is not passed as it is not 
passed now for artifact decryption, but the copy* function will get the 
key from a global function.

The copyfile / swupdate_copyfile functions should know if the incoming 
stream should be decrypted with an asymmetric key. My idea is that the 
"encrypted" parameters will change from bool to enum to select the 
decryption type (and in future, maybe the algo used).


> However, I could combine
> aes-key and ivt in the struct decryption_key, then you only have one
> argument, but
> the fingerprint of the core function also changes as a result. What do you
> think?
> 
> struct decryption_key {
>      unsigned char key[AES_256_KEY_LEN];
>      char keylen;
>      unsigned char ivt[AES_BLK_SIZE];
> };

It does change again the fingerprint - really required ?

If activated, sw-description must always be encrypted with asymmetric 
key. In stream_interface.c, #ifdef ASYM_ENCRYPTED_SW_DESCRIPTION can be 
used.

> 
>> also not convinced about "recipient" tag in several part of the series.
> 
> The term "recipient" is taken from openssl cms and comes from email encryption,

Right, but it sounds weird in SWUpdate where CMS is not used for mails.

> but I'm open to a better suggestion if you have one. Would probably want to
> introduce a new parameter rather than rename -K --key-aes and aeskeyfname to
> something more generic for compatibility reasons though.

Yes - both parameters can be used at once. -K is the symmetric key for 
artifact and can be used as default key. For the asymettric key, I 
suggest a new command line parameter.

> 
>>
>> SWUpdate works sequentially, that means two decryption of an artifact at
>> the same time is excluded by design.
>>
>> The decryption of sw-description is done inside a single function
>> (extract_file_to_tmp), and this is local to the stream_interface module.
>> Type of decryption (symmetric vs asymettric) should be managed inside
>> this function without spreading to all interfaces.
>>
>> The sw-dgst structure (global) can have a flag to distinguish between
>> symmetric and asymmetric decryption. The cpio functions should just know
>> if artifact (or sw-description) to be handled is encrypted without being
>> aware of detailt like type of the decryption and keys.
> 
> I can move the asymmetric decryption of the sw-description from the
> stream_interface.c to __swupdate_copy (cpio_utils.c) (is that what you mean?),
> but to do this you would have to:
> - pass the file name or
> - change the bool encrypted to enum encrypted { NO_ENC, SYM, ASYM)
     ^
     |____ I like this !


> to recognize the sw-description. Asymmetrical decryption doesn't make sense for
> the other artifacts anyway.

Fully agree.

> And the asym keypair has to be read from global swupdate_cfg.dgst via
> get_swupdate_cfg(), if I understand you correctly.

Or via a command line parameter as usual.

> 
> Another aspect regarding the partial decryption like we do with the
> swupdate_DECRYPT_* functions, it is not supported by openssl cms. This means
> that you cannot decrypt the sw-description chunk by chunk. At the end, the
> sw-description has to be read completely once in order to be able to decrypt it.

Ok


Best regards,
Stefano

> 
>>
>> Best regards,
>> Stefano
>>
>>>    {
>>>        unsigned int percent, prevpercent = 0;
>>>        int ret = 0;
>>> @@ -705,7 +706,8 @@ 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, bool encrypted,
>>> +     const char *imgaeskey, const char *imgivt, writeimage callback)
>>>    {
>>>        return __swupdate_copy(fdin,
>>>                                NULL,
>>> @@ -718,12 +720,13 @@ int copyfile(int fdin, void *out, size_t nbytes, unsigned long *offs, unsigned l
>>>                                checksum,
>>>                                hash,
>>>                                encrypted,
>>> +                             imgaeskey,
>>>                                imgivt,
>>>                                callback);
>>>    }
>>>
>>>    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, bool encrypted, const char *imgaeskey, const char *imgivt, writeimage callback)
>>>    {
>>>        return __swupdate_copy(-1,
>>>                                inbuf,
>>> @@ -736,6 +739,7 @@ int copybuffer(unsigned char *inbuf, void *out, size_t nbytes, int __attribute__
>>>                                NULL,
>>>                                hash,
>>>                                encrypted,
>>> +                             imgaeskey,
>>>                                imgivt,
>>>                                callback);
>>>    }
>>> @@ -752,6 +756,7 @@ int copyimage(void *out, struct img_type *img, writeimage callback)
>>>                        &img->checksum,
>>>                        img->sha256,
>>>                        img->is_encrypted,
>>> +                     img->aeskey_ascii,
>>>                        img->ivt_ascii,
>>>                        callback);
>>>    }
>>> @@ -837,7 +842,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) {
>>> +                             false, NULL, NULL, NULL) != 0) {
>>>                        ERROR("invalid archive");
>>>                        return -1;
>>>                }
>>> diff --git a/core/installer.c b/core/installer.c
>>> index 20b5b51..db86075 100644
>>> --- a/core/installer.c
>>> +++ b/core/installer.c
>>> @@ -145,6 +145,7 @@ static int extract_scripts(struct imglist *head)
>>>                                &checksum,
>>>                                script->sha256,
>>>                                script->is_encrypted,
>>> +                             script->aeskey_ascii,
>>>                                script->ivt_ascii,
>>>                                NULL);
>>>                close(fdin);
>>> diff --git a/core/stream_interface.c b/core/stream_interface.c
>>> index 0b78329..bfafa30 100644
>>> --- a/core/stream_interface.c
>>> +++ b/core/stream_interface.c
>>> @@ -104,7 +104,7 @@ static int extract_file_to_tmp(int fd, const char *fname, unsigned long *poffs,
>>>                return -1;
>>>
>>>        if (copyfile(fd, &fdout, fdh.size, poffs, 0, 0, 0, &checksum, NULL,
>>> -                  encrypted, NULL, NULL) < 0) {
>>> +                  encrypted, NULL, NULL, NULL) < 0) {
>>>                close(fdout);
>>>                return -1;
>>>        }
>>> @@ -243,7 +243,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, false, NULL, NULL, NULL) < 0) {
>>>                                        close(fdout);
>>>                                        return -1;
>>>                                }
>>> @@ -255,7 +255,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, false, NULL, NULL, NULL) < 0) {
>>>                                        return -1;
>>>                                }
>>>                                if (!swupdate_verify_chksum(checksum, &fdh)) {
>>> diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
>>> index af7b554..1533e9d 100644
>>> --- a/corelib/lua_interface.c
>>> +++ b/corelib/lua_interface.c
>>> @@ -401,6 +401,7 @@ static int l_copy2file(lua_State *L)
>>>                                 &checksum,
>>>                                 img.sha256,
>>>                                 img.is_encrypted,
>>> +                              img.aeskey_ascii,
>>>                                 img.ivt_ascii,
>>>                                 NULL);
>>>        update_table(L, &img);
>>> @@ -473,6 +474,7 @@ static int l_istream_read(lua_State* L)
>>>                                 &checksum,
>>>                                 img.sha256,
>>>                                 img.is_encrypted,
>>> +                              img.aeskey_ascii,
>>>                                 img.ivt_ascii,
>>>                                 istream_read_callback);
>>>
>>> diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c
>>> index e463bb5..d09ca52 100644
>>> --- a/handlers/copy_handler.c
>>> +++ b/handlers/copy_handler.c
>>> @@ -131,6 +131,7 @@ static int copy_single_file(const char *path, ssize_t size, struct img_type *img
>>>                        &checksum,
>>>                        0, /* no sha256 */
>>>                        false, /* no encrypted */
>>> +                     NULL, /* no AES Key */
>>>                        NULL, /* no IVT */
>>>                        NULL);
>>>
>>> diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c
>>> index d1ff783..a5ee2a6 100644
>>> --- a/handlers/delta_handler.c
>>> +++ b/handlers/delta_handler.c
>>> @@ -169,6 +169,7 @@ static int network_process_data(multipart_parser* p, const char *at, size_t leng
>>>                                                 hash,
>>>                                                 0,
>>>                                                 NULL,
>>> +                                              NULL,
>>>                                                 NULL);
>>>                        } else
>>>                                ret = 0; /* skipping, nothing to be copied */
>>> diff --git a/handlers/rdiff_handler.c b/handlers/rdiff_handler.c
>>> index e01a127..3f09ec2 100644
>>> --- a/handlers/rdiff_handler.c
>>> +++ b/handlers/rdiff_handler.c
>>> @@ -347,6 +347,7 @@ static int apply_rdiff_patch(struct img_type *img,
>>>                        &img->checksum,
>>>                        img->sha256,
>>>                        img->is_encrypted,
>>> +                     img->aeskey_ascii,
>>>                        img->ivt_ascii,
>>>                        apply_rdiff_chunk_cb);
>>>        if (ret != 0) {
>>> diff --git a/handlers/readback_handler.c b/handlers/readback_handler.c
>>> index 4b910bd..6d2eefa 100644
>>> --- a/handlers/readback_handler.c
>>> +++ b/handlers/readback_handler.c
>>> @@ -113,6 +113,7 @@ static int readback_postinst(struct img_type *img)
>>>                        NULL,  /* no checksum */
>>>                        hash,
>>>                        false,     /* no encrypted */
>>> +                     NULL,  /* no AES Key */
>>>                        NULL,     /* no IVT */
>>>                        NULL); /* no callback */
>>>        if (status == 0) {
>>> diff --git a/include/util.h b/include/util.h
>>> index 062840f..0c5564c 100644
>>> --- a/include/util.h
>>> +++ b/include/util.h
>>> @@ -204,10 +204,12 @@ 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, bool encrypted, const char *imgaeskey, 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, bool encrypted, const char *imgaeskey, 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);
> 
> Best regards,
> Michael
>
diff mbox series

Patch

diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index 5b99904..6e2c239 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -431,7 +431,8 @@  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, bool encrypted,
+	const char __attribute__ ((__unused__)) *imgaeskey, const char *imgivt, writeimage callback)
 {
 	unsigned int percent, prevpercent = 0;
 	int ret = 0;
@@ -705,7 +706,8 @@  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, bool encrypted,
+	const char *imgaeskey, const char *imgivt, writeimage callback)
 {
 	return __swupdate_copy(fdin,
 				NULL,
@@ -718,12 +720,13 @@  int copyfile(int fdin, void *out, size_t nbytes, unsigned long *offs, unsigned l
 				checksum,
 				hash,
 				encrypted,
+				imgaeskey,
 				imgivt,
 				callback);
 }
 
 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, bool encrypted, const char *imgaeskey, const char *imgivt, writeimage callback)
 {
 	return __swupdate_copy(-1,
 				inbuf,
@@ -736,6 +739,7 @@  int copybuffer(unsigned char *inbuf, void *out, size_t nbytes, int __attribute__
 				NULL,
 				hash,
 				encrypted,
+				imgaeskey,
 				imgivt,
 				callback);
 }
@@ -752,6 +756,7 @@  int copyimage(void *out, struct img_type *img, writeimage callback)
 			&img->checksum,
 			img->sha256,
 			img->is_encrypted,
+			img->aeskey_ascii,
 			img->ivt_ascii,
 			callback);
 }
@@ -837,7 +842,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) {
+				false, NULL, NULL, NULL) != 0) {
 			ERROR("invalid archive");
 			return -1;
 		}
diff --git a/core/installer.c b/core/installer.c
index 20b5b51..db86075 100644
--- a/core/installer.c
+++ b/core/installer.c
@@ -145,6 +145,7 @@  static int extract_scripts(struct imglist *head)
 				&checksum,
 				script->sha256,
 				script->is_encrypted,
+				script->aeskey_ascii,
 				script->ivt_ascii,
 				NULL);
 		close(fdin);
diff --git a/core/stream_interface.c b/core/stream_interface.c
index 0b78329..bfafa30 100644
--- a/core/stream_interface.c
+++ b/core/stream_interface.c
@@ -104,7 +104,7 @@  static int extract_file_to_tmp(int fd, const char *fname, unsigned long *poffs,
 		return -1;
 
 	if (copyfile(fd, &fdout, fdh.size, poffs, 0, 0, 0, &checksum, NULL,
-		     encrypted, NULL, NULL) < 0) {
+		     encrypted, NULL, NULL, NULL) < 0) {
 		close(fdout);
 		return -1;
 	}
@@ -243,7 +243,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, false, NULL, NULL, NULL) < 0) {
 					close(fdout);
 					return -1;
 				}
@@ -255,7 +255,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, false, NULL, NULL, NULL) < 0) {
 					return -1;
 				}
 				if (!swupdate_verify_chksum(checksum, &fdh)) {
diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
index af7b554..1533e9d 100644
--- a/corelib/lua_interface.c
+++ b/corelib/lua_interface.c
@@ -401,6 +401,7 @@  static int l_copy2file(lua_State *L)
 				 &checksum,
 				 img.sha256,
 				 img.is_encrypted,
+				 img.aeskey_ascii,
 				 img.ivt_ascii,
 				 NULL);
 	update_table(L, &img);
@@ -473,6 +474,7 @@  static int l_istream_read(lua_State* L)
 				 &checksum,
 				 img.sha256,
 				 img.is_encrypted,
+				 img.aeskey_ascii,
 				 img.ivt_ascii,
 				 istream_read_callback);
 
diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c
index e463bb5..d09ca52 100644
--- a/handlers/copy_handler.c
+++ b/handlers/copy_handler.c
@@ -131,6 +131,7 @@  static int copy_single_file(const char *path, ssize_t size, struct img_type *img
 			&checksum,
 			0, /* no sha256 */
 			false, /* no encrypted */
+			NULL, /* no AES Key */
 			NULL, /* no IVT */
 			NULL);
 
diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c
index d1ff783..a5ee2a6 100644
--- a/handlers/delta_handler.c
+++ b/handlers/delta_handler.c
@@ -169,6 +169,7 @@  static int network_process_data(multipart_parser* p, const char *at, size_t leng
 						 hash,
 						 0,
 						 NULL,
+						 NULL,
 						 NULL);
 			} else
 				ret = 0; /* skipping, nothing to be copied */
diff --git a/handlers/rdiff_handler.c b/handlers/rdiff_handler.c
index e01a127..3f09ec2 100644
--- a/handlers/rdiff_handler.c
+++ b/handlers/rdiff_handler.c
@@ -347,6 +347,7 @@  static int apply_rdiff_patch(struct img_type *img,
 			&img->checksum,
 			img->sha256,
 			img->is_encrypted,
+			img->aeskey_ascii,
 			img->ivt_ascii,
 			apply_rdiff_chunk_cb);
 	if (ret != 0) {
diff --git a/handlers/readback_handler.c b/handlers/readback_handler.c
index 4b910bd..6d2eefa 100644
--- a/handlers/readback_handler.c
+++ b/handlers/readback_handler.c
@@ -113,6 +113,7 @@  static int readback_postinst(struct img_type *img)
 			NULL,  /* no checksum */
 			hash,
 			false,     /* no encrypted */
+			NULL,  /* no AES Key */
 			NULL,     /* no IVT */
 			NULL); /* no callback */
 	if (status == 0) {
diff --git a/include/util.h b/include/util.h
index 062840f..0c5564c 100644
--- a/include/util.h
+++ b/include/util.h
@@ -204,10 +204,12 @@  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, bool encrypted, const char *imgaeskey, 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, bool encrypted, const char *imgaeskey, 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);